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 <[email protected]>
> ---
>  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??

NeilBrown


> +     if (test_and_set_bit(NFSD_FILE_CACHE_UP, &nfsd_file_flags) == 1) {
> +             mutex_unlock(&nfsd_file_cache_mutex);
>               return 0;
> +     }
>  
>       ret = rhltable_init(&nfsd_file_rhltable, &nfsd_file_rhash_params);
>       if (ret)
> @@ -937,6 +950,7 @@ nfsd_file_cache_init(void)
>  out:
>       if (ret)
>               clear_bit(NFSD_FILE_CACHE_UP, &nfsd_file_flags);
> +     mutex_unlock(&nfsd_file_cache_mutex);
>       return ret;
>  out_notify_group:
>       fsnotify_put_group(nfsd_file_fsnotify_group);
> @@ -1011,16 +1025,16 @@ nfsd_file_cache_start_net(struct net *net)
>   * nfsd_file_cache_purge - Remove all cache items associated with @net
>   * @net: target net namespace
>   *
> - * Takes nfsd_global_mutex so the cache cannot be torn down underneath the
> - * walk.  Callers must not already hold it.
> + * Takes nfsd_file_cache_mutex so the cache cannot be torn down underneath
> + * the walk.  Callers must not already hold it.
>   */
>  void
>  nfsd_file_cache_purge(struct net *net)
>  {
> -     mutex_lock(&nfsd_global_mutex);
> +     mutex_lock(&nfsd_file_cache_mutex);
>       if (test_bit(NFSD_FILE_CACHE_UP, &nfsd_file_flags) == 1)
>               __nfsd_file_cache_purge(net);
> -     mutex_unlock(&nfsd_global_mutex);
> +     mutex_unlock(&nfsd_file_cache_mutex);
>  }
>  
>  void
> @@ -1045,9 +1059,11 @@ nfsd_file_cache_shutdown(void)
>  {
>       int i;
>  
> -     lockdep_assert_held(&nfsd_global_mutex);
> -     if (test_and_clear_bit(NFSD_FILE_CACHE_UP, &nfsd_file_flags) == 0)
> +     mutex_lock(&nfsd_file_cache_mutex);
> +     if (test_and_clear_bit(NFSD_FILE_CACHE_UP, &nfsd_file_flags) == 0) {
> +             mutex_unlock(&nfsd_file_cache_mutex);
>               return;
> +     }
>  
>       lease_unregister_notifier(&nfsd_file_lease_notifier);
>       shrinker_free(nfsd_file_shrinker);
> @@ -1078,6 +1094,7 @@ nfsd_file_cache_shutdown(void)
>               per_cpu(nfsd_file_total_age, i) = 0;
>               per_cpu(nfsd_file_evictions, i) = 0;
>       }
> +     mutex_unlock(&nfsd_file_cache_mutex);
>  }
>  
>  static struct nfsd_file *
> @@ -1476,8 +1493,8 @@ int nfsd_file_cache_stats_show(struct seq_file *m, void 
> *v)
>       unsigned int i, count = 0, buckets = 0;
>       unsigned long lru = 0, total_age = 0;
>  
> -     /* Serialize with server shutdown */
> -     mutex_lock(&nfsd_global_mutex);
> +     /* Serialize with cache teardown */
> +     mutex_lock(&nfsd_file_cache_mutex);
>       if (test_bit(NFSD_FILE_CACHE_UP, &nfsd_file_flags) == 1) {
>               struct bucket_table *tbl;
>               struct rhashtable *ht;
> @@ -1491,7 +1508,7 @@ int nfsd_file_cache_stats_show(struct seq_file *m, void 
> *v)
>               buckets = tbl->size;
>               rcu_read_unlock();
>       }
> -     mutex_unlock(&nfsd_global_mutex);
> +     mutex_unlock(&nfsd_file_cache_mutex);
>  
>       for_each_possible_cpu(i) {
>               hits += per_cpu(nfsd_file_cache_hits, i);
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index 5cb92c3829f9..b2c215775e08 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -66,11 +66,12 @@ static __be32                     
> nfsd_init_request(struct svc_rqst *,
>   *
>   * nfsd_global_mutex covers only what is genuinely shared between
>   * namespaces: the nfsd_users refcount and the host-wide resources it
> - * brings up and tears down (the open file cache and the NFSv4 global
> - * tables), the address-notifier registration, and user_recovery_dirname.
> + * brings up and tears down (the NFSv4 global tables, and the open file
> + * cache -- which guards its own internals with nfsd_file_cache_mutex),
> + * the address-notifier registration, and user_recovery_dirname.
>   *
> - * Lock ordering is nn->nfsd_mutex outside nfsd_global_mutex.  Nothing takes
> - * two namespaces' nfsd_mutexes.
> + * Lock ordering is nn->nfsd_mutex outside nfsd_global_mutex outside
> + * nfsd_file_cache_mutex.  Nothing takes two namespaces' nfsd_mutexes.
>   */
>  DEFINE_MUTEX(nfsd_global_mutex);
>  
> 
> -- 
> 2.55.0
> 
> 


Reply via email to