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

[PATCH v2] 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.

A client fills in sa_highest_slotid from the slots in use when it
builds a request, so a request it sends afterward on a higher slot can
reach NFSD first. When the reply to that request is lost, the
acknowledging SEQUENCE retires and rebuilds the slot inside one
request, and the client's retransmit never sees NFS4ERR_BADSLOT. A
retransmit 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 later request 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>
---
Changes in v2:
- Explain the request in flight on the retired slot (per Neil's review)
- Rewrite the comment on the growth test (per Neil's review)
- Link to v1: https://lore.kernel.org/r/20261101154742.1950822-1-cel@kernel.org

 fs/nfsd/nfs4state.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index cafdf80f32de..8b7483278f50 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,15 @@ 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.
+	 *
+	 * Do not grow the table in the call that retired slots.  A
+	 * rebuilt slot has no cached reply, so a retransmit on it
+	 * is executed again or fails with SEQ_MISORDERED.  Left
+	 * retired, the slot returns BADSLOT, which the client
+	 * recovers from.
 	 */
-	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 v2] NFSD: Do not regrow a slot in the SEQUENCE that retired it

[Jeff Layton]

On Sun, 2026-11-01 at 20:10 -0500, 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.
> 
> A client fills in sa_highest_slotid from the slots in use when it
> builds a request, so a request it sends afterward on a higher slot can
> reach NFSD first. When the reply to that request is lost, the
> acknowledging SEQUENCE retires and rebuilds the slot inside one
> request, and the client's retransmit never sees NFS4ERR_BADSLOT. A
> retransmit 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 later request 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>
> ---
> Changes in v2:
> - Explain the request in flight on the retired slot (per Neil's review)
> - Rewrite the comment on the growth test (per Neil's review)
> - Link to v1: https://lore.kernel.org/r/20261101154742.1950822-1-cel@kernel.org
> 
>  fs/nfsd/nfs4state.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index cafdf80f32de..8b7483278f50 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,15 @@ 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.
> +	 *
> +	 * Do not grow the table in the call that retired slots.  A
> +	 * rebuilt slot has no cached reply, so a retransmit on it
> +	 * is executed again or fails with SEQ_MISORDERED.  Left
> +	 * retired, the slot returns BADSLOT, which the client
> +	 * recovers from.
>  	 */
> -	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,

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