[PATCH v1] NFSD: Do not regrow a slot in the SEQUENCE that retired it

[PATCH v1] NFSD: Do not regrow a slot in the SEQUENCE that retired it

[Chuck Lever]

When a SEQUENCE acknowledges a smaller slot table, free_session_slots()
retires the slots above the target. If that SEQUENCE is on the new top
slot, the growth test later in the same nfsd4_sequence() call finds the
table at its target and allocates back the slot the call just freed.

The slot is retired and rebuilt inside one request, so a client with a
request outstanding on it never sees NFS4ERR_BADSLOT. A copy of the
slot's first request passes the seqid 1 rule for a rebuilt slot and
executes a second time. A CREATE replayed this way returns
NFS4ERR_EXIST, reproduced with pynfs against a two-slot session and one
write to drop_caches. A retransmit of a request whose reply was lost
matches the retained seqid on the uninitialized rebuilt slot and gets
NFS4ERR_SEQ_MISORDERED, which the Linux client retries every 15 seconds
without limit.

Skip the growth block in a call that retired slots. A retransmit that
arrives while the slot is retired now gets NFS4ERR_BADSLOT, which the
Linux client handles as a session reset. The next request on the top
live slot still grows the table on demand, so a copy delayed past that
request meets a rebuilt slot as before.

Fixes: fc8738c68d0b ("nfsd: add support for freeing unused session-DRC slots")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfs4state.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index cafdf80f32de..1b8fe62cc756 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5230,6 +5230,7 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfs4_client *clp;
 	struct nfsd4_slot *slot;
 	struct nfsd4_conn *conn;
+	bool retired = false;
 	__be32 status;
 	int buflen;
 	u32 maxlen, respsize;
@@ -5301,9 +5302,11 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	    slot->sl_generation == session->se_slot_gen &&
 	    seq->maxslots <= session->se_target_maxslots &&
 	    seq->slotid < session->se_target_maxslots &&
-	    !nfsd4_slots_inuse(session, session->se_target_maxslots))
+	    !nfsd4_slots_inuse(session, session->se_target_maxslots)) {
 		/* Client acknowledged our reduce maxreqs */
 		free_session_slots(session, session->se_target_maxslots);
+		retired = true;
+	}
 
 	buflen = (seq->cachethis) ?
 			session->se_fchannel.maxresp_cached :
@@ -5360,8 +5363,14 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	 * configured maximum, not the running thread count, so a
 	 * client resuming from idle can grow back before the pool
 	 * scales up.
+	 *
+	 * A slot rebuilt in the call that retired it carries
+	 * no cached reply for its outstanding request, so a
+	 * retransmit of that request is executed again or fails
+	 * with SEQ_MISORDERED instead of BADSLOT.
 	 */
