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@xxxxxxxxxxxx> 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@xxxxxxxxxx> wrote: > > > > > > > On Fri, 6 Jun 2008 13:24:31 -0400 > > > > "J. Bruce Fields" <bfields@xxxxxxxxxxxx> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
