Linux NFS development
 help / color / mirror / Atom feed
From: NeilBrown <neilb@ownmail.net>
To: Chuck Lever <chuck.lever@oracle.com>, Jeff Layton <jlayton@kernel.org>
Cc: Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
	linux-nfs@vger.kernel.org
Subject: [PATCH v5 07/11] nfsd: simplify clearing of current-state-id
Date: Wed, 19 Nov 2025 14:28:53 +1100	[thread overview]
Message-ID: <20251119033204.360415-8-neilb@ownmail.net> (raw)
In-Reply-To: <20251119033204.360415-1-neilb@ownmail.net>

From: NeilBrown <neil@brown.name>

In NFSv4.1 the current and saved filehandle each have a corresponding
stateid.  RFC 8881 requires that in certain circumstances - particularly
when the current filehandle is changed - the current stateid should be
set to the anonymous stateid (all zeros).  It also requires that when a
request tries to use the current stateid, if that stateid is "special"
(which includes the anonymous stateid), then a BAD_STATEID error must
be produced.

Linux nfsd current implements these requirements using a flag to record
if the stateid (current or saved) is valid rather than actually storing
the anon stateid when invalid.  This has the same net effect.

The aim of this patch is to simplify this code and particularly to
remove the per-op OP_CLEAR_STATEID flag which is unnecessary.

The "is valid" flag is moved from 'struct nfsd4_compound_state' to
'struct svc_fh' which already has a number of flags related to the
filehandle.  This flag will only ever be set for the v4 current_fh and
saved_fh and records if the corresponding stateid is valid.

fh_put() is changed to clear this flag.  As this is called on current_fh
in each op that currently has OP_CLEAR_STATEID set, this automatically
clears the stored stateid when required so OP_CLEAR_STATEID is no longer
needed.

As fh_dup2() already copies all of the svc_fh, it now copies the new
fh_have_stateid flag as well, so savefh and restorefh are simplified.

Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/nfsd/current_stateid.h |  1 -
 fs/nfsd/nfs4proc.c        | 25 ++++++++-----------------
 fs/nfsd/nfs4state.c       | 10 ++--------
 fs/nfsd/nfsfh.h           |  1 +
 fs/nfsd/xdr4.h            | 13 -------------
 5 files changed, 11 insertions(+), 39 deletions(-)

diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
index c28540d86742..24d769043207 100644
--- a/fs/nfsd/current_stateid.h
+++ b/fs/nfsd/current_stateid.h
@@ -5,7 +5,6 @@
 #include "state.h"
 #include "xdr4.h"
 
-extern void clear_current_stateid(struct nfsd4_compound_state *cstate);
 /*
  * functions to set current state id
  */
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index e61b1ee6c8d8..ae4094ff12dc 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -737,10 +737,7 @@ nfsd4_restorefh(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		return nfserr_restorefh;
 
 	fh_dup2(&cstate->current_fh, &cstate->save_fh);
-	if (HAS_CSTATE_FLAG(cstate, SAVED_STATE_ID_FLAG)) {
-		memcpy(&cstate->current_stateid, &cstate->save_stateid, sizeof(stateid_t));
-		SET_CSTATE_FLAG(cstate, CURRENT_STATE_ID_FLAG);
-	}
+	memcpy(&cstate->current_stateid, &cstate->save_stateid, sizeof(stateid_t));
 	return nfs_ok;
 }
 
@@ -757,10 +754,7 @@ nfsd4_savefh(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		return nfserr_nofilehandle;
 
 	fh_dup2(&cstate->save_fh, &cstate->current_fh);
-	if (HAS_CSTATE_FLAG(cstate, CURRENT_STATE_ID_FLAG)) {
-		memcpy(&cstate->save_stateid, &cstate->current_stateid, sizeof(stateid_t));
-		SET_CSTATE_FLAG(cstate, SAVED_STATE_ID_FLAG);
-	}
+	memcpy(&cstate->save_stateid, &cstate->current_stateid, sizeof(stateid_t));
 	return nfs_ok;
 }
 
@@ -2954,9 +2948,6 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 			if (op->opdesc->op_set_currentstateid)
 				op->opdesc->op_set_currentstateid(cstate, &op->u);
 