-	if (seq->slotid == session->se_fchannel.maxreqs - 1 &&
+	if (!retired &&
+	    seq->slotid == session->se_fchannel.maxreqs - 1 &&
 	    session->se_target_maxslots >= session->se_fchannel.maxreqs) {
 		int s = session->se_fchannel.maxreqs;
 		int ceiling = min_t(int, NFSD_MAX_SLOTS_PER_SESSION,
-- 
2.54.0

Re: [PATCH v1] NFSD: Do not regrow a slot in the SEQUENCE that retired it

[NeilBrown]

On Mon, 02 Nov 2026, Chuck Lever wrote:
> When a SEQUENCE acknowledges a smaller slot table, free_session_slots()
> retires the slots above the target. If that SEQUENCE is on the new top
> slot, the growth test later in the same nfsd4_sequence() call finds the
> table at its target and allocates back the slot the call just freed.
> 
> The slot is retired and rebuilt inside one request, so a client with a
> request outstanding on it never sees NFS4ERR_BADSLOT. A copy of the

How would a client have a slot outstanding if it has acknowledged the
smaller slot table?

I accept that shrinking and the growing the slot table in one op could
easily have edge cases and is best avoided, but I'd like to understand
the motivation of the patch.


> slot's first request passes the seqid 1 rule for a rebuilt slot and
> executes a second time. A CREATE replayed this way returns
> NFS4ERR_EXIST, reproduced with pynfs against a two-slot session and one
> write to drop_caches. A retransmit of a request whose reply was lost
> matches the retained seqid on the uninitialized rebuilt slot and gets
> NFS4ERR_SEQ_MISORDERED, which the Linux client retries every 15 seconds
> without limit.
> 
> Skip the growth block in a call that retired slots. A retransmit that
> arrives while the slot is retired now gets NFS4ERR_BADSLOT, which the
> Linux client handles as a session reset. The next request on the top
> live slot still grows the table on demand, so a copy delayed past that
> request meets a rebuilt slot as before.
> 
> Fixes: fc8738c68d0b ("nfsd: add support for freeing unused session-DRC slots")
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
>  fs/nfsd/nfs4state.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index cafdf80f32de..1b8fe62cc756 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5230,6 +5230,7 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	struct nfs4_client *clp;
>  	struct nfsd4_slot *slot;
>  	struct nfsd4_conn *conn;
> +	bool retired = false;
>  	__be32 status;
>  	int buflen;
>  	u32 maxlen, respsize;
> @@ -5301,9 +5302,11 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	    slot->sl_generation == session->se_slot_gen &&
>  	    seq->maxslots <= session->se_target_maxslots &&
>  	    seq->slotid < session->se_target_maxslots &&
> -	    !nfsd4_slots_inuse(session, session->se_target_maxslots))
> +	    !nfsd4_slots_inuse(session, session->se_target_maxslots)) {
>  		/* Client acknowledged our reduce maxreqs */
>  		free_session_slots(session, session->se_target_maxslots);
> +		retired = true;
> +	}
>  
>  	buflen = (seq->cachethis) ?
>  			session->se_fchannel.maxresp_cached :
> @@ -5360,8 +5363,14 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	 * configured maximum, not the running thread count, so a
>  	 * client resuming from idle can grow back before the pool
>  	 * scales up.
> +	 *
> +	 * A slot rebuilt in the call that retired it carries
> +	 * no cached reply for its outstanding request, so a
> +	 * retransmit of that request is executed again or fails
> +	 * with SEQ_MISORDERED instead of BADSLOT.

This comment could probably be improved.  As we don't (now) rebuild a
slot in the call that retires it, "A slot rebuilt in the call that
retires it" does not exist, so is not of interest...
Maybe:

   Don't grow the slot table in the same call that retired some slots.
   Doing that would mean the client never sees a confirmation
   that the server dropped all the slots, so ...... something that I
   don't understand yet.

I don't object to any of the code here - I'm just not clear on
motivation.

Thanks,
NeilBrown


>  	 */
> -	if (seq->slotid == session->se_fchannel.maxreqs - 1 &&
> +	if (!retired &&
> +	    seq->slotid == session->se_fchannel.maxreqs - 1 &&
>  	    session->se_target_maxslots >= session->se_fchannel.maxreqs) {
>  		int s = session->se_fchannel.maxreqs;
>  		int ceiling = min_t(int, NFSD_MAX_SLOTS_PER_SESSION,
> -- 
> 2.54.0
> 
> 

Re: [PATCH v1] NFSD: Do not regrow a slot in the SEQUENCE that retired it

[Chuck Lever]

On Mon, 02 Nov 2026, NeilBrown wrote:
> How would a client have a slot outstanding if it has acknowledged the
> smaller slot table?

It had none when it encoded the acknowledgement. The client fills in
sa_highest_slotid when it builds the request, from the slots in use
at that moment. The request on the higher slot is sent afterward, and
the two are in flight together. RFC 8881 Section 2.10.6.1 warns about
exactly this ordering: "the request may have been sent from the
requester before the update was received".

The trace I have is from a TLA+ model of the slot table, and pynfs
then reproduced it against a live NFSD. The target has been lowered
to 1 and an earlier request on slot 0 has stamped the new generation.
The client builds a request on slot 0 while slot 0 is its only slot
in use, so sa_highest_slotid is 0, then sends a request on slot 1
with the slot's current seqid. The nfsd threads handle the slot 1
request first and cache its reply. The connection drops and that
reply is lost. The slot 0 request then arrives. Slot 1 is no longer
INUSE, the generation matches, sa_highest_slotid is 0, so NFSD frees
slot 1. Slot 0 is now the top slot, so the same call allocates slot 1
again, with the retained seqid and NFSD4_SLOT_REUSED but without
NFSD4_SLOT_INITIALIZED or the cached reply.

The client reconnects and retransmits the slot 1 request with the
same seqid. check_slot_seqid() reports a replay, the slot is not
initialized, and NFSD returns SEQ_MISORDERED. The Linux client takes
the out_retry path in nfs41_sequence_process() and resends the same
seqid after NFS4_POLL_RETRY_MAX, with no limit. If the lost request
was the first on the slot, its seqid 1 passes the REUSED rule instead
and NFSD executes the compound a second time. pynfs shows both
outcomes with a two-slot session: a CREATE on slot 1 whose reply the
script discards, the table shrink, two requests on slot 0 with
sa_highest_slotid 0, then the CREATE sent again on slot 1. At seqid 1
the second CREATE returns NFS4ERR_EXIST. At seqid 2 every retransmit
gets SEQ_MISORDERED.

"Outstanding" in the description was my word for a request whose
reply the client has not seen. I'll fix that in v2.

With the growth block skipped, slot 1 stays retired until a later
request on the top live slot grows the table. The retransmit that
arrives before then gets BADSLOT, which the Linux client handles as a
session reset. That is not free, but it ends.

A copy delayed past that later request still meets a rebuilt slot, as
the last paragraph of the description admits. Closing that window
takes more than this patch, though.


> This comment could probably be improved.  As we don't (now) rebuild a
> slot in the call that retires it, "A slot rebuilt in the call that
> retires it" does not exist, so is not of interest...

Agreed. I’ll clean this up for v2.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)