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 v6 11/14] nfsd: simplify use of the current stateid.
Date: Sat, 22 Nov 2025 11:47:09 +1100	[thread overview]
Message-ID: <20251122005236.3440177-12-neilb@ownmail.net> (raw)
In-Reply-To: <20251122005236.3440177-1-neilb@ownmail.net>

From: NeilBrown <neil@brown.name>

In NFSv4.1 any use of a stateid in an OP should check if it is
the special stateid that
   has "other" set to zero and "seqid" set to one

If it is, then the stateid saved from a previous operation should be
used unless that stateid as "special" stateid, in which case BAD_STATEID
must be returned.

Most ops that include a stateid pass it (indirectly) to
nfsd4_lookup_stateid().  The one exception is FREE_STATEID which uses
find_stateid_locked() directly.  (TEST_STATEID is not considered.  It is
given a list of stateids, among which any special stateid is explicitly
forbidden).

So rather than providing dedicated op_get_currentstateid functions for
each relevant op, we can include the required logic directly in
nfsd4_lookup_stateid() and nfsd4_free_stateid().  That is what this
patch does.

We also include an explicit return of nfserr_bad_stateid when the
special stateid is used, but no valid stateid is available.  This does
not change behaviour as the error would have been returned eventually.

This patch also removes the "minorversion" test in put_stateid() and
instead checks that sessions are in use when the stateid is used.  This
seems more natural.  Note that nfsd4_free_stateid() doesn't need this
explicit test as the function cannot be called for v4.0.

Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/nfsd/current_stateid.h | 21 ----------
 fs/nfsd/nfs4proc.c        | 11 -----
 fs/nfsd/nfs4state.c       | 85 ++++++---------------------------------
 fs/nfsd/xdr4.h            |  2 -
 4 files changed, 13 insertions(+), 106 deletions(-)

diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
index 24d769043207..eaa4b8e40dc8 100644
--- a/fs/nfsd/current_stateid.h
+++ b/fs/nfsd/current_stateid.h
@@ -16,25 +16,4 @@ extern void nfsd4_set_lockstateid(struct nfsd4_compound_state *,
 		union nfsd4_op_u *);
 extern void nfsd4_set_closestateid(struct nfsd4_compound_state *,
 		union nfsd4_op_u *);
-
-/*
- * functions to consume current state id
- */
-extern void nfsd4_get_opendowngradestateid(struct nfsd4_compound_state *,
-		union nfsd4_op_u *);
-extern void nfsd4_get_delegreturnstateid(struct nfsd4_compound_state *,
-		union nfsd4_op_u *);
-extern void nfsd4_get_freestateid(struct nfsd4_compound_state *,
-		union nfsd4_op_u *);
-extern void nfsd4_get_setattrstateid(struct nfsd4_compound_state *,
-		union nfsd4_op_u *);
-extern void nfsd4_get_closestateid(struct nfsd4_compound_state *,
-		union nfsd4_op_u *);
-extern void nfsd4_get_lockustateid(struct nfsd4_compound_state *,
-		union nfsd4_op_u *);
-extern void nfsd4_get_readstateid(struct nfsd4_compound_state *,
-		union nfsd4_op_u *);
-extern void nfsd4_get_writestateid(struct nfsd4_compound_state *,
-		union nfsd4_op_u *);
-
 #endif   /* _NFSD4_CURRENT_STATE_H */
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index ae9427ac7e5e..784f241c7119 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2943,8 +2943,6 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 		if (op->status)
 			goto encode_op;
 
-		if (op->opdesc->op_get_currentstateid)
-			op->opdesc->op_get_currentstateid(cstate, &op->u);
 		op->status = op->opdesc->op_func(rqstp, cstate, &op->u);
 		trace_nfsd_compound_op_err(rqstp, op->opnum, op->status);
 
@@ -3391,7 +3389,6 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 		.op_flags = OP_MODIFIES_SOMETHING,
 		.op_name = "OP_CLOSE",
 		.op_rsize_bop = nfsd4_status_stateid_rsize,