-			if (op->opdesc->op_flags & OP_CLEAR_STATEID)
-				clear_current_stateid(cstate);
-
 			if (current_fh->fh_export &&
 					need_wrongsec_check(rqstp))
 				op->status = check_nfsd_access(current_fh->fh_export, rqstp, false);
@@ -3401,7 +3392,7 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_CREATE] = {
 		.op_func = nfsd4_create,
-		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME | OP_CLEAR_STATEID,
+		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
 		.op_name = "OP_CREATE",
 		.op_rsize_bop = nfsd4_create_rsize,
 	},
@@ -3455,13 +3446,13 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_LOOKUP] = {
 		.op_func = nfsd4_lookup,
-		.op_flags = OP_HANDLES_WRONGSEC | OP_CLEAR_STATEID,
+		.op_flags = OP_HANDLES_WRONGSEC,
 		.op_name = "OP_LOOKUP",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
 	[OP_LOOKUPP] = {
 		.op_func = nfsd4_lookupp,
-		.op_flags = OP_HANDLES_WRONGSEC | OP_CLEAR_STATEID,
+		.op_flags = OP_HANDLES_WRONGSEC,
 		.op_name = "OP_LOOKUPP",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
@@ -3494,21 +3485,21 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	[OP_PUTFH] = {
 		.op_func = nfsd4_putfh,
 		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_ON_ABSENT_FS
-				| OP_IS_PUTFH_LIKE | OP_CLEAR_STATEID,
+				| OP_IS_PUTFH_LIKE,
 		.op_name = "OP_PUTFH",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
 	[OP_PUTPUBFH] = {
 		.op_func = nfsd4_putrootfh,
 		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_ON_ABSENT_FS
-				| OP_IS_PUTFH_LIKE | OP_CLEAR_STATEID,
+				| OP_IS_PUTFH_LIKE,
 		.op_name = "OP_PUTPUBFH",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
 	[OP_PUTROOTFH] = {
 		.op_func = nfsd4_putrootfh,
 		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_ON_ABSENT_FS
-				| OP_IS_PUTFH_LIKE | OP_CLEAR_STATEID,
+				| OP_IS_PUTFH_LIKE,
 		.op_name = "OP_PUTROOTFH",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f92b01bdb4dd..7ffe0b8e44de 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -9106,7 +9106,7 @@ nfs4_state_shutdown(void)
 static void
 get_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
 {
-	if (HAS_CSTATE_FLAG(cstate, CURRENT_STATE_ID_FLAG) &&
+	if (cstate->current_fh.fh_have_stateid &&
 	    is_current_stateid(stateid))
 		memcpy(stateid, &cstate->current_stateid, sizeof(stateid_t));
 }
@@ -9116,16 +9116,10 @@ put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
 {
 	if (cstate->minorversion) {
 		memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
-		SET_CSTATE_FLAG(cstate, CURRENT_STATE_ID_FLAG);
+		cstate->current_fh.fh_have_stateid = true;
 	}
 }
 
-void
-clear_current_stateid(struct nfsd4_compound_state *cstate)
-{
-	CLEAR_CSTATE_FLAG(cstate, CURRENT_STATE_ID_FLAG);
-}
-
 /*
  * functions to set current state id
  */
diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index 43fcc1dcf69a..0142227c90e6 100644
--- a/fs/nfsd/nfsfh.h
+++ b/fs/nfsd/nfsfh.h
@@ -93,6 +93,7 @@ typedef struct svc_fh {
 						 */
 	bool			fh_use_wgather;	/* NFSv2 wgather option */
 	bool			fh_64bit_cookies;/* readdir cookie size */
+	bool			fh_have_stateid; /* associated v4.1 stateid is not special */
 	bool			fh_post_saved;	/* post-op attrs saved */
 	bool			fh_pre_saved;	/* pre-op attrs saved */
 
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index bcaf631ec12d..b6ab32c7b243 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -43,13 +43,6 @@
 #define NFSD4_MAX_TAGLEN	128
 #define XDR_LEN(n)                     (((n) + 3) & ~3)
 
-#define CURRENT_STATE_ID_FLAG (1<<0)
-#define SAVED_STATE_ID_FLAG (1<<1)
-
-#define SET_CSTATE_FLAG(c, f) ((c)->sid_flags |= (f))
-#define HAS_CSTATE_FLAG(c, f) ((c)->sid_flags & (f))
-#define CLEAR_CSTATE_FLAG(c, f) ((c)->sid_flags &= ~(f))
-
 /**
  * nfsd4_encode_bool - Encode an XDR bool type result
  * @xdr: target XDR stream
@@ -194,8 +187,6 @@ struct nfsd4_compound_state {
 	__be32			saved_status;
 	stateid_t	current_stateid;
 	stateid_t	save_stateid;
-	/* to indicate current and saved state id presents */
-	u32		sid_flags;
 };
 
 static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
@@ -1030,10 +1021,6 @@ enum nfsd4_op_flags {
 	 * the v4.0 case).
 	 */
 	OP_CACHEME = 1 << 6,
-	/*
-	 * These are ops which clear current state id.
-	 */
-	OP_CLEAR_STATEID = 1 << 7,
 	/* Most ops return only an error on failure; some may do more: */
 	OP_NONTRIVIAL_ERROR_ENCODE = 1 << 8,
 };
-- 
2.50.0.107.gf914562f5916.dirty


  parent reply	other threads:[~2025-11-19  3:32 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-19  3:28 [PATCH v5 00/11] nfsd: assorted cleanups involving v4 special stateids NeilBrown
2025-11-19  3:28 ` [PATCH v5 01/11] nfsd: rename ALLOWED_WITHOUT_FH to ALLOWED_WITHOUT_LOCAL_FH and revise use NeilBrown
2025-11-19 16:02   ` Chuck Lever
2025-11-19 21:13     ` NeilBrown
2025-11-19 19:12   ` Jeff Layton
2025-11-19  3:28 ` [PATCH v5 02/11] nfsd: discard NFSD4_FH_FOREIGN NeilBrown
2025-11-19 16:27   ` Chuck Lever
2025-11-19 21:25     ` NeilBrown
2025-11-19 19:13   ` Jeff Layton
2025-11-19  3:28 ` [PATCH v5 03/11] nfsd: simplify foreign-filehandle handling to better match RFC-7862 NeilBrown
2025-11-19 16:55   ` Chuck Lever
2025-11-19 21:38     ` NeilBrown
2025-11-20 21:58       ` Chuck Lever
2025-11-22  0:46         ` NeilBrown
2025-11-19 19:23   ` Jeff Layton
2025-11-19  3:28 ` [PATCH v5 04/11] nfsd: report correct error for attempt to use foreign filehandle NeilBrown
2025-11-19 19:26   ` Jeff Layton
2025-11-19  3:28 ` [PATCH v5 05/11] nfsd: drop explicit tests for special stateids which would be invalid NeilBrown
2025-11-19 19:11   ` Chuck Lever
2025-11-19 19:32   ` Jeff Layton
2025-11-19  3:28 ` [PATCH v5 06/11] nfsd: revise names of special stateid, and predicate functions NeilBrown
2025-11-19 19:27   ` Chuck Lever
2025-11-19 21:47     ` NeilBrown
2025-11-19  3:28 ` NeilBrown [this message]
2025-11-19 20:23   ` [PATCH v5 07/11] nfsd: simplify clearing of current-state-id Chuck Lever
2025-11-19 21:55     ` NeilBrown
2025-11-19  3:28 ` [PATCH v5 08/11] nfsd: simplify use of the current stateid NeilBrown
2025-11-19  3:28 ` [PATCH v5 09/11] nfsd: simplify saving " NeilBrown
2025-11-19  3:28 ` [PATCH v5 10/11] nfsd: discard current_stateid.h NeilBrown
2025-11-19  3:28 ` [PATCH v5 11/11] nfsd: conditionally clear seqid when current_stateid is used NeilBrown
2025-11-19 20:32 ` [PATCH v5 00/11] nfsd: assorted cleanups involving v4 special stateids Chuck Lever

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251119033204.360415-8-neilb@ownmail.net \
    --to=neilb@ownmail.net \
    --cc=Dai.Ngo@oracle.com \
    --cc=chuck.lever@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox