Re: [PATCH v4 4/8] NFSD: Send a meaningful CB_RECALL_ANY keep count

On Sat, 15 Aug 2026 at 18:39, Chuck Lever <[email protected]> wrote:
>
> deleg_reaper() sets craa_objects_to_keep to zero on every
> CB_RECALL_ANY. RFC 8881 Section 20.6.3 defines that field as the
> number of objects the client may keep, leaving the client to choose
> which of the excess to return, because the server cannot read lack
> of recent use as lack of usefulness. Zero asks for every delegation
> the client holds, including the ones backing files an application
> still has open.
>
> There is also no reason NFSD has to reclaim the entire delegation
> working set on the first sign of memory pressure.
>
> Derive the keep count from cl_deleg_count so that each callback
> asks for one delegation. Both the shrinker and the laundromat re-arm
> while their condition lasts, so a client with more to give is asked
> again on the next pass.
>
> The Linux client ignores craa_objects_to_keep and returns unused
> delegations selected from the type mask alone, so the count changes
> nothing for it.
>
> Fixes: 44df6f439a17 ("NFSD: add delegation reaper to react to low memory condition")
> Signed-off-by: Chuck Lever <[email protected]>
> Reviewed-by: Jeff Layton <[email protected]>
> ---
>  fs/nfsd/nfs4state.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 2214b4d60c7e..5c2dd6657fac 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -7942,6 +7942,7 @@ deleg_reaper(struct nfsd_net *nn)
>  {
>         struct list_head *pos, *next;
>         struct nfs4_client *clp;
> +       unsigned int count;
>
>         spin_lock(&nn->client_lock);
>         list_for_each_safe(pos, next, &nn->client_lru) {
> @@ -7951,21 +7952,33 @@ deleg_reaper(struct nfsd_net *nn)
>                         continue;
>                 if (clp->cl_state != NFSD4_ACTIVE)
>                         continue;
> -               if (list_empty(&clp->cl_delegations))
> -                       continue;
>                 if (atomic_read(&clp->cl_delegs_in_recall))
>                         continue;
>                 if (ktime_get_boottime_seconds() - clp->cl_ra_time < 5)
>                         continue;
>                 if (clp->cl_cb_state != NFSD4_CB_UP)
>                         continue;
> +               /*
> +                * This read races with hash_delegation_locked() and
> +                * unhash_delegation_locked() on other CPUs. A stale
> +                * count only skews the keep value; the next
> +                * laundromat pass sees a more current one.
> +                */
> +               count = data_race(READ_ONCE(clp->cl_deleg_count));
> +               if (!count)
> +                       continue;
>                 if (test_and_set_bit(NFSD4_CALLBACK_RUNNING, &clp->cl_ra->ra_cb.cb_flags))
>                         continue;
>
>                 /* release in nfsd4_cb_recall_any_release */
>                 kref_get(&clp->cl_nfsdfs.cl_ref);
>                 clp->cl_ra_time = ktime_get_boottime_seconds();
> -               clp->cl_ra->ra_keep = 0;
> +               /*
> +                * Ask for a single delegation. Recalling one before it
> +                * is needed costs the client an OPEN when it next
> +                * touches the file.
> +                */
> +               clp->cl_ra->ra_keep = count - 1;
>                 clp->cl_ra->ra_bmval[0] = BIT(RCA4_TYPE_MASK_RDATA_DLG) |
>                                                 BIT(RCA4_TYPE_MASK_WDATA_DLG) |
>                                                 BIT(RCA4_TYPE_MASK_DIR_DLG);

You can add a Reviewed-by: Cedric Blancher <[email protected]>.

Could you please backport this to the stable branches up to Linux
5.10, because conforming NFSv4.1 clients get robbed of all their
delegations if craa_objects_to_keep=0.

Ced
-- 
Cedric Blancher <[email protected]>
[https://plus.google.com/u/0/+CedricBlancher/]
Institute Pasteur