On 7/4/25 3:20 AM, NeilBrown wrote:
> We currently set client_tracking_active precisely when it is safe for
> laundromat_work to be scheduled. It is possible to enable/disable
> laundromat_work, so we can do that instead of having a separate flag.
>
> Doing this avoids overloading ->state_lock with a use that is only
> tangentially related to the other uses.
>
> Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
> ---
> fs/nfsd/netns.h | 1 -
> fs/nfsd/nfs4state.c | 24 ++++++++++--------------
> 2 files changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index fe8338735e7c..d83c68872c4c 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -67,7 +67,6 @@ struct nfsd_net {
> struct lock_manager nfsd4_manager;
> bool grace_ended;
> bool grace_end_forced;
> - bool client_tracking_active;
> time64_t boot_time;
>
> struct dentry *nfsd_client_dir;
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 124fe4f669aa..db292ac473c6 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -6512,12 +6512,12 @@ nfsd4_force_end_grace(struct nfsd_net *nn)
> {
> if (!nn->client_tracking_ops)
> return false;
> - spin_lock(&nn->client_lock);
> - if (nn->client_tracking_active) {
> - nn->grace_end_forced = true;
> - mod_delayed_work(laundry_wq, &nn->laundromat_work, 0);
> - }
> - spin_unlock(&nn->client_lock);
> + /* laundromat_work must be initialised now, though it might be disabled */
> + nn->grace_end_forced = true;
> + /* This is a no-op after nfs4_state_shutdown_net() has called
> + * disable_delayed_work_sync()
> + */
> + mod_delayed_work(laundry_wq, &nn->laundromat_work, 0);
> return true;
> }
>
> @@ -8840,7 +8840,6 @@ static int nfs4_state_create_net(struct net *net)
> nn->boot_time = ktime_get_real_seconds();
> nn->grace_ended = false;
> nn->grace_end_forced = false;
> - nn->client_tracking_active = false;
> nn->nfsd4_manager.block_opens = true;
> INIT_LIST_HEAD(&nn->nfsd4_manager.list);
> INIT_LIST_HEAD(&nn->client_lru);
> @@ -8855,6 +8854,8 @@ static int nfs4_state_create_net(struct net *net)
> INIT_LIST_HEAD(&nn->blocked_locks_lru);
>
> INIT_DELAYED_WORK(&nn->laundromat_work, laundromat_main);
> + /* Make sure his cannot run until client tracking is initialised */
Nit: maybe it's "Make sure /t/his cannot run " ?
I agree that backporting enable_delayed_work() is not practical.
> + disable_delayed_work(&nn->laundromat_work);
> INIT_WORK(&nn->nfsd_shrinker_work, nfsd4_state_shrinker_worker);
> get_net(net);
>
> @@ -8922,9 +8923,7 @@ nfs4_state_start_net(struct net *net)
> locks_start_grace(net, &nn->nfsd4_manager);
> nfsd4_client_tracking_init(net);
> /* safe for laundromat to run now */
> - spin_lock(&nn->client_lock);
> - nn->client_tracking_active = true;
> - spin_unlock(&nn->client_lock);
> + enable_delayed_work(&nn->laundromat_work);
> if (nn->track_reclaim_completes && nn->reclaim_str_hashtbl_size == 0)
> goto skip_grace;
> printk(KERN_INFO "NFSD: starting %lld-second grace period (net %x)\n",
> @@ -8973,10 +8972,7 @@ nfs4_state_shutdown_net(struct net *net)
>
> shrinker_free(nn->nfsd_client_shrinker);
> cancel_work_sync(&nn->nfsd_shrinker_work);
> - spin_lock(&nn->client_lock);
> - nn->client_tracking_active = false;
> - spin_unlock(&nn->client_lock);
> - cancel_delayed_work_sync(&nn->laundromat_work);
> + disable_delayed_work_sync(&nn->laundromat_work);
> locks_end_grace(&nn->nfsd4_manager);
>
> INIT_LIST_HEAD(&reaplist);
--
Chuck Lever
