On Sun, 22 Jun 2025, Chuck Lever wrote: > On 6/20/25 7:33 PM, NeilBrown wrote: > > The first patch hopefully fixes a bug with locking as reported by Li > > Lingfeng: some write_foo functions aren't locked properly. > > > > The other two improve the locking code, particulary so that we don't > > need a global mutex to change per-netns data. > > > > I've revised the locking to use guard(mutex) for (almost) all places > > that the per-netfs mutex is used. I think this is an improvement but > > would like to know what others think. > > > > I haven't changed _get/_put to _pin/_unpin as Chuck wondered about. I'm > > not against that (though get/put are widely understood) but nor am I > > particularly for it yet. Again, opinions are welcome. > > I think of get and put as operations you do on an object. Saying > > nfsd_startup_get(); > > seems a little strange to me. As I said before, it seems like you > are protecting a critical section, not a particular object. I agree it looks like a critical section. That suggests lock/unlock naming. A "get" is somewhat like a read_trylock. But a put isn't much like an unlock. But maybe I can side-step the whole issue. I think it is reasonable to move the nfsd_shutdown_generic() code in to nfsd_exit() which is called at module_put time. There is no real need for any this before then. With that done the ref on the module that we already maintain correctly will ensure no code can race with the cleanup in nfsd_shutdown_generic(). Then we don't need nfsd_users as a counter - just a flag to say if startup has completed yet. This series does that. It also delays the conversion to guard(mutex) until a final patch so it can be reviewed separately - or rejected separately. Thanks, NeilBrown