-		.op_get_currentstateid = nfsd4_get_closestateid,
 		.op_set_currentstateid = nfsd4_set_closestateid,
 	},
 	[OP_COMMIT] = {
@@ -3411,7 +3408,6 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 		.op_flags = OP_MODIFIES_SOMETHING,
 		.op_name = "OP_DELEGRETURN",
 		.op_rsize_bop = nfsd4_only_status_rsize,
-		.op_get_currentstateid = nfsd4_get_delegreturnstateid,
 	},
 	[OP_GETATTR] = {
 		.op_func = nfsd4_getattr,
@@ -3452,7 +3448,6 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 		.op_flags = OP_MODIFIES_SOMETHING,
 		.op_name = "OP_LOCKU",
 		.op_rsize_bop = nfsd4_status_stateid_rsize,
-		.op_get_currentstateid = nfsd4_get_lockustateid,
 	},
 	[OP_LOOKUP] = {
 		.op_func = nfsd4_lookup,
@@ -3489,7 +3484,6 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 		.op_flags = OP_MODIFIES_SOMETHING,
 		.op_name = "OP_OPEN_DOWNGRADE",
 		.op_rsize_bop = nfsd4_status_stateid_rsize,
-		.op_get_currentstateid = nfsd4_get_opendowngradestateid,
 		.op_set_currentstateid = nfsd4_set_opendowngradestateid,
 	},
 	[OP_PUTFH] = {
@@ -3518,7 +3512,6 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 		.op_release = nfsd4_read_release,
 		.op_name = "OP_READ",
 		.op_rsize_bop = nfsd4_read_rsize,
-		.op_get_currentstateid = nfsd4_get_readstateid,
 	},
 	[OP_READDIR] = {
 		.op_func = nfsd4_readdir,
@@ -3577,7 +3570,6 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME
 				| OP_NONTRIVIAL_ERROR_ENCODE,
 		.op_rsize_bop = nfsd4_setattr_rsize,
-		.op_get_currentstateid = nfsd4_get_setattrstateid,
 	},
 	[OP_SETCLIENTID] = {
 		.op_func = nfsd4_setclientid,
@@ -3604,7 +3596,6 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
 		.op_name = "OP_WRITE",
 		.op_rsize_bop = nfsd4_write_rsize,
-		.op_get_currentstateid = nfsd4_get_writestateid,
 	},
 	[OP_RELEASE_LOCKOWNER] = {
 		.op_func = nfsd4_release_lockowner,
@@ -3686,7 +3677,6 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 		.op_func = nfsd4_free_stateid,
 		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | OP_MODIFIES_SOMETHING,
 		.op_name = "OP_FREE_STATEID",
-		.op_get_currentstateid = nfsd4_get_freestateid,
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
 	[OP_GET_DIR_DELEGATION] = {
@@ -3754,7 +3744,6 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 		.op_release = nfsd4_read_release,
 		.op_name = "OP_READ_PLUS",
 		.op_rsize_bop = nfsd4_read_plus_rsize,
-		.op_get_currentstateid = nfsd4_get_readstateid,
 	},
 	[OP_SEEK] = {
 		.op_func = nfsd4_seek,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 03f29a6d7c14..de66b2971cd3 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7191,6 +7191,11 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 	struct nfs4_stid *stid;
 	bool return_revoked = false;
 
+	if (nfsd4_has_session(cstate) && is_current_stateid(stateid)) {
+		if (!cstate->current_fh.fh_have_stateid)
+			return nfserr_bad_stateid;
+		memcpy(stateid, &cstate->current_stateid, sizeof(stateid_t));
+	}
 	/*
 	 *  only return revoked delegations if explicitly asked.
 	 *  otherwise we report revoked or bad_stateid status.
@@ -7507,6 +7512,12 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfs4_client *cl = cstate->clp;
 	__be32 ret = nfserr_bad_stateid;
 
+	if (is_current_stateid(stateid)) {
+		if (!cstate->current_fh.fh_have_stateid)
+			return nfserr_bad_stateid;
+		memcpy(stateid, &cstate->current_stateid, sizeof(stateid_t));
+	}
+
 	spin_lock(&cl->cl_lock);
 	s = find_stateid_locked(cl, stateid);
 	if (!s || s->sc_status & SC_STATUS_CLOSED)
@@ -9102,21 +9113,11 @@ nfs4_state_shutdown(void)
 	shrinker_free(nfsd_slot_shrinker);
 }
 
-static void
-get_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
-{
-	if (cstate->current_fh.fh_have_stateid &&
-	    is_current_stateid(stateid))
-		memcpy(stateid, &cstate->current_stateid, sizeof(stateid_t));
-}
-
 static void
 put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
 {
-	if (cstate->minorversion) {
-		memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
-		cstate->current_fh.fh_have_stateid = true;
-	}
+	memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
+	cstate->current_fh.fh_have_stateid = true;
 }
 
 /*
@@ -9150,66 +9151,6 @@ nfsd4_set_lockstateid(struct nfsd4_compound_state *cstate,
 	put_stateid(cstate, &u->lock.lk_resp_stateid);
 }
 
-/*
- * functions to consume current state id
- */
-
-void
-nfsd4_get_opendowngradestateid(struct nfsd4_compound_state *cstate,
-		union nfsd4_op_u *u)
-{
-	get_stateid(cstate, &u->open_downgrade.od_stateid);
-}
-
-void
-nfsd4_get_delegreturnstateid(struct nfsd4_compound_state *cstate,
-		union nfsd4_op_u *u)
-{
-	get_stateid(cstate, &u->delegreturn.dr_stateid);
-}
-
-void
-nfsd4_get_freestateid(struct nfsd4_compound_state *cstate,
-		union nfsd4_op_u *u)
-{
-	get_stateid(cstate, &u->free_stateid.fr_stateid);
-}
-
-void
-nfsd4_get_setattrstateid(struct nfsd4_compound_state *cstate,
-		union nfsd4_op_u *u)
-{
-	get_stateid(cstate, &u->setattr.sa_stateid);
-}
-
-void
-nfsd4_get_closestateid(struct nfsd4_compound_state *cstate,
-		union nfsd4_op_u *u)
-{
-	get_stateid(cstate, &u->close.cl_stateid);
-}
-
-void
-nfsd4_get_lockustateid(struct nfsd4_compound_state *cstate,
-		union nfsd4_op_u *u)
-{
-	get_stateid(cstate, &u->locku.lu_stateid);
-}
-
-void
-nfsd4_get_readstateid(struct nfsd4_compound_state *cstate,
-		union nfsd4_op_u *u)
-{
-	get_stateid(cstate, &u->read.rd_stateid);
-}
-
-void
-nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
-		union nfsd4_op_u *u)
-{
-	get_stateid(cstate, &u->write.wr_stateid);
-}
-
 /**
  * nfsd4_vet_deleg_time - vet and set the timespec for a delegated timestamp update
  * @req: timestamp from the client
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 87986f1d5506..4f54108f24f9 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -1036,8 +1036,6 @@ struct nfsd4_operation {
 	/* Try to get response size before operation */
 	u32 (*op_rsize_bop)(const struct svc_rqst *rqstp,
 			const struct nfsd4_op *op);
-	void (*op_get_currentstateid)(struct nfsd4_compound_state *,
-			union nfsd4_op_u *);
 	void (*op_set_currentstateid)(struct nfsd4_compound_state *,
 			union nfsd4_op_u *);
 };
