Re: [PATCH 5/8] nfsd: give the open file cache its own mutex

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2026-09-22 at 16:37 +1000, NeilBrown wrote:
> On Tue, 22 Sep 2026, Jeff Layton wrote:
> > nfsd_global_mutex still serializes file cache flushes against server
> > start/stop in unrelated namespaces. nfsd_file_cache_purge() is reached from
> > expkey_flush() on every "exportfs -f", which makes it the only frequent
> > taker of the global lock -- and it has nothing to do with nfsd_users or the
> > NFSv4 global tables it would be waiting on.
> > 
> > Move NFSD_FILE_CACHE_UP and the objects it covers (rhltable, LRU, slabs,
> > shrinker, fsnotify groups) onto a nfsd_file_cache_mutex private to
> > filecache.c. nfsd_file_cache_init() and nfsd_file_cache_shutdown() take it
> > themselves rather than asserting the caller holds the global one, so
> > nfssvc.c no longer needs to know how the cache locks itself.
> > 
> > nfsd_global_mutex is left holding only nfsd_users, the notifier count and
> > user_recovery_dirname, all of which are touched solely on server
> > start/stop.
> > 
> > Ordering is nn->nfsd_mutex outside nfsd_global_mutex outside
> > nfsd_file_cache_mutex. The cache mutex is reached three ways, all
> > consistent with that: from nfsd_startup_generic()/nfsd_shutdown_generic()
> > under the global mutex, from nfsd_shutdown_net() under nn->nfsd_mutex, and
> > bare from expkey_flush() and the filecache stats file. Nothing called with
> > the cache mutex held takes either of the others.
> > 
> > Assisted-by: LLM
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/nfsd/filecache.c | 39 ++++++++++++++++++++++++++++-----------
> >  fs/nfsd/nfssvc.c    |  9 +++++----
> >  2 files changed, 33 insertions(+), 15 deletions(-)
> > 
> > diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> > index 501772bc8f8d..179362e5394b 100644
> > --- a/fs/nfsd/filecache.c
> > +++ b/fs/nfsd/filecache.c
> > @@ -67,6 +67,17 @@
> >   */
> >  static DEFINE_SPINLOCK(nfsd_gc_lock);
> >  
> > +/*
> > + * Guards NFSD_FILE_CACHE_UP and the host-wide objects it covers: the
> > + * rhltable, the LRU, the slabs, the shrinker and the fsnotify groups.
> > + * Held across cache bring-up and teardown, and by readers and walkers
> > + * that need the cache to stay up for the duration (->cache_purge, the
> > + * stats file).
> > + *
> > + * Nests inside nfsd_global_mutex and inside nn->nfsd_mutex.
> > + */
> > +static DEFINE_MUTEX(nfsd_file_cache_mutex);
> > +
> >  static DEFINE_PER_CPU(unsigned long, nfsd_file_cache_hits);
> >  static DEFINE_PER_CPU(unsigned long, nfsd_file_acquisitions);
> >  static DEFINE_PER_CPU(unsigned long, nfsd_file_allocations);
> > @@ -869,9 +880,11 @@ nfsd_file_cache_init(void)
> >  {
> >  	int ret;
> >  
> > -	lockdep_assert_held(&nfsd_global_mutex);
> > -	if (test_and_set_bit(NFSD_FILE_CACHE_UP, &nfsd_file_flags) == 1)
> > +	mutex_lock(&nfsd_file_cache_mutex);
> 
> What would you think of making this
> 
>    guard(mutex)(&nfsd_file_cache_mutex);
> 
> so you don't need to make other changes in this function??
> 
> 

Sounds good. I'll do that.

Thanks!
-- 
Jeff Layton <jlayton@kernel.org>