Re: [PATCH] nfsd: use a multithreaded workqueue for nfsd4_callbacks

[Jeff Layton <jlayton@poochiereds.net>]

On Mon, 12 Oct 2015 14:11:17 -0400
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Fri, Oct 09, 2015 at 05:39:23PM -0400, Jeff Layton wrote:
> > On Fri, 9 Oct 2015 17:24:59 -0400
> > "J. Bruce Fields" <bfields@fieldses.org> wrote:
> > 
> > > On Sat, Oct 03, 2015 at 08:38:02AM -0400, Jeff Layton wrote:
> > > > I don't see any need to order callbacks with respect to one another.
> > > 
> > > I thought the code in nfsd4_process_cb_update really depended on this.
> > > The locking it has is against nfsd threads, it probably assumes it's
> > > only run from a cb thread and that it's the only one running at a time.
> > > 
> > > But I haven't reviewed it lately.
> > > 
> > > --b.
> > > 
> > 
> > Yikes -- ok. That's not at all obvious. That should prob be documented
> > if so.
> 
> Yes, my bad.
> 
> > Yeah, ok...I guess you could end up with multiple threads racing to
> > tear down the cb_client and xprt and create a new one, and it looks
> > like the cl_cb_client and cl_cred pointers could get clobbered by new
> > ones in that case.
> > 
> > Shouldn't be too hard to fix protecting those pointers with the
> > cl_lock. That said, I prob won't be able to spend time on it right now.
> > You can go ahead and drop that patch and I'll resend if/when I get
> > around to fixing that.
> 
> What's the problem that this fixes exactly?
> 

Unnecessary serialization of callbacks?

Not that that's necessarily a problem, but with the newer workqueue
implementation there is more overhead in running a single threaded
workqueue since it implies a rescuer thread and has to do extra work
to ensure that the jobs are run in sequential order.

If that serialization is not actually needed, then it's better to move
it to a multithreaded workqueue.

-- 
Jeff Layton <jlayton@poochiereds.net>