Re: [PATCH 4/5] knfsd: convert knfsd to kthread API

["J. Bruce Fields" <bfields@fieldses.org>]

On Mon, Jun 09, 2008 at 09:19:48AM -0400, Jeff Layton wrote:
> On Mon, 9 Jun 2008 09:08:30 -0400
> "J. Bruce Fields" <bfields@fieldses.org> wrote:
> 
> > On Fri, Jun 06, 2008 at 03:49:48PM -0400, Jeff Layton wrote:
> > > On Fri, 6 Jun 2008 14:11:16 -0400
> > > Jeff Layton <jlayton@redhat.com> wrote:
> > > 
> > > > On Fri, 6 Jun 2008 13:24:31 -0400
> > > > "J. Bruce Fields" <bfields@fieldses.org> wrote:
> > > > 
> > > ...
> > > > > How does the module refcounting work after this patch?
> > > > > 
> > > > > --b.
> > > > > 
> > > > 
> > > > I think I've goofed this part, actually. I was thinking that we didn't
> > > > need to bump the refcount here, and that the kernel would realize that
> > > > nfsd() hadn't returned and would prevent unloading until it had. This
> > > > doesn't seem to be the case. I'll need to go back and add refcounting
> > > > back in.
> > > > 
> > > 
> > > Here's a respun patch that adds back in the module refcounts and also
> > > removes the unneeded "err = 0;" at the bottom of the loop. Thoughts?
> > 
> > Looks good to me.  I'll apply all 5 (with this version of #4) if noone
> > catches something else.
> > 
> > --b.
> > 
> 
> Sounds good. My only concern here is whether moving the __module_get
> from the RPC layer to nfsd() itself is OK.

Oh, I see, I missed that.

> I *think* it is since the
> nfsctl and /proc/fs/nfsd routines are all part of the nfsd module, so
> we're guaranteed to have a reference there anyway, but if there are
> potential races then we may want to go back to the old way.

The vfs should take care that e.g. it gets a reference on the module
before creating an open file for the nfsd filesystem.  But it don't see
how anything can guarantee that the __module_get() in the new nfsd
thread completes before whoever called svc_set_num_threads() returns and
drops their reference.

So, yeah, I think it's not right.

> I'd appreciate an ack/nack on this from someone who understands module
> refcounts better than me.

Not that I'll calim to be an expert there....

--b.