[linux-cifs-client] [PATCH 2/3] [CIFS] convert cifs_demultiplex_thread to use nonblocking sockets

[Jeff Layton]

On Thu, 1 May 2008 12:30:43 -0400
Christoph Hellwig <hch at infradead.org> wrote:

> On Thu, Apr 17, 2008 at 04:59:05PM -0400, Jeff Layton wrote:
> > Currently, cifs_demultiplex_thread uses blocking calls to
> > kernel_recvmsg to receive data. These calls do not return immediately
> > when kthread_stop tries to wake the thread, so cifs_mount and
> > cifs_umount also signal the thread to make it break out of kernel_recvmsg.
> > 
> > This patch converts cifs_demultiplex_thread to repeatedly check the
> > socket with a TIOCINQ ioctl to see if there is data waiting. If there
> > isn't then it sleeps a bit and polls again. If there is data then
> > we call kernel_recvmsg in non-blocking mode to get it. Since we're
> > polling like this, we can check kthread_should_stop on each poll and
> > no longer need to signal when tearing down the thread.
> 
> Wouldn't it be better if we do real event driver non-blocking I/O,
> like an poll or epoll loop in userspace. I don't think this has
> been done in kernelspace before, but I don't see any fundamental reason
> why we can't do it. 
> 

I had a quick look over what poll() does and this doesn't look
trivial. It sounds interesting though and is something we should
probably consider for the future.

I think it would probably be best to start with something like this
ioctl() polling since that is used elsewhere in kernel and we know it
works. Once we have that in place and feel good about it we can
consider converting that to use something similar to what poll() calls
do.

My main concern with the current patch is error handling. I pretty much
set this up to not worry too much about dealing with errors from the
ioctl, and to just assume that if the ioctl returns a real error that
the following kernel_recvmsg call would also return one. I think that's
a reasonably safe assumption, but let me know if you don't think it
is...

Cheers,
-- 
Jeff Layton <jlayton at redhat.com>