[PATCH v2 0/4] NFSD: CB_RECALL_ANY fixes and a meaningful keep count

[PATCH v2 0/4] NFSD: CB_RECALL_ANY fixes and a meaningful keep count

[Chuck Lever]

NFSD sends every CB_RECALL_ANY with craa_objects_to_keep set to
zero, as RFC 8881 Section 20.6.3 does not mandate any particular
way for a client to choose which delegations to choose, if any.

However, a zero value can result in some clients giving back more
delegations than is necessary to relieve temporary memory pressure
on the server, which needlessly impacts performance.

NFSD intends the callback as a signal to return unused delegations.
Only the Linux client has been tested against it, and that client
ignores craa_objects_to_keep and returns only one unused delegation
of the named types, so the fixed zero never produced visible
misbehavior during our testing.

So, change NFSD so that each CB_RECALL_ANY asks a client to give up
one delegation. Both reaper callers (the shrinker to relieve memory
pressure, and the laundromat to cap the total number of delegations
the server tracks) re-arm while their condition lasts.

NFSD sets no recall target and remembers nothing across CB_RECALL_ANY
callbacks. RFC 8881 Section 20.6.4 prescribes the use of CB_RECALL
to target specific delegations if a client fails to return any. NFSD
does not take that step yet. CB_RECALL_ANY is asynchronous and
reports no completion, so NFSD treats it as advisory.

---
Changes in v2:
- Drop the gate that skipped clients holding a single delegation.
- Cover letter rewritten to give performance rationale.
- Link to v1: https://patch.msgid.link/20260811-recall-any-keep-count-v1-0-de9ca00493b7@kernel.org

---
Chuck Lever (4):
      NFSD: Do not send CB_RECALL_ANY to NFSv4.0 clients
      NFSD: Count the delegations held by each client
      NFSD: Name directory delegations in the CB_RECALL_ANY type mask
      NFSD: Send a meaningful CB_RECALL_ANY keep count

 fs/nfsd/nfs4state.c | 27 +++++++++++++++++++++++----
 fs/nfsd/state.h     |  2 ++
 2 files changed, 25 insertions(+), 4 deletions(-)
---
base-commit: 1d479c6b53f684b27da84ec352b7efb97f7f115f
change-id: 20260810-recall-any-keep-count-f50c2ae1b792

Best regards,
--  
Chuck Lever <cel@kernel.org>

[PATCH v2 1/4] NFSD: Do not send CB_RECALL_ANY to NFSv4.0 clients

[Chuck Lever]

deleg_reaper() sends CB_RECALL_ANY to every ACTIVE client holding
delegations, but CB_RECALL_ANY is an NFSv4.1 operation. An NFSv4.0
client's callback service accepts only CB_GETATTR and CB_RECALL, so it
replies OP_ILLEGAL. The decoder maps the unexpected opnum to -EIO, and
nfsd4_cb_done() marks the client's callback channel down.

Nothing brings the channel back. nfsd4_run_cb_work() sets NFSD4_CB_UP
only for a minor version above zero, and the only nfsd4_probe_callback()
call site an NFSv4.0 client reaches is nfsd4_setclientid_confirm(). One
visit from the reaper therefore leaves the channel marked down until the
client re-establishes its clientid. RENEW then returns
NFS4ERR_CB_PATH_DOWN for as long as the client holds delegations.
nfsd4_cb_channel_good() stops returning true, so the client is granted
no further delegations.

Skip clients at minor version zero.

Fixes: 44df6f439a17 ("NFSD: add delegation reaper to react to low memory condition")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfs4state.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 510380b6aa7a..09b1aa2914bc 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7947,6 +7947,8 @@ deleg_reaper(struct nfsd_net *nn)
 	list_for_each_safe(pos, next, &nn->client_lru) {
 		clp = list_entry(pos, struct nfs4_client, cl_lru);
 
+		if (clp->cl_minorversion == 0)
+			continue;
 		if (clp->cl_state != NFSD4_ACTIVE)
 			continue;
 		if (list_empty(&clp->cl_delegations))

-- 
2.54.0

[PATCH v2 2/4] NFSD: Count the delegations held by each client

[Chuck Lever]

struct nfs4_client records the delegations it holds on cl_delegations
but keeps no count of them. deleg_reaper() walks nn->client_lru under
nn->client_lock, but cl_delegations is serialized by nn->deleg_lock,
which nests outside nn->client_lock. A caller there cannot take
nn->deleg_lock to count the list. The cost tells against the walk as
well: an O(n) count per client, on a pass that already visits every
client.

Add cl_deleg_count, maintained at the two sites that mutate
cl_delegations. Both hold nn->deleg_lock, so the counter is already
serialized against itself and needs no atomic of its own. The decrement
sits below the delegation_hashed() test, next to the list_del_init it
pairs with, so it runs only when the delegation really leaves the list.

A reader that holds only nn->client_lock is not synchronized against
either update site, so it can see a count that does not match the
list. Such a reader marks the access with data_race() and may not
depend on the value for correctness.

No functional change.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfs4state.c | 2 ++
 fs/nfsd/state.h     | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 09b1aa2914bc..2ddc77ac7312 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1521,6 +1521,7 @@ hash_delegation_locked(struct nfs4_delegation *dp, struct nfs4_file *fp)
 	dp->dl_stid.sc_type = SC_TYPE_DELEG;
 	list_add(&dp->dl_perfile, &fp->fi_delegations);
 	list_add(&dp->dl_perclnt, &clp->cl_delegations);
+	clp->cl_deleg_count++;
 	return 0;
 }
 
@@ -1552,6 +1553,7 @@ unhash_delegation_locked(struct nfs4_delegation *dp, unsigned short statusmask)
 	++dp->dl_time;
 	spin_lock(&fp->fi_lock);
 	list_del_init(&dp->dl_perclnt);