-- 
2.50.0.107.gf914562f5916.dirty


  parent reply	other threads:[~2025-11-22  0:53 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-22  0:46 [PATCH v7 00/14] nfsd: assorted cleanups involving v4 special stateids NeilBrown
2025-11-22  0:46 ` [PATCH v6 01/14] nfsd: rename ALLOWED_WITHOUT_FH to ALLOWED_WITHOUT_LOCAL_FH and revise use NeilBrown
2025-11-22 20:40   ` Chuck Lever
2025-11-22  0:47 ` [PATCH v6 02/14] nfsd: discard NFSD4_FH_FOREIGN NeilBrown
2025-11-22  0:47 ` [PATCH v6 03/14] nfsd: simplify foreign-filehandle handling to better match RFC-7862 NeilBrown
2025-11-22  0:47 ` [PATCH v6 04/14] nfsd: allow unrecognisable filehandle for foreign servers in COPY NeilBrown
2025-11-22 21:16   ` Chuck Lever
2025-11-23 16:43     ` Chuck Lever
2025-11-27  0:55       ` NeilBrown
2026-01-23 17:03         ` Chuck Lever
2025-11-22  0:47 ` [PATCH v6 05/14] nfsd: report correct error for attempt to use foreign filehandle NeilBrown
2025-11-22  0:47 ` [PATCH v6 06/14] nfsd: drop explicit tests for special stateids which would be invalid NeilBrown
2025-11-22  0:47 ` [PATCH v6 07/14] nfsd: revise names of special stateid, and predicate functions NeilBrown
2025-11-22  0:47 ` [PATCH v6 08/14] nfsd: pass parent_fh explicitly to nfsd4_process_open2() NeilBrown
2025-11-22  0:47 ` [PATCH v6 09/14] nfsd: revert nfsd4: delay setting current_fh in open NeilBrown
2025-11-22  0:47 ` [PATCH v6 10/14] nfsd: simplify clearing of current-state-id NeilBrown
2025-11-22  0:47 ` NeilBrown [this message]
2025-11-22  0:47 ` [PATCH v6 12/14] nfsd: simplify saving of the current stateid NeilBrown
2025-11-22  0:47 ` [PATCH v6 13/14] nfsd: discard current_stateid.h NeilBrown
2025-11-22  0:47 ` [PATCH v6 14/14] nfsd: conditionally clear seqid when current_stateid is used NeilBrown

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=20251122005236.3440177-12-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