+	dp->dl_stid.sc_client->cl_deleg_count--;
 	list_del_init(&dp->dl_recall_lru);
 	list_del_init(&dp->dl_perfile);
 	spin_unlock(&fp->fi_lock);
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index ff1c9fa731aa..10beeb851cf6 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -632,6 +632,8 @@ struct nfs4_client {
 
 	unsigned int		cl_state;
 	atomic_t		cl_delegs_in_recall;
+	/* Length of cl_delegations, updated under nn->deleg_lock */
+	unsigned int		cl_deleg_count;
 
 	struct nfsd4_cb_recall_any	*cl_ra;
 	time64_t		cl_ra_time;

-- 
2.54.0

[PATCH v2 3/4] NFSD: Name directory delegations in the CB_RECALL_ANY type mask

[Chuck Lever]

RFC 8881 Section 20.6.3 distinguishes an NFSv4.1 server
implementation that shares one pool among all classes of recallable
objects from one that keeps separate pools per class. NFSD falls
in the former category.

The CB_RECALL_ANY operation's craa_type_mask argument names the
types of objects in the recallable resource pool, but NFSD's
implementation does not name directory delegations, even though
they are allocated through __alloc_init_deleg(), they are counted
against the max_delegations budget, and the state shrinker reclaims
them.

Add RCA4_TYPE_MASK_DIR_DLG to craa_type_mask so clients that
implement directory delegations consider them when choosing which
delegations to return.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfs4state.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 2ddc77ac7312..3017a93261ff 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7969,7 +7969,8 @@ deleg_reaper(struct nfsd_net *nn)
 		clp->cl_ra_time = ktime_get_boottime_seconds();
 		clp->cl_ra->ra_keep = 0;
 		clp->cl_ra->ra_bmval[0] = BIT(RCA4_TYPE_MASK_RDATA_DLG) |
-						BIT(RCA4_TYPE_MASK_WDATA_DLG);
+						BIT(RCA4_TYPE_MASK_WDATA_DLG) |
+						BIT(RCA4_TYPE_MASK_DIR_DLG);
 		trace_nfsd_cb_recall_any(clp->cl_ra);
 		nfsd4_run_cb(&clp->cl_ra->ra_cb);
 	}

-- 
2.54.0

[PATCH v2 4/4] NFSD: Send a meaningful CB_RECALL_ANY keep count

[Chuck Lever]

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.

A client that returns those must keep an OPEN stateid anyway, so
NFSD trades a delegation for an open stateid and recovers nothing.
Plus there's 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 <cel@kernel.org>
---
 fs/nfsd/nfs4state.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 3017a93261ff..c8be38bc3c76 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7944,6 +7944,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) {
@@ -7953,21 +7954,34 @@ 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 one delegation at a time. A larger request
+		 * reaches delegations backing files that applications
+		 * still have open. Returning one of those trades a DELEG
+		 * stateid for an OPEN stateid and frees nothing.
+		 */
+		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);

-- 
2.54.0

Re: [PATCH v2 0/4] NFSD: CB_RECALL_ANY fixes and a meaningful keep count

[Jeff Layton]

On Wed, 2026-08-12 at 10:59 -0400, Chuck Lever wrote:
> NFSD sends every CB_RECALL_ANY with craa_objects_to_keep set to
> zero, as RFC 8881 Section 20.6.3 does not mandate any particular
> way for a client to choose which delegations to choose, if any.
> 
> However, a zero value can result in some clients giving back more
> delegations than is necessary to relieve temporary memory pressure
> on the server, which needlessly impacts performance.
> 
> NFSD intends the callback as a signal to return unused delegations.
> Only the Linux client has been tested against it, and that client
> ignores craa_objects_to_keep and returns only one unused delegation
> of the named types, so the fixed zero never produced visible
> misbehavior during our testing.
> 
> So, change NFSD so that each CB_RECALL_ANY asks a client to give up
> one delegation. Both reaper callers (the shrinker to relieve memory
> pressure, and the laundromat to cap the total number of delegations
> the server tracks) re-arm while their condition lasts.
> 
> NFSD sets no recall target and remembers nothing across CB_RECALL_ANY
> callbacks. RFC 8881 Section 20.6.4 prescribes the use of CB_RECALL
> to target specific delegations if a client fails to return any. NFSD
> does not take that step yet. CB_RECALL_ANY is asynchronous and
> reports no completion, so NFSD treats it as advisory.
> 
> ---
> Changes in v2:
> - Drop the gate that skipped clients holding a single delegation.
> - Cover letter rewritten to give performance rationale.
> - Link to v1: https://patch.msgid.link/20260811-recall-any-keep-count-v1-0-de9ca00493b7@kernel.org
> 
> ---
> Chuck Lever (4):
>       NFSD: Do not send CB_RECALL_ANY to NFSv4.0 clients
>       NFSD: Count the delegations held by each client
>       NFSD: Name directory delegations in the CB_RECALL_ANY type mask
>       NFSD: Send a meaningful CB_RECALL_ANY keep count
> 
>  fs/nfsd/nfs4state.c | 27 +++++++++++++++++++++++----
>  fs/nfsd/state.h     |  2 ++
>  2 files changed, 25 insertions(+), 4 deletions(-)
> ---
> base-commit: 1d479c6b53f684b27da84ec352b7efb97f7f115f
> change-id: 20260810-recall-any-keep-count-f50c2ae1b792
> 
> Best regards,
> --  
> Chuck Lever <cel@kernel.org>

Looks reasonable. Given that the Linux client ignores
craa_objects_to_keep and just sends back a single delegation, this
should keep things working the same even when it's brought into
compliance.

Reviewed-by: Jeff Layton <jlayton@kernel.org>