All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] miscellaneous nfsd4 state cleanup
@ 2021-01-19 22:35 J. Bruce Fields
  2021-01-19 22:35 ` [PATCH 1/8] nfsd4: simplify process_lookup1 J. Bruce Fields
                   ` (7 more replies)
  0 siblings, 8 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-19 22:35 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

This is some miscellaneous cleanup of NFSv4 state code that I ran into
while working on another project.

I think it's all pretty routine.  The only behavior difference should be
changing the error returned in some situations where there are multiple
reasons an operation could fail; none look like a problem to me.

--b.

J. Bruce Fields (8):
  nfsd4: simplify process_lookup1
  nfsd: simplify process_lock
  nfsd: simplify nfsd_renew
  nfsd: refactor lookup_clientid
  nfsd: find_cpntf_state cleanup
  nfsd: remove unused set_clientid argument
  nfsd: simplify nfsd4_check_open_reclaim
  nfsd: cstate->session->se_client -> cstate->clp

 fs/nfsd/nfs4proc.c  |   8 ++-
 fs/nfsd/nfs4state.c | 125 ++++++++++++++++++--------------------------
 fs/nfsd/state.h     |   3 +-
 3 files changed, 56 insertions(+), 80 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 1/8] nfsd4: simplify process_lookup1
  2021-01-19 22:35 [PATCH 0/8] miscellaneous nfsd4 state cleanup J. Bruce Fields
@ 2021-01-19 22:35 ` J. Bruce Fields
  2021-01-19 22:35 ` [PATCH 2/8] nfsd: simplify process_lock J. Bruce Fields
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-19 22:35 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

This STALE_CLIENTID check is redundant with the one in
lookup_clientid().

There's a difference in behavior is in case of memory allocation
failure, which I think isn't a big deal.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1d2cd6a88f61..f9f89229dba6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4680,8 +4680,6 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
 	struct nfs4_openowner *oo = NULL;
 	__be32 status;
 
-	if (STALE_CLIENTID(&open->op_clientid, nn))
-		return nfserr_stale_clientid;
 	/*
 	 * In case we need it later, after we've already created the
 	 * file and don't want to risk a further failure:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 2/8] nfsd: simplify process_lock
  2021-01-19 22:35 [PATCH 0/8] miscellaneous nfsd4 state cleanup J. Bruce Fields
  2021-01-19 22:35 ` [PATCH 1/8] nfsd4: simplify process_lookup1 J. Bruce Fields
@ 2021-01-19 22:35 ` J. Bruce Fields
  2021-01-20 21:01   ` Chuck Lever
  2021-01-19 22:35 ` [PATCH 3/8] nfsd: simplify nfsd_renew J. Bruce Fields
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-19 22:35 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

Similarly, this STALE_CLIENTID check is already handled inside
preprocess_confirmed_seqid_op().

(This may cause it to return a different error in some cases where
there are multiple things wrong; pynfs test SEQ10 regressed on this
commit because of that, but I think that's the test's fault, and I've
fixed it separately.)

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f9f89229dba6..7ea63d7cec4d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6697,10 +6697,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 				&cstate->session->se_client->cl_clientid,
 				sizeof(clientid_t));
 
-		status = nfserr_stale_clientid;
-		if (STALE_CLIENTID(&lock->lk_new_clientid, nn))
-			goto out;
-
 		/* validate and update open stateid and open seqid */
 		status = nfs4_preprocess_confirmed_seqid_op(cstate,
 				        lock->lk_new_open_seqid,
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 3/8] nfsd: simplify nfsd_renew
  2021-01-19 22:35 [PATCH 0/8] miscellaneous nfsd4 state cleanup J. Bruce Fields
  2021-01-19 22:35 ` [PATCH 1/8] nfsd4: simplify process_lookup1 J. Bruce Fields
  2021-01-19 22:35 ` [PATCH 2/8] nfsd: simplify process_lock J. Bruce Fields
@ 2021-01-19 22:35 ` J. Bruce Fields
  2021-01-19 22:35 ` [PATCH 4/8] nfsd: refactor lookup_clientid J. Bruce Fields
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-19 22:35 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

You can take the single-exit thing too far, I think.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7ea63d7cec4d..ba955bbf21df 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5300,15 +5300,12 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	trace_nfsd_clid_renew(clid);
 	status = lookup_clientid(clid, cstate, nn, false);
 	if (status)
-		goto out;
+		return status;
 	clp = cstate->clp;
-	status = nfserr_cb_path_down;
 	if (!list_empty(&clp->cl_delegations)
 			&& clp->cl_cb_state != NFSD4_CB_UP)
-		goto out;
-	status = nfs_ok;
-out:
-	return status;
+		return nfserr_cb_path_down;
+	return nfs_ok;
 }
 
 void
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 4/8] nfsd: refactor lookup_clientid
  2021-01-19 22:35 [PATCH 0/8] miscellaneous nfsd4 state cleanup J. Bruce Fields
                   ` (2 preceding siblings ...)
  2021-01-19 22:35 ` [PATCH 3/8] nfsd: simplify nfsd_renew J. Bruce Fields
@ 2021-01-19 22:35 ` J. Bruce Fields
  2021-01-20 21:02   ` Chuck Lever
  2021-01-19 22:35 ` [PATCH 5/8] nfsd: find_cpntf_state cleanup J. Bruce Fields
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-19 22:35 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

I think set_client is a better name, and the lookup itself I want to
use somewhere else.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 50 ++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ba955bbf21df..d7128e16c86e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4633,40 +4633,40 @@ static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4
 	return nfserr_bad_seqid;
 }
 
-static __be32 lookup_clientid(clientid_t *clid,
+static struct nfs4_client *lookup_clientid(clientid_t *clid, bool sessions,
+						struct nfsd_net *nn)
+{
+	struct nfs4_client *found;
+
+	spin_lock(&nn->client_lock);
+	found = find_confirmed_client(clid, sessions, nn);
+	if (found)
+		atomic_inc(&found->cl_rpc_users);
+	spin_unlock(&nn->client_lock);
+	return found;
+}
+
+static __be32 set_client(clientid_t *clid,
 		struct nfsd4_compound_state *cstate,
 		struct nfsd_net *nn,
 		bool sessions)
 {
-	struct nfs4_client *found;
-
 	if (cstate->clp) {
-		found = cstate->clp;
-		if (!same_clid(&found->cl_clientid, clid))
+		if (!same_clid(&cstate->clp->cl_clientid, clid))
 			return nfserr_stale_clientid;
 		return nfs_ok;
 	}
-
 	if (STALE_CLIENTID(clid, nn))
 		return nfserr_stale_clientid;
-
 	/*
 	 * For v4.1+ we get the client in the SEQUENCE op. If we don't have one
 	 * cached already then we know this is for is for v4.0 and "sessions"
 	 * will be false.
 	 */
 	WARN_ON_ONCE(cstate->session);
-	spin_lock(&nn->client_lock);
-	found = find_confirmed_client(clid, sessions, nn);
-	if (!found) {
-		spin_unlock(&nn->client_lock);
+	cstate->clp = lookup_clientid(clid, sessions, nn);
+	if (!cstate->clp)
 		return nfserr_expired;
-	}
-	atomic_inc(&found->cl_rpc_users);
-	spin_unlock(&nn->client_lock);
-
-	/* Cache the nfs4_client in cstate! */
-	cstate->clp = found;
 	return nfs_ok;
 }
 
@@ -4688,7 +4688,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
 	if (open->op_file == NULL)
 		return nfserr_jukebox;
 
-	status = lookup_clientid(clientid, cstate, nn, false);
+	status = set_client(clientid, cstate, nn, false);
 	if (status)
 		return status;
 	clp = cstate->clp;
@@ -5298,7 +5298,7 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 
 	trace_nfsd_clid_renew(clid);
-	status = lookup_clientid(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn, false);
 	if (status)
 		return status;
 	clp = cstate->clp;
@@ -5681,8 +5681,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
 		CLOSE_STATEID(stateid))
 		return nfserr_bad_stateid;
-	status = lookup_clientid(&stateid->si_opaque.so_clid, cstate, nn,
-				 false);
+	status = set_client(&stateid->si_opaque.so_clid, cstate, nn, false);
 	if (status == nfserr_stale_clientid) {
 		if (cstate->session)
 			return nfserr_bad_stateid;
@@ -5821,7 +5820,7 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 
 	cps->cpntf_time = ktime_get_boottime_seconds();
 	memset(&cstate, 0, sizeof(cstate));
-	status = lookup_clientid(&cps->cp_p_clid, &cstate, nn, true);
+	status = set_clientid(&cps->cp_p_clid, &cstate, nn, true);
 	if (status)
 		goto out;
 	status = nfsd4_lookup_stateid(&cstate, &cps->cp_p_stateid,
@@ -6900,8 +6899,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		 return nfserr_inval;
 
 	if (!nfsd4_has_session(cstate)) {
-		status = lookup_clientid(&lockt->lt_clientid, cstate, nn,
-					 false);
+		status = set_client(&lockt->lt_clientid, cstate, nn, false);
 		if (status)
 			goto out;
 	}
@@ -7085,7 +7083,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
 	dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n",
 		clid->cl_boot, clid->cl_id);
 
-	status = lookup_clientid(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn, false);
 	if (status)
 		return status;
 
@@ -7232,7 +7230,7 @@ nfs4_check_open_reclaim(clientid_t *clid,
 	__be32 status;
 
 	/* find clientid in conf_id_hashtbl */
-	status = lookup_clientid(clid, cstate, nn, false);
+	status = set_clientid(clid, cstate, nn, false);
 	if (status)
 		return nfserr_reclaim_bad;
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 5/8] nfsd: find_cpntf_state cleanup
  2021-01-19 22:35 [PATCH 0/8] miscellaneous nfsd4 state cleanup J. Bruce Fields
                   ` (3 preceding siblings ...)
  2021-01-19 22:35 ` [PATCH 4/8] nfsd: refactor lookup_clientid J. Bruce Fields
@ 2021-01-19 22:35 ` J. Bruce Fields
  2021-01-19 22:35 ` [PATCH 6/8] nfsd: remove unused set_clientid argument J. Bruce Fields
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-19 22:35 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

I think this unusual use of struct compound_state could cause confusion.

It's not that much more complicated just to open-code this stateid
lookup.

The only change in behavior should be a different error return in the
case the copy is using a source stateid that is a revoked delegation,
but I doubt that matters.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d7128e16c86e..01191d2dddc8 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5812,21 +5812,27 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 {
 	__be32 status;
 	struct nfs4_cpntf_state *cps = NULL;
-	struct nfsd4_compound_state cstate;
+	struct nfs4_client *found;
 
 	status = manage_cpntf_state(nn, st, NULL, &cps);
 	if (status)
 		return status;
 
 	cps->cpntf_time = ktime_get_boottime_seconds();
-	memset(&cstate, 0, sizeof(cstate));
-	status = set_clientid(&cps->cp_p_clid, &cstate, nn, true);
-	if (status)
+
+	status = nfserr_expired;
+	found = lookup_clientid(&cps->cp_p_clid, true, nn);
+	if (!found)
 		goto out;
-	status = nfsd4_lookup_stateid(&cstate, &cps->cp_p_stateid,
-				NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID,
-				stid, nn);
-	put_client_renew(cstate.clp);
+
+	*stid = find_stateid_by_type(found, &cps->cp_p_stateid,
+			NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID);
+	if (stid)
+		status = nfs_ok;
+	else
+		status = nfserr_bad_stateid;
+
+	put_client_renew(found);
 out:
 	nfs4_put_cpntf_state(nn, cps);
 	return status;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 6/8] nfsd: remove unused set_clientid argument
  2021-01-19 22:35 [PATCH 0/8] miscellaneous nfsd4 state cleanup J. Bruce Fields
                   ` (4 preceding siblings ...)
  2021-01-19 22:35 ` [PATCH 5/8] nfsd: find_cpntf_state cleanup J. Bruce Fields
@ 2021-01-19 22:35 ` J. Bruce Fields
  2021-01-19 22:35 ` [PATCH 7/8] nfsd: simplify nfsd4_check_open_reclaim J. Bruce Fields
  2021-01-19 22:35 ` [PATCH 8/8] nfsd: cstate->session->se_client -> cstate->clp J. Bruce Fields
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-19 22:35 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

Every caller is setting this argument to false, so we don't need it.

Also clarify comments a little.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 01191d2dddc8..2c580ef6e337 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4648,8 +4648,7 @@ static struct nfs4_client *lookup_clientid(clientid_t *clid, bool sessions,
 
 static __be32 set_client(clientid_t *clid,
 		struct nfsd4_compound_state *cstate,
-		struct nfsd_net *nn,
-		bool sessions)
+		struct nfsd_net *nn)
 {
 	if (cstate->clp) {
 		if (!same_clid(&cstate->clp->cl_clientid, clid))
@@ -4658,13 +4657,10 @@ static __be32 set_client(clientid_t *clid,
 	}
 	if (STALE_CLIENTID(clid, nn))
 		return nfserr_stale_clientid;
-	/*
-	 * For v4.1+ we get the client in the SEQUENCE op. If we don't have one
-	 * cached already then we know this is for is for v4.0 and "sessions"
-	 * will be false.
-	 */
+	/* For v4.1+ we should have gotten the client in the SEQUENCE op: */
 	WARN_ON_ONCE(cstate->session);
-	cstate->clp = lookup_clientid(clid, sessions, nn);
+	/* So we're looking for a 4.0 client (sessions = false): */
+	cstate->clp = lookup_clientid(clid, false, nn);
 	if (!cstate->clp)
 		return nfserr_expired;
 	return nfs_ok;
@@ -4688,7 +4684,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
 	if (open->op_file == NULL)
 		return nfserr_jukebox;
 
-	status = set_client(clientid, cstate, nn, false);
+	status = set_client(clientid, cstate, nn);
 	if (status)
 		return status;
 	clp = cstate->clp;
@@ -5298,7 +5294,7 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 
 	trace_nfsd_clid_renew(clid);
-	status = set_client(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn);
 	if (status)
 		return status;
 	clp = cstate->clp;
@@ -5681,7 +5677,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
 		CLOSE_STATEID(stateid))
 		return nfserr_bad_stateid;
-	status = set_client(&stateid->si_opaque.so_clid, cstate, nn, false);
+	status = set_client(&stateid->si_opaque.so_clid, cstate, nn);
 	if (status == nfserr_stale_clientid) {
 		if (cstate->session)
 			return nfserr_bad_stateid;
@@ -6905,7 +6901,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		 return nfserr_inval;
 
 	if (!nfsd4_has_session(cstate)) {
-		status = set_client(&lockt->lt_clientid, cstate, nn, false);
+		status = set_client(&lockt->lt_clientid, cstate, nn);
 		if (status)
 			goto out;
 	}
@@ -7089,7 +7085,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
 	dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n",
 		clid->cl_boot, clid->cl_id);
 
-	status = set_client(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn);
 	if (status)
 		return status;
 
@@ -7236,7 +7232,7 @@ nfs4_check_open_reclaim(clientid_t *clid,
 	__be32 status;
 
 	/* find clientid in conf_id_hashtbl */
-	status = set_clientid(clid, cstate, nn, false);
+	status = set_clientid(clid, cstate, nn);
 	if (status)
 		return nfserr_reclaim_bad;
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 7/8] nfsd: simplify nfsd4_check_open_reclaim
  2021-01-19 22:35 [PATCH 0/8] miscellaneous nfsd4 state cleanup J. Bruce Fields
                   ` (5 preceding siblings ...)
  2021-01-19 22:35 ` [PATCH 6/8] nfsd: remove unused set_clientid argument J. Bruce Fields
@ 2021-01-19 22:35 ` J. Bruce Fields
  2021-01-19 22:35 ` [PATCH 8/8] nfsd: cstate->session->se_client -> cstate->clp J. Bruce Fields
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-19 22:35 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

The set_client() was already taken care of by process_open1().

The comments here are mostly redundant with the code.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c  |  3 +--
 fs/nfsd/nfs4state.c | 18 +++---------------
 fs/nfsd/state.h     |  3 +--
 3 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4727b7f03c5b..567af1f10d2c 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -423,8 +423,7 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 				goto out;
 			break;
 		case NFS4_OPEN_CLAIM_PREVIOUS:
-			status = nfs4_check_open_reclaim(&open->op_clientid,
-							 cstate, nn);
+			status = nfs4_check_open_reclaim(cstate->clp);
 			if (status)
 				goto out;
 			open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 2c580ef6e337..860805ffde1a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7221,25 +7221,13 @@ nfsd4_find_reclaim_client(struct xdr_netobj name, struct nfsd_net *nn)
 	return NULL;
 }
 
-/*
-* Called from OPEN. Look for clientid in reclaim list.
-*/
 __be32
-nfs4_check_open_reclaim(clientid_t *clid,
-		struct nfsd4_compound_state *cstate,
-		struct nfsd_net *nn)
+nfs4_check_open_reclaim(struct nfs4_client *clp)
 {
-	__be32 status;
-
-	/* find clientid in conf_id_hashtbl */
-	status = set_clientid(clid, cstate, nn);
-	if (status)
-		return nfserr_reclaim_bad;
-
-	if (test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &cstate->clp->cl_flags))
+	if (test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &clp->cl_flags))
 		return nfserr_no_grace;
 
-	if (nfsd4_client_record_check(cstate->clp))
+	if (nfsd4_client_record_check(clp))
 		return nfserr_reclaim_bad;
 
 	return nfs_ok;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 9eae11a9d21c..73deea353169 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -649,8 +649,7 @@ void nfs4_remove_reclaim_record(struct nfs4_client_reclaim *, struct nfsd_net *)
 extern void nfs4_release_reclaim(struct nfsd_net *);
 extern struct nfs4_client_reclaim *nfsd4_find_reclaim_client(struct xdr_netobj name,
 							struct nfsd_net *nn);
-extern __be32 nfs4_check_open_reclaim(clientid_t *clid,
-		struct nfsd4_compound_state *cstate, struct nfsd_net *nn);
+extern __be32 nfs4_check_open_reclaim(struct nfs4_client *);
 extern void nfsd4_probe_callback(struct nfs4_client *clp);
 extern void nfsd4_probe_callback_sync(struct nfs4_client *clp);
 extern void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 8/8] nfsd: cstate->session->se_client -> cstate->clp
  2021-01-19 22:35 [PATCH 0/8] miscellaneous nfsd4 state cleanup J. Bruce Fields
                   ` (6 preceding siblings ...)
  2021-01-19 22:35 ` [PATCH 7/8] nfsd: simplify nfsd4_check_open_reclaim J. Bruce Fields
@ 2021-01-19 22:35 ` J. Bruce Fields
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-19 22:35 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

I'm not sure why we're writing this out the hard way in so many places.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c  |  5 ++---
 fs/nfsd/nfs4state.c | 16 ++++++++--------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 567af1f10d2c..f63a12a5278a 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -373,8 +373,7 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	 * Before RECLAIM_COMPLETE done, server should deny new lock
 	 */
 	if (nfsd4_has_session(cstate) &&
-	    !test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE,
-		      &cstate->session->se_client->cl_flags) &&
+	    !test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &cstate->clp->cl_flags) &&
 	    open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
 		return nfserr_grace;
 
@@ -1882,7 +1881,7 @@ nfsd4_getdeviceinfo(struct svc_rqst *rqstp,
 	nfserr = nfs_ok;
 	if (gdp->gd_maxcount != 0) {
 		nfserr = ops->proc_getdeviceinfo(exp->ex_path.mnt->mnt_sb,
-				rqstp, cstate->session->se_client, gdp);
+				rqstp, cstate->clp, gdp);
 	}
 
 	gdp->gd_notify_types &= ops->notify_types;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 860805ffde1a..7b865ed7c9d7 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3891,6 +3891,7 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *cstate, union nfsd4_op_u *u)
 {
 	struct nfsd4_reclaim_complete *rc = &u->reclaim_complete;
+	struct nfs4_client *clp = cstate->clp;
 	__be32 status = 0;
 
 	if (rc->rca_one_fs) {
@@ -3904,12 +3905,11 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp,
 	}
 
 	status = nfserr_complete_already;
-	if (test_and_set_bit(NFSD4_CLIENT_RECLAIM_COMPLETE,
-			     &cstate->session->se_client->cl_flags))
+	if (test_and_set_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &clp->cl_flags))
 		goto out;
 
 	status = nfserr_stale_clientid;
-	if (is_client_expired(cstate->session->se_client))
+	if (is_client_expired(clp))
 		/*
 		 * The following error isn't really legal.
 		 * But we only get here if the client just explicitly
@@ -3920,8 +3920,8 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp,
 		goto out;
 
 	status = nfs_ok;
-	nfsd4_client_record_create(cstate->session->se_client);
-	inc_reclaim_complete(cstate->session->se_client);
+	nfsd4_client_record_create(clp);
+	inc_reclaim_complete(clp);
 out:
 	return status;
 }
@@ -5917,7 +5917,7 @@ nfsd4_test_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 {
 	struct nfsd4_test_stateid *test_stateid = &u->test_stateid;
 	struct nfsd4_test_stateid_id *stateid;
-	struct nfs4_client *cl = cstate->session->se_client;
+	struct nfs4_client *cl = cstate->clp;
 
 	list_for_each_entry(stateid, &test_stateid->ts_stateid_list, ts_id_list)
 		stateid->ts_id_status =
@@ -5963,7 +5963,7 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	stateid_t *stateid = &free_stateid->fr_stateid;
 	struct nfs4_stid *s;
 	struct nfs4_delegation *dp;
-	struct nfs4_client *cl = cstate->session->se_client;
+	struct nfs4_client *cl = cstate->clp;
 	__be32 ret = nfserr_bad_stateid;
 
 	spin_lock(&cl->cl_lock);
@@ -6692,7 +6692,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		if (nfsd4_has_session(cstate))
 			/* See rfc 5661 18.10.3: given clientid is ignored: */
 			memcpy(&lock->lk_new_clientid,
-				&cstate->session->se_client->cl_clientid,
+				&cstate->clp->cl_clientid,
 				sizeof(clientid_t));
 
 		/* validate and update open stateid and open seqid */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH 2/8] nfsd: simplify process_lock
  2021-01-19 22:35 ` [PATCH 2/8] nfsd: simplify process_lock J. Bruce Fields
@ 2021-01-20 21:01   ` Chuck Lever
  2021-01-20 21:25     ` J. Bruce Fields
  0 siblings, 1 reply; 35+ messages in thread
From: Chuck Lever @ 2021-01-20 21:01 UTC (permalink / raw)
  To: Bruce Fields; +Cc: Linux NFS Mailing List



> On Jan 19, 2021, at 5:35 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> 
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> Similarly, this STALE_CLIENTID check is already handled inside
> preprocess_confirmed_seqid_op().

I can't confirm this claim. Where is clid->cl_boot checked?

Did you mean nfs4_preprocess_confirmed_seqid_op() here?


> (This may cause it to return a different error in some cases where
> there are multiple things wrong; pynfs test SEQ10 regressed on this
> commit because of that, but I think that's the test's fault, and I've
> fixed it separately.)
> 
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
> fs/nfsd/nfs4state.c | 4 ----
> 1 file changed, 4 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index f9f89229dba6..7ea63d7cec4d 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -6697,10 +6697,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 				&cstate->session->se_client->cl_clientid,
> 				sizeof(clientid_t));
> 
> -		status = nfserr_stale_clientid;
> -		if (STALE_CLIENTID(&lock->lk_new_clientid, nn))
> -			goto out;
> -
> 		/* validate and update open stateid and open seqid */
> 		status = nfs4_preprocess_confirmed_seqid_op(cstate,
> 				        lock->lk_new_open_seqid,
> -- 
> 2.29.2
> 

--
Chuck Lever




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 4/8] nfsd: refactor lookup_clientid
  2021-01-19 22:35 ` [PATCH 4/8] nfsd: refactor lookup_clientid J. Bruce Fields
@ 2021-01-20 21:02   ` Chuck Lever
  2021-01-20 23:01     ` J. Bruce Fields
  0 siblings, 1 reply; 35+ messages in thread
From: Chuck Lever @ 2021-01-20 21:02 UTC (permalink / raw)
  To: Bruce Fields; +Cc: Linux NFS Mailing List



> On Jan 19, 2021, at 5:35 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> 
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> I think set_client is a better name, and the lookup itself I want to
> use somewhere else.

Should this be two patches?
- Rename lookup_clientid() to set_client()
- Refactor the lookup_clientid() helper

nfs4state.c stops compiling with this patch. See below.


> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
> fs/nfsd/nfs4state.c | 50 ++++++++++++++++++++++-----------------------
> 1 file changed, 24 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index ba955bbf21df..d7128e16c86e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4633,40 +4633,40 @@ static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4
> 	return nfserr_bad_seqid;
> }
> 
> -static __be32 lookup_clientid(clientid_t *clid,
> +static struct nfs4_client *lookup_clientid(clientid_t *clid, bool sessions,
> +						struct nfsd_net *nn)
> +{
> +	struct nfs4_client *found;
> +
> +	spin_lock(&nn->client_lock);
> +	found = find_confirmed_client(clid, sessions, nn);
> +	if (found)
> +		atomic_inc(&found->cl_rpc_users);
> +	spin_unlock(&nn->client_lock);
> +	return found;
> +}
> +
> +static __be32 set_client(clientid_t *clid,
> 		struct nfsd4_compound_state *cstate,
> 		struct nfsd_net *nn,
> 		bool sessions)
> {
> -	struct nfs4_client *found;
> -
> 	if (cstate->clp) {
> -		found = cstate->clp;
> -		if (!same_clid(&found->cl_clientid, clid))
> +		if (!same_clid(&cstate->clp->cl_clientid, clid))
> 			return nfserr_stale_clientid;
> 		return nfs_ok;
> 	}
> -
> 	if (STALE_CLIENTID(clid, nn))
> 		return nfserr_stale_clientid;
> -
> 	/*
> 	 * For v4.1+ we get the client in the SEQUENCE op. If we don't have one
> 	 * cached already then we know this is for is for v4.0 and "sessions"
> 	 * will be false.
> 	 */
> 	WARN_ON_ONCE(cstate->session);
> -	spin_lock(&nn->client_lock);
> -	found = find_confirmed_client(clid, sessions, nn);
> -	if (!found) {
> -		spin_unlock(&nn->client_lock);
> +	cstate->clp = lookup_clientid(clid, sessions, nn);
> +	if (!cstate->clp)
> 		return nfserr_expired;
> -	}
> -	atomic_inc(&found->cl_rpc_users);
> -	spin_unlock(&nn->client_lock);
> -
> -	/* Cache the nfs4_client in cstate! */
> -	cstate->clp = found;
> 	return nfs_ok;
> }
> 
> @@ -4688,7 +4688,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
> 	if (open->op_file == NULL)
> 		return nfserr_jukebox;
> 
> -	status = lookup_clientid(clientid, cstate, nn, false);
> +	status = set_client(clientid, cstate, nn, false);
> 	if (status)
> 		return status;
> 	clp = cstate->clp;
> @@ -5298,7 +5298,7 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> 
> 	trace_nfsd_clid_renew(clid);
> -	status = lookup_clientid(clid, cstate, nn, false);
> +	status = set_client(clid, cstate, nn, false);
> 	if (status)
> 		return status;
> 	clp = cstate->clp;
> @@ -5681,8 +5681,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> 	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
> 		CLOSE_STATEID(stateid))
> 		return nfserr_bad_stateid;
> -	status = lookup_clientid(&stateid->si_opaque.so_clid, cstate, nn,
> -				 false);
> +	status = set_client(&stateid->si_opaque.so_clid, cstate, nn, false);
> 	if (status == nfserr_stale_clientid) {
> 		if (cstate->session)
> 			return nfserr_bad_stateid;
> @@ -5821,7 +5820,7 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
> 
> 	cps->cpntf_time = ktime_get_boottime_seconds();
> 	memset(&cstate, 0, sizeof(cstate));
> -	status = lookup_clientid(&cps->cp_p_clid, &cstate, nn, true);
> +	status = set_clientid(&cps->cp_p_clid, &cstate, nn, true);

This doesn't compile. I think you meant set_client() here.


> 	if (status)
> 		goto out;
> 	status = nfsd4_lookup_stateid(&cstate, &cps->cp_p_stateid,
> @@ -6900,8 +6899,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 		 return nfserr_inval;
> 
> 	if (!nfsd4_has_session(cstate)) {
> -		status = lookup_clientid(&lockt->lt_clientid, cstate, nn,
> -					 false);
> +		status = set_client(&lockt->lt_clientid, cstate, nn, false);
> 		if (status)
> 			goto out;
> 	}
> @@ -7085,7 +7083,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
> 	dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n",
> 		clid->cl_boot, clid->cl_id);
> 
> -	status = lookup_clientid(clid, cstate, nn, false);
> +	status = set_client(clid, cstate, nn, false);
> 	if (status)
> 		return status;
> 
> @@ -7232,7 +7230,7 @@ nfs4_check_open_reclaim(clientid_t *clid,
> 	__be32 status;
> 
> 	/* find clientid in conf_id_hashtbl */
> -	status = lookup_clientid(clid, cstate, nn, false);
> +	status = set_clientid(clid, cstate, nn, false);

Ditto.


> 	if (status)
> 		return nfserr_reclaim_bad;
> 
> -- 
> 2.29.2
> 

--
Chuck Lever




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 2/8] nfsd: simplify process_lock
  2021-01-20 21:01   ` Chuck Lever
@ 2021-01-20 21:25     ` J. Bruce Fields
  0 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-20 21:25 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing List

On Wed, Jan 20, 2021 at 09:01:53PM +0000, Chuck Lever wrote:
> 
> 
> > On Jan 19, 2021, at 5:35 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> > 
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > Similarly, this STALE_CLIENTID check is already handled inside
> > preprocess_confirmed_seqid_op().
> 
> I can't confirm this claim. Where is clid->cl_boot checked?


nfs4_preprocess_confirmed_seqid_op()->
	nfs4_preprocess_seqid_op()->
		nfsd4_lookup_stateid()->
			set_client()->
				STALE_CLIENTID()

> Did you mean nfs4_preprocess_confirmed_seqid_op() here?

Yeah.  But maybe it'd be better just to include that whole call stack in
the changelog.

--b.

> 
> 
> > (This may cause it to return a different error in some cases where
> > there are multiple things wrong; pynfs test SEQ10 regressed on this
> > commit because of that, but I think that's the test's fault, and I've
> > fixed it separately.)
> > 
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> > fs/nfsd/nfs4state.c | 4 ----
> > 1 file changed, 4 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index f9f89229dba6..7ea63d7cec4d 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -6697,10 +6697,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > 				&cstate->session->se_client->cl_clientid,
> > 				sizeof(clientid_t));
> > 
> > -		status = nfserr_stale_clientid;
> > -		if (STALE_CLIENTID(&lock->lk_new_clientid, nn))
> > -			goto out;
> > -
> > 		/* validate and update open stateid and open seqid */
> > 		status = nfs4_preprocess_confirmed_seqid_op(cstate,
> > 				        lock->lk_new_open_seqid,
> > -- 
> > 2.29.2
> > 
> 
> --
> Chuck Lever
> 
> 
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 4/8] nfsd: refactor lookup_clientid
  2021-01-20 21:02   ` Chuck Lever
@ 2021-01-20 23:01     ` J. Bruce Fields
  2021-01-21  1:54       ` Chuck Lever
  0 siblings, 1 reply; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-20 23:01 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing List

On Wed, Jan 20, 2021 at 09:02:25PM +0000, Chuck Lever wrote:
> 
> 
> > On Jan 19, 2021, at 5:35 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> > 
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > I think set_client is a better name, and the lookup itself I want to
> > use somewhere else.
> 
> Should this be two patches?
> - Rename lookup_clientid() to set_client()
> - Refactor the lookup_clientid() helper

Sure.

> nfs4state.c stops compiling with this patch. See below.

Oops, thanks.

If these look OK otherwise, I can resend.

--b.

> 
> 
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> > fs/nfsd/nfs4state.c | 50 ++++++++++++++++++++++-----------------------
> > 1 file changed, 24 insertions(+), 26 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index ba955bbf21df..d7128e16c86e 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -4633,40 +4633,40 @@ static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4
> > 	return nfserr_bad_seqid;
> > }
> > 
> > -static __be32 lookup_clientid(clientid_t *clid,
> > +static struct nfs4_client *lookup_clientid(clientid_t *clid, bool sessions,
> > +						struct nfsd_net *nn)
> > +{
> > +	struct nfs4_client *found;
> > +
> > +	spin_lock(&nn->client_lock);
> > +	found = find_confirmed_client(clid, sessions, nn);
> > +	if (found)
> > +		atomic_inc(&found->cl_rpc_users);
> > +	spin_unlock(&nn->client_lock);
> > +	return found;
> > +}
> > +
> > +static __be32 set_client(clientid_t *clid,
> > 		struct nfsd4_compound_state *cstate,
> > 		struct nfsd_net *nn,
> > 		bool sessions)
> > {
> > -	struct nfs4_client *found;
> > -
> > 	if (cstate->clp) {
> > -		found = cstate->clp;
> > -		if (!same_clid(&found->cl_clientid, clid))
> > +		if (!same_clid(&cstate->clp->cl_clientid, clid))
> > 			return nfserr_stale_clientid;
> > 		return nfs_ok;
> > 	}
> > -
> > 	if (STALE_CLIENTID(clid, nn))
> > 		return nfserr_stale_clientid;
> > -
> > 	/*
> > 	 * For v4.1+ we get the client in the SEQUENCE op. If we don't have one
> > 	 * cached already then we know this is for is for v4.0 and "sessions"
> > 	 * will be false.
> > 	 */
> > 	WARN_ON_ONCE(cstate->session);
> > -	spin_lock(&nn->client_lock);
> > -	found = find_confirmed_client(clid, sessions, nn);
> > -	if (!found) {
> > -		spin_unlock(&nn->client_lock);
> > +	cstate->clp = lookup_clientid(clid, sessions, nn);
> > +	if (!cstate->clp)
> > 		return nfserr_expired;
> > -	}
> > -	atomic_inc(&found->cl_rpc_users);
> > -	spin_unlock(&nn->client_lock);
> > -
> > -	/* Cache the nfs4_client in cstate! */
> > -	cstate->clp = found;
> > 	return nfs_ok;
> > }
> > 
> > @@ -4688,7 +4688,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
> > 	if (open->op_file == NULL)
> > 		return nfserr_jukebox;
> > 
> > -	status = lookup_clientid(clientid, cstate, nn, false);
> > +	status = set_client(clientid, cstate, nn, false);
> > 	if (status)
> > 		return status;
> > 	clp = cstate->clp;
> > @@ -5298,7 +5298,7 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> > 
> > 	trace_nfsd_clid_renew(clid);
> > -	status = lookup_clientid(clid, cstate, nn, false);
> > +	status = set_client(clid, cstate, nn, false);
> > 	if (status)
> > 		return status;
> > 	clp = cstate->clp;
> > @@ -5681,8 +5681,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> > 	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
> > 		CLOSE_STATEID(stateid))
> > 		return nfserr_bad_stateid;
> > -	status = lookup_clientid(&stateid->si_opaque.so_clid, cstate, nn,
> > -				 false);
> > +	status = set_client(&stateid->si_opaque.so_clid, cstate, nn, false);
> > 	if (status == nfserr_stale_clientid) {
> > 		if (cstate->session)
> > 			return nfserr_bad_stateid;
> > @@ -5821,7 +5820,7 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
> > 
> > 	cps->cpntf_time = ktime_get_boottime_seconds();
> > 	memset(&cstate, 0, sizeof(cstate));
> > -	status = lookup_clientid(&cps->cp_p_clid, &cstate, nn, true);
> > +	status = set_clientid(&cps->cp_p_clid, &cstate, nn, true);
> 
> This doesn't compile. I think you meant set_client() here.
> 
> 
> > 	if (status)
> > 		goto out;
> > 	status = nfsd4_lookup_stateid(&cstate, &cps->cp_p_stateid,
> > @@ -6900,8 +6899,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > 		 return nfserr_inval;
> > 
> > 	if (!nfsd4_has_session(cstate)) {
> > -		status = lookup_clientid(&lockt->lt_clientid, cstate, nn,
> > -					 false);
> > +		status = set_client(&lockt->lt_clientid, cstate, nn, false);
> > 		if (status)
> > 			goto out;
> > 	}
> > @@ -7085,7 +7083,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
> > 	dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n",
> > 		clid->cl_boot, clid->cl_id);
> > 
> > -	status = lookup_clientid(clid, cstate, nn, false);
> > +	status = set_client(clid, cstate, nn, false);
> > 	if (status)
> > 		return status;
> > 
> > @@ -7232,7 +7230,7 @@ nfs4_check_open_reclaim(clientid_t *clid,
> > 	__be32 status;
> > 
> > 	/* find clientid in conf_id_hashtbl */
> > -	status = lookup_clientid(clid, cstate, nn, false);
> > +	status = set_clientid(clid, cstate, nn, false);
> 
> Ditto.
> 
> 
> > 	if (status)
> > 		return nfserr_reclaim_bad;
> > 
> > -- 
> > 2.29.2
> > 
> 
> --
> Chuck Lever
> 
> 
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 4/8] nfsd: refactor lookup_clientid
  2021-01-20 23:01     ` J. Bruce Fields
@ 2021-01-21  1:54       ` Chuck Lever
  2021-01-21 18:49         ` [PATCH 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
  0 siblings, 1 reply; 35+ messages in thread
From: Chuck Lever @ 2021-01-21  1:54 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Linux NFS Mailing List



> On Jan 20, 2021, at 6:01 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> 
> On Wed, Jan 20, 2021 at 09:02:25PM +0000, Chuck Lever wrote:
>> 
>> 
>>>> On Jan 19, 2021, at 5:35 PM, J. Bruce Fields <bfields@redhat.com> wrote:
>>> 
>>> From: "J. Bruce Fields" <bfields@redhat.com>
>>> 
>>> I think set_client is a better name, and the lookup itself I want to
>>> use somewhere else.
>> 
>> Should this be two patches?
>> - Rename lookup_clientid() to set_client()
>> - Refactor the lookup_clientid() helper
> 
> Sure.
> 
>> nfs4state.c stops compiling with this patch. See below.
> 
> Oops, thanks.
> 
> If these look OK otherwise, I can resend.

Yes, please resend.


> 
> --b.
> 
>> 
>> 
>>> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>>> ---
>>> fs/nfsd/nfs4state.c | 50 ++++++++++++++++++++++-----------------------
>>> 1 file changed, 24 insertions(+), 26 deletions(-)
>>> 
>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>> index ba955bbf21df..d7128e16c86e 100644
>>> --- a/fs/nfsd/nfs4state.c
>>> +++ b/fs/nfsd/nfs4state.c
>>> @@ -4633,40 +4633,40 @@ static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4
>>>    return nfserr_bad_seqid;
>>> }
>>> 
>>> -static __be32 lookup_clientid(clientid_t *clid,
>>> +static struct nfs4_client *lookup_clientid(clientid_t *clid, bool sessions,
>>> +                        struct nfsd_net *nn)
>>> +{
>>> +    struct nfs4_client *found;
>>> +
>>> +    spin_lock(&nn->client_lock);
>>> +    found = find_confirmed_client(clid, sessions, nn);
>>> +    if (found)
>>> +        atomic_inc(&found->cl_rpc_users);
>>> +    spin_unlock(&nn->client_lock);
>>> +    return found;
>>> +}
>>> +
>>> +static __be32 set_client(clientid_t *clid,
>>>        struct nfsd4_compound_state *cstate,
>>>        struct nfsd_net *nn,
>>>        bool sessions)
>>> {
>>> -    struct nfs4_client *found;
>>> -
>>>    if (cstate->clp) {
>>> -        found = cstate->clp;
>>> -        if (!same_clid(&found->cl_clientid, clid))
>>> +        if (!same_clid(&cstate->clp->cl_clientid, clid))
>>>            return nfserr_stale_clientid;
>>>        return nfs_ok;
>>>    }
>>> -
>>>    if (STALE_CLIENTID(clid, nn))
>>>        return nfserr_stale_clientid;
>>> -
>>>    /*
>>>     * For v4.1+ we get the client in the SEQUENCE op. If we don't have one
>>>     * cached already then we know this is for is for v4.0 and "sessions"
>>>     * will be false.
>>>     */
>>>    WARN_ON_ONCE(cstate->session);
>>> -    spin_lock(&nn->client_lock);
>>> -    found = find_confirmed_client(clid, sessions, nn);
>>> -    if (!found) {
>>> -        spin_unlock(&nn->client_lock);
>>> +    cstate->clp = lookup_clientid(clid, sessions, nn);
>>> +    if (!cstate->clp)
>>>        return nfserr_expired;
>>> -    }
>>> -    atomic_inc(&found->cl_rpc_users);
>>> -    spin_unlock(&nn->client_lock);
>>> -
>>> -    /* Cache the nfs4_client in cstate! */
>>> -    cstate->clp = found;
>>>    return nfs_ok;
>>> }
>>> 
>>> @@ -4688,7 +4688,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
>>>    if (open->op_file == NULL)
>>>        return nfserr_jukebox;
>>> 
>>> -    status = lookup_clientid(clientid, cstate, nn, false);
>>> +    status = set_client(clientid, cstate, nn, false);
>>>    if (status)
>>>        return status;
>>>    clp = cstate->clp;
>>> @@ -5298,7 +5298,7 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>>    struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
>>> 
>>>    trace_nfsd_clid_renew(clid);
>>> -    status = lookup_clientid(clid, cstate, nn, false);
>>> +    status = set_client(clid, cstate, nn, false);
>>>    if (status)
>>>        return status;
>>>    clp = cstate->clp;
>>> @@ -5681,8 +5681,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
>>>    if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
>>>        CLOSE_STATEID(stateid))
>>>        return nfserr_bad_stateid;
>>> -    status = lookup_clientid(&stateid->si_opaque.so_clid, cstate, nn,
>>> -                 false);
>>> +    status = set_client(&stateid->si_opaque.so_clid, cstate, nn, false);
>>>    if (status == nfserr_stale_clientid) {
>>>        if (cstate->session)
>>>            return nfserr_bad_stateid;
>>> @@ -5821,7 +5820,7 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
>>> 
>>>    cps->cpntf_time = ktime_get_boottime_seconds();
>>>    memset(&cstate, 0, sizeof(cstate));
>>> -    status = lookup_clientid(&cps->cp_p_clid, &cstate, nn, true);
>>> +    status = set_clientid(&cps->cp_p_clid, &cstate, nn, true);
>> 
>> This doesn't compile. I think you meant set_client() here.
>> 
>> 
>>>    if (status)
>>>        goto out;
>>>    status = nfsd4_lookup_stateid(&cstate, &cps->cp_p_stateid,
>>> @@ -6900,8 +6899,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>>         return nfserr_inval;
>>> 
>>>    if (!nfsd4_has_session(cstate)) {
>>> -        status = lookup_clientid(&lockt->lt_clientid, cstate, nn,
>>> -                     false);
>>> +        status = set_client(&lockt->lt_clientid, cstate, nn, false);
>>>        if (status)
>>>            goto out;
>>>    }
>>> @@ -7085,7 +7083,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
>>>    dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n",
>>>        clid->cl_boot, clid->cl_id);
>>> 
>>> -    status = lookup_clientid(clid, cstate, nn, false);
>>> +    status = set_client(clid, cstate, nn, false);
>>>    if (status)
>>>        return status;
>>> 
>>> @@ -7232,7 +7230,7 @@ nfs4_check_open_reclaim(clientid_t *clid,
>>>    __be32 status;
>>> 
>>>    /* find clientid in conf_id_hashtbl */
>>> -    status = lookup_clientid(clid, cstate, nn, false);
>>> +    status = set_clientid(clid, cstate, nn, false);
>> 
>> Ditto.
>> 
>> 
>>>    if (status)
>>>        return nfserr_reclaim_bad;
>>> 
>>> -- 
>>> 2.29.2
>>> 
>> 
>> --
>> Chuck Lever
>> 
>> 
>> 
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 1/9] nfsd4: simplify process_lookup1
  2021-01-21  1:54       ` Chuck Lever
@ 2021-01-21 18:49         ` J. Bruce Fields
  2021-01-21 18:49           ` [PATCH 2/9] nfsd: simplify process_lock J. Bruce Fields
                             ` (7 more replies)
  0 siblings, 8 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 18:49 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

This STALE_CLIENTID check is redundant with the one in
lookup_clientid().

There's a difference in behavior is in case of memory allocation
failure, which I think isn't a big deal.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1d2cd6a88f61..f9f89229dba6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4680,8 +4680,6 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
 	struct nfs4_openowner *oo = NULL;
 	__be32 status;
 
-	if (STALE_CLIENTID(&open->op_clientid, nn))
-		return nfserr_stale_clientid;
 	/*
 	 * In case we need it later, after we've already created the
 	 * file and don't want to risk a further failure:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 2/9] nfsd: simplify process_lock
  2021-01-21 18:49         ` [PATCH 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
@ 2021-01-21 18:49           ` J. Bruce Fields
  2021-01-21 18:49           ` [PATCH 3/9] nfsd: simplify nfsd_renew J. Bruce Fields
                             ` (6 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 18:49 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

Similarly, this STALE_CLIENTID check is already handled by:

nfs4_preprocess_confirmed_seqid_op()->
        nfs4_preprocess_seqid_op()->
                nfsd4_lookup_stateid()->
                        set_client()->
                                STALE_CLIENTID()

(This may cause it to return a different error in some cases where
there are multiple things wrong; pynfs test SEQ10 regressed on this
commit because of that, but I think that's the test's fault, and I've
fixed it separately.)

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f9f89229dba6..7ea63d7cec4d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6697,10 +6697,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 				&cstate->session->se_client->cl_clientid,
 				sizeof(clientid_t));
 
-		status = nfserr_stale_clientid;
-		if (STALE_CLIENTID(&lock->lk_new_clientid, nn))
-			goto out;
-
 		/* validate and update open stateid and open seqid */
 		status = nfs4_preprocess_confirmed_seqid_op(cstate,
 				        lock->lk_new_open_seqid,
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 3/9] nfsd: simplify nfsd_renew
  2021-01-21 18:49         ` [PATCH 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
  2021-01-21 18:49           ` [PATCH 2/9] nfsd: simplify process_lock J. Bruce Fields
@ 2021-01-21 18:49           ` J. Bruce Fields
  2021-01-21 18:49           ` [PATCH 4/9] nfsd: rename lookup_clientid->set_client J. Bruce Fields
                             ` (5 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 18:49 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

You can take the single-exit thing too far, I think.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7ea63d7cec4d..ba955bbf21df 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5300,15 +5300,12 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	trace_nfsd_clid_renew(clid);
 	status = lookup_clientid(clid, cstate, nn, false);
 	if (status)
-		goto out;
+		return status;
 	clp = cstate->clp;
-	status = nfserr_cb_path_down;
 	if (!list_empty(&clp->cl_delegations)
 			&& clp->cl_cb_state != NFSD4_CB_UP)
-		goto out;
-	status = nfs_ok;
-out:
-	return status;
+		return nfserr_cb_path_down;
+	return nfs_ok;
 }
 
 void
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 4/9] nfsd: rename lookup_clientid->set_client
  2021-01-21 18:49         ` [PATCH 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
  2021-01-21 18:49           ` [PATCH 2/9] nfsd: simplify process_lock J. Bruce Fields
  2021-01-21 18:49           ` [PATCH 3/9] nfsd: simplify nfsd_renew J. Bruce Fields
@ 2021-01-21 18:49           ` J. Bruce Fields
  2021-01-21 18:49           ` [PATCH 5/9] nfsd: refactor lookup_clientid J. Bruce Fields
                             ` (4 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 18:49 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

I think this is a better name, and I'm going to reuse elsewhere the code
that does the lookup itself.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ba955bbf21df..4bdd90074e24 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4633,7 +4633,7 @@ static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4
 	return nfserr_bad_seqid;
 }
 
-static __be32 lookup_clientid(clientid_t *clid,
+static __be32 set_client(clientid_t *clid,
 		struct nfsd4_compound_state *cstate,
 		struct nfsd_net *nn,
 		bool sessions)
@@ -4688,7 +4688,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
 	if (open->op_file == NULL)
 		return nfserr_jukebox;
 
-	status = lookup_clientid(clientid, cstate, nn, false);
+	status = set_client(clientid, cstate, nn, false);
 	if (status)
 		return status;
 	clp = cstate->clp;
@@ -5298,7 +5298,7 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 
 	trace_nfsd_clid_renew(clid);
-	status = lookup_clientid(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn, false);
 	if (status)
 		return status;
 	clp = cstate->clp;
@@ -5681,8 +5681,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
 		CLOSE_STATEID(stateid))
 		return nfserr_bad_stateid;
-	status = lookup_clientid(&stateid->si_opaque.so_clid, cstate, nn,
-				 false);
+	status = set_client(&stateid->si_opaque.so_clid, cstate, nn, false);
 	if (status == nfserr_stale_clientid) {
 		if (cstate->session)
 			return nfserr_bad_stateid;
@@ -5821,7 +5820,7 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 
 	cps->cpntf_time = ktime_get_boottime_seconds();
 	memset(&cstate, 0, sizeof(cstate));
-	status = lookup_clientid(&cps->cp_p_clid, &cstate, nn, true);
+	status = set_client(&cps->cp_p_clid, &cstate, nn, true);
 	if (status)
 		goto out;
 	status = nfsd4_lookup_stateid(&cstate, &cps->cp_p_stateid,
@@ -6900,8 +6899,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		 return nfserr_inval;
 
 	if (!nfsd4_has_session(cstate)) {
-		status = lookup_clientid(&lockt->lt_clientid, cstate, nn,
-					 false);
+		status = set_client(&lockt->lt_clientid, cstate, nn, false);
 		if (status)
 			goto out;
 	}
@@ -7085,7 +7083,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
 	dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n",
 		clid->cl_boot, clid->cl_id);
 
-	status = lookup_clientid(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn, false);
 	if (status)
 		return status;
 
@@ -7232,7 +7230,7 @@ nfs4_check_open_reclaim(clientid_t *clid,
 	__be32 status;
 
 	/* find clientid in conf_id_hashtbl */
-	status = lookup_clientid(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn, false);
 	if (status)
 		return nfserr_reclaim_bad;
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 5/9] nfsd: refactor lookup_clientid
  2021-01-21 18:49         ` [PATCH 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
                             ` (2 preceding siblings ...)
  2021-01-21 18:49           ` [PATCH 4/9] nfsd: rename lookup_clientid->set_client J. Bruce Fields
@ 2021-01-21 18:49           ` J. Bruce Fields
  2021-01-21 18:49           ` [PATCH 6/9] nfsd: find_cpntf_state cleanup J. Bruce Fields
                             ` (3 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 18:49 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

This'll be useful elsewhere.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 4bdd90074e24..c74bf3b5b0de 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4633,40 +4633,40 @@ static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4
 	return nfserr_bad_seqid;
 }
 
+static struct nfs4_client *lookup_clientid(clientid_t *clid, bool sessions,
+						struct nfsd_net *nn)
+{
+	struct nfs4_client *found;
+
+	spin_lock(&nn->client_lock);
+	found = find_confirmed_client(clid, sessions, nn);
+	if (found)
+		atomic_inc(&found->cl_rpc_users);
+	spin_unlock(&nn->client_lock);
+	return found;
+}
+
 static __be32 set_client(clientid_t *clid,
 		struct nfsd4_compound_state *cstate,
 		struct nfsd_net *nn,
 		bool sessions)
 {
-	struct nfs4_client *found;
-
 	if (cstate->clp) {
-		found = cstate->clp;
-		if (!same_clid(&found->cl_clientid, clid))
+		if (!same_clid(&cstate->clp->cl_clientid, clid))
 			return nfserr_stale_clientid;
 		return nfs_ok;
 	}
-
 	if (STALE_CLIENTID(clid, nn))
 		return nfserr_stale_clientid;
-
 	/*
 	 * For v4.1+ we get the client in the SEQUENCE op. If we don't have one
 	 * cached already then we know this is for is for v4.0 and "sessions"
 	 * will be false.
 	 */
 	WARN_ON_ONCE(cstate->session);
-	spin_lock(&nn->client_lock);
-	found = find_confirmed_client(clid, sessions, nn);
-	if (!found) {
-		spin_unlock(&nn->client_lock);
+	cstate->clp = lookup_clientid(clid, sessions, nn);
+	if (!cstate->clp)
 		return nfserr_expired;
-	}
-	atomic_inc(&found->cl_rpc_users);
-	spin_unlock(&nn->client_lock);
-
-	/* Cache the nfs4_client in cstate! */
-	cstate->clp = found;
 	return nfs_ok;
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 6/9] nfsd: find_cpntf_state cleanup
  2021-01-21 18:49         ` [PATCH 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
                             ` (3 preceding siblings ...)
  2021-01-21 18:49           ` [PATCH 5/9] nfsd: refactor lookup_clientid J. Bruce Fields
@ 2021-01-21 18:49           ` J. Bruce Fields
  2021-01-21 18:49           ` [PATCH 7/9] nfsd: remove unused set_clientid argument J. Bruce Fields
                             ` (2 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 18:49 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

I think this unusual use of struct compound_state could cause confusion.

It's not that much more complicated just to open-code this stateid
lookup.

The only change in behavior should be a different error return in the
case the copy is using a source stateid that is a revoked delegation,
but I doubt that matters.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c74bf3b5b0de..db10fef1c1d2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5812,21 +5812,27 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 {
 	__be32 status;
 	struct nfs4_cpntf_state *cps = NULL;
-	struct nfsd4_compound_state cstate;
+	struct nfs4_client *found;
 
 	status = manage_cpntf_state(nn, st, NULL, &cps);
 	if (status)
 		return status;
 
 	cps->cpntf_time = ktime_get_boottime_seconds();
-	memset(&cstate, 0, sizeof(cstate));
-	status = set_client(&cps->cp_p_clid, &cstate, nn, true);
-	if (status)
+
+	status = nfserr_expired;
+	found = lookup_clientid(&cps->cp_p_clid, true, nn);
+	if (!found)
 		goto out;
-	status = nfsd4_lookup_stateid(&cstate, &cps->cp_p_stateid,
-				NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID,
-				stid, nn);
-	put_client_renew(cstate.clp);
+
+	*stid = find_stateid_by_type(found, &cps->cp_p_stateid,
+			NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID);
+	if (stid)
+		status = nfs_ok;
+	else
+		status = nfserr_bad_stateid;
+
+	put_client_renew(found);
 out:
 	nfs4_put_cpntf_state(nn, cps);
 	return status;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 7/9] nfsd: remove unused set_clientid argument
  2021-01-21 18:49         ` [PATCH 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
                             ` (4 preceding siblings ...)
  2021-01-21 18:49           ` [PATCH 6/9] nfsd: find_cpntf_state cleanup J. Bruce Fields
@ 2021-01-21 18:49           ` J. Bruce Fields
  2021-01-21 20:20             ` Chuck Lever
  2021-01-21 18:49           ` [PATCH 8/9] nfsd: simplify nfsd4_check_open_reclaim J. Bruce Fields
  2021-01-21 18:49           ` [PATCH 9/9] nfsd: cstate->session->se_client -> cstate->clp J. Bruce Fields
  7 siblings, 1 reply; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 18:49 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

Every caller is setting this argument to false, so we don't need it.

Also clarify comments a little.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index db10fef1c1d2..7c95f8808324 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4648,8 +4648,7 @@ static struct nfs4_client *lookup_clientid(clientid_t *clid, bool sessions,
 
 static __be32 set_client(clientid_t *clid,
 		struct nfsd4_compound_state *cstate,
-		struct nfsd_net *nn,
-		bool sessions)
+		struct nfsd_net *nn)
 {
 	if (cstate->clp) {
 		if (!same_clid(&cstate->clp->cl_clientid, clid))
@@ -4658,13 +4657,10 @@ static __be32 set_client(clientid_t *clid,
 	}
 	if (STALE_CLIENTID(clid, nn))
 		return nfserr_stale_clientid;
-	/*
-	 * For v4.1+ we get the client in the SEQUENCE op. If we don't have one
-	 * cached already then we know this is for is for v4.0 and "sessions"
-	 * will be false.
-	 */
+	/* For v4.1+ we should have gotten the client in the SEQUENCE op: */
 	WARN_ON_ONCE(cstate->session);
-	cstate->clp = lookup_clientid(clid, sessions, nn);
+	/* So we're looking for a 4.0 client (sessions = false): */
+	cstate->clp = lookup_clientid(clid, false, nn);
 	if (!cstate->clp)
 		return nfserr_expired;
 	return nfs_ok;
@@ -4688,7 +4684,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
 	if (open->op_file == NULL)
 		return nfserr_jukebox;
 
-	status = set_client(clientid, cstate, nn, false);
+	status = set_client(clientid, cstate, nn);
 	if (status)
 		return status;
 	clp = cstate->clp;
@@ -5298,7 +5294,7 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 
 	trace_nfsd_clid_renew(clid);
-	status = set_client(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn);
 	if (status)
 		return status;
 	clp = cstate->clp;
@@ -5681,7 +5677,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
 		CLOSE_STATEID(stateid))
 		return nfserr_bad_stateid;
-	status = set_client(&stateid->si_opaque.so_clid, cstate, nn, false);
+	status = set_client(&stateid->si_opaque.so_clid, cstate, nn);
 	if (status == nfserr_stale_clientid) {
 		if (cstate->session)
 			return nfserr_bad_stateid;
@@ -6905,7 +6901,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		 return nfserr_inval;
 
 	if (!nfsd4_has_session(cstate)) {
-		status = set_client(&lockt->lt_clientid, cstate, nn, false);
+		status = set_client(&lockt->lt_clientid, cstate, nn);
 		if (status)
 			goto out;
 	}
@@ -7089,7 +7085,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
 	dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n",
 		clid->cl_boot, clid->cl_id);
 
-	status = set_client(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn);
 	if (status)
 		return status;
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 8/9] nfsd: simplify nfsd4_check_open_reclaim
  2021-01-21 18:49         ` [PATCH 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
                             ` (5 preceding siblings ...)
  2021-01-21 18:49           ` [PATCH 7/9] nfsd: remove unused set_clientid argument J. Bruce Fields
@ 2021-01-21 18:49           ` J. Bruce Fields
  2021-01-21 18:49           ` [PATCH 9/9] nfsd: cstate->session->se_client -> cstate->clp J. Bruce Fields
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 18:49 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

The set_client() was already taken care of by process_open1().

The comments here are mostly redundant with the code.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c  |  3 +--
 fs/nfsd/nfs4state.c | 18 +++---------------
 fs/nfsd/state.h     |  3 +--
 3 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4727b7f03c5b..567af1f10d2c 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -423,8 +423,7 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 				goto out;
 			break;
 		case NFS4_OPEN_CLAIM_PREVIOUS:
-			status = nfs4_check_open_reclaim(&open->op_clientid,
-							 cstate, nn);
+			status = nfs4_check_open_reclaim(cstate->clp);
 			if (status)
 				goto out;
 			open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7c95f8808324..860805ffde1a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7221,25 +7221,13 @@ nfsd4_find_reclaim_client(struct xdr_netobj name, struct nfsd_net *nn)
 	return NULL;
 }
 
-/*
-* Called from OPEN. Look for clientid in reclaim list.
-*/
 __be32
-nfs4_check_open_reclaim(clientid_t *clid,
-		struct nfsd4_compound_state *cstate,
-		struct nfsd_net *nn)
+nfs4_check_open_reclaim(struct nfs4_client *clp)
 {
-	__be32 status;
-
-	/* find clientid in conf_id_hashtbl */
-	status = set_client(clid, cstate, nn, false);
-	if (status)
-		return nfserr_reclaim_bad;
-
-	if (test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &cstate->clp->cl_flags))
+	if (test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &clp->cl_flags))
 		return nfserr_no_grace;
 
-	if (nfsd4_client_record_check(cstate->clp))
+	if (nfsd4_client_record_check(clp))
 		return nfserr_reclaim_bad;
 
 	return nfs_ok;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 9eae11a9d21c..73deea353169 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -649,8 +649,7 @@ void nfs4_remove_reclaim_record(struct nfs4_client_reclaim *, struct nfsd_net *)
 extern void nfs4_release_reclaim(struct nfsd_net *);
 extern struct nfs4_client_reclaim *nfsd4_find_reclaim_client(struct xdr_netobj name,
 							struct nfsd_net *nn);
-extern __be32 nfs4_check_open_reclaim(clientid_t *clid,
-		struct nfsd4_compound_state *cstate, struct nfsd_net *nn);
+extern __be32 nfs4_check_open_reclaim(struct nfs4_client *);
 extern void nfsd4_probe_callback(struct nfs4_client *clp);
 extern void nfsd4_probe_callback_sync(struct nfs4_client *clp);
 extern void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 9/9] nfsd: cstate->session->se_client -> cstate->clp
  2021-01-21 18:49         ` [PATCH 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
                             ` (6 preceding siblings ...)
  2021-01-21 18:49           ` [PATCH 8/9] nfsd: simplify nfsd4_check_open_reclaim J. Bruce Fields
@ 2021-01-21 18:49           ` J. Bruce Fields
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 18:49 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

I'm not sure why we're writing this out the hard way in so many places.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c  |  5 ++---
 fs/nfsd/nfs4state.c | 16 ++++++++--------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 567af1f10d2c..f63a12a5278a 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -373,8 +373,7 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	 * Before RECLAIM_COMPLETE done, server should deny new lock
 	 */
 	if (nfsd4_has_session(cstate) &&
-	    !test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE,
-		      &cstate->session->se_client->cl_flags) &&
+	    !test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &cstate->clp->cl_flags) &&
 	    open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
 		return nfserr_grace;
 
@@ -1882,7 +1881,7 @@ nfsd4_getdeviceinfo(struct svc_rqst *rqstp,
 	nfserr = nfs_ok;
 	if (gdp->gd_maxcount != 0) {
 		nfserr = ops->proc_getdeviceinfo(exp->ex_path.mnt->mnt_sb,
-				rqstp, cstate->session->se_client, gdp);
+				rqstp, cstate->clp, gdp);
 	}
 
 	gdp->gd_notify_types &= ops->notify_types;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 860805ffde1a..7b865ed7c9d7 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3891,6 +3891,7 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *cstate, union nfsd4_op_u *u)
 {
 	struct nfsd4_reclaim_complete *rc = &u->reclaim_complete;
+	struct nfs4_client *clp = cstate->clp;
 	__be32 status = 0;
 
 	if (rc->rca_one_fs) {
@@ -3904,12 +3905,11 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp,
 	}
 
 	status = nfserr_complete_already;
-	if (test_and_set_bit(NFSD4_CLIENT_RECLAIM_COMPLETE,
-			     &cstate->session->se_client->cl_flags))
+	if (test_and_set_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &clp->cl_flags))
 		goto out;
 
 	status = nfserr_stale_clientid;
-	if (is_client_expired(cstate->session->se_client))
+	if (is_client_expired(clp))
 		/*
 		 * The following error isn't really legal.
 		 * But we only get here if the client just explicitly
@@ -3920,8 +3920,8 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp,
 		goto out;
 
 	status = nfs_ok;
-	nfsd4_client_record_create(cstate->session->se_client);
-	inc_reclaim_complete(cstate->session->se_client);
+	nfsd4_client_record_create(clp);
+	inc_reclaim_complete(clp);
 out:
 	return status;
 }
@@ -5917,7 +5917,7 @@ nfsd4_test_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 {
 	struct nfsd4_test_stateid *test_stateid = &u->test_stateid;
 	struct nfsd4_test_stateid_id *stateid;
-	struct nfs4_client *cl = cstate->session->se_client;
+	struct nfs4_client *cl = cstate->clp;
 
 	list_for_each_entry(stateid, &test_stateid->ts_stateid_list, ts_id_list)
 		stateid->ts_id_status =
@@ -5963,7 +5963,7 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	stateid_t *stateid = &free_stateid->fr_stateid;
 	struct nfs4_stid *s;
 	struct nfs4_delegation *dp;
-	struct nfs4_client *cl = cstate->session->se_client;
+	struct nfs4_client *cl = cstate->clp;
 	__be32 ret = nfserr_bad_stateid;
 
 	spin_lock(&cl->cl_lock);
@@ -6692,7 +6692,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		if (nfsd4_has_session(cstate))
 			/* See rfc 5661 18.10.3: given clientid is ignored: */
 			memcpy(&lock->lk_new_clientid,
-				&cstate->session->se_client->cl_clientid,
+				&cstate->clp->cl_clientid,
 				sizeof(clientid_t));
 
 		/* validate and update open stateid and open seqid */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH 7/9] nfsd: remove unused set_clientid argument
  2021-01-21 18:49           ` [PATCH 7/9] nfsd: remove unused set_clientid argument J. Bruce Fields
@ 2021-01-21 20:20             ` Chuck Lever
  2021-01-21 20:42               ` J. Bruce Fields
  0 siblings, 1 reply; 35+ messages in thread
From: Chuck Lever @ 2021-01-21 20:20 UTC (permalink / raw)
  To: Bruce Fields; +Cc: Linux NFS Mailing List



> On Jan 21, 2021, at 1:49 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> 
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> Every caller is setting this argument to false, so we don't need it.
> 
> Also clarify comments a little.

Some nits:

- The subject says "set_clientid" but the function name is "set_client"

- Is the WARN_ON_ONCE() still needed? (noticed yesterday, but I forgot
to mention it)

- This one doesn't compile:

[cel@klimt linux]$ make C=1 W=1 fs/nfsd/nfs4state.o
make[1]: Entering directory '/home/cel/src/linux/obj/klimt.1015granger.net'
  GEN     Makefile
  UPD     include/config/kernel.release
  UPD     include/generated/utsrelease.h
  CALL    /home/cel/src/linux/linux/scripts/checksyscalls.sh
  CALL    /home/cel/src/linux/linux/scripts/atomic/check-atomics.sh
  DESCEND  objtool
  DESCEND  bpf/resolve_btfids
  CC [M]  fs/nfsd/nfs4state.o
/home/cel/src/linux/linux/fs/nfsd/nfs4state.c: In function ‘nfs4_check_open_reclaim’:
/home/cel/src/linux/linux/fs/nfsd/nfs4state.c:7235:11: error: too many arguments to function ‘set_client’
 7235 |  status = set_client(clid, cstate, nn, false);
      |           ^~~~~~~~~~
/home/cel/src/linux/linux/fs/nfsd/nfs4state.c:4649:15: note: declared here
 4649 | static __be32 set_client(clientid_t *clid,
      |               ^~~~~~~~~~
make[3]: *** [/home/cel/src/linux/linux/scripts/Makefile.build:279: fs/nfsd/nfs4state.o] Error 1
make[2]: *** [/home/cel/src/linux/linux/scripts/Makefile.build:496: fs/nfsd] Error 2
make[1]: *** [/home/cel/src/linux/linux/Makefile:1805: fs] Error 2
make[1]: Leaving directory '/home/cel/src/linux/obj/klimt.1015granger.net'
make: *** [Makefile:185: __sub-make] Error 2
[cel@klimt linux]$

I think I will need a v3 of this series because 8/9 will have to
be fixed up too. Please be sure to add the series version number
when you repost.

Thanks!


> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
> fs/nfsd/nfs4state.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index db10fef1c1d2..7c95f8808324 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4648,8 +4648,7 @@ static struct nfs4_client *lookup_clientid(clientid_t *clid, bool sessions,
> 
> static __be32 set_client(clientid_t *clid,
> 		struct nfsd4_compound_state *cstate,
> -		struct nfsd_net *nn,
> -		bool sessions)
> +		struct nfsd_net *nn)
> {
> 	if (cstate->clp) {
> 		if (!same_clid(&cstate->clp->cl_clientid, clid))
> @@ -4658,13 +4657,10 @@ static __be32 set_client(clientid_t *clid,
> 	}
> 	if (STALE_CLIENTID(clid, nn))
> 		return nfserr_stale_clientid;
> -	/*
> -	 * For v4.1+ we get the client in the SEQUENCE op. If we don't have one
> -	 * cached already then we know this is for is for v4.0 and "sessions"
> -	 * will be false.
> -	 */
> +	/* For v4.1+ we should have gotten the client in the SEQUENCE op: */
> 	WARN_ON_ONCE(cstate->session);
> -	cstate->clp = lookup_clientid(clid, sessions, nn);
> +	/* So we're looking for a 4.0 client (sessions = false): */
> +	cstate->clp = lookup_clientid(clid, false, nn);
> 	if (!cstate->clp)
> 		return nfserr_expired;
> 	return nfs_ok;
> @@ -4688,7 +4684,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
> 	if (open->op_file == NULL)
> 		return nfserr_jukebox;
> 
> -	status = set_client(clientid, cstate, nn, false);
> +	status = set_client(clientid, cstate, nn);
> 	if (status)
> 		return status;
> 	clp = cstate->clp;
> @@ -5298,7 +5294,7 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> 
> 	trace_nfsd_clid_renew(clid);
> -	status = set_client(clid, cstate, nn, false);
> +	status = set_client(clid, cstate, nn);
> 	if (status)
> 		return status;
> 	clp = cstate->clp;
> @@ -5681,7 +5677,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> 	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
> 		CLOSE_STATEID(stateid))
> 		return nfserr_bad_stateid;
> -	status = set_client(&stateid->si_opaque.so_clid, cstate, nn, false);
> +	status = set_client(&stateid->si_opaque.so_clid, cstate, nn);
> 	if (status == nfserr_stale_clientid) {
> 		if (cstate->session)
> 			return nfserr_bad_stateid;
> @@ -6905,7 +6901,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 		 return nfserr_inval;
> 
> 	if (!nfsd4_has_session(cstate)) {
> -		status = set_client(&lockt->lt_clientid, cstate, nn, false);
> +		status = set_client(&lockt->lt_clientid, cstate, nn);
> 		if (status)
> 			goto out;
> 	}
> @@ -7089,7 +7085,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
> 	dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n",
> 		clid->cl_boot, clid->cl_id);
> 
> -	status = set_client(clid, cstate, nn, false);
> +	status = set_client(clid, cstate, nn);
> 	if (status)
> 		return status;
> 
> -- 
> 2.29.2
> 

--
Chuck Lever




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 7/9] nfsd: remove unused set_clientid argument
  2021-01-21 20:20             ` Chuck Lever
@ 2021-01-21 20:42               ` J. Bruce Fields
  2021-01-21 22:57                 ` [PATCH v3 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
  0 siblings, 1 reply; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 20:42 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing List

On Thu, Jan 21, 2021 at 08:20:53PM +0000, Chuck Lever wrote:
> 
> 
> > On Jan 21, 2021, at 1:49 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> > 
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > Every caller is setting this argument to false, so we don't need it.
> > 
> > Also clarify comments a little.
> 
> Some nits:
> 
> - The subject says "set_clientid" but the function name is "set_client"

Thanks, fixed.

> - Is the WARN_ON_ONCE() still needed? (noticed yesterday, but I forgot
> to mention it)

No, I don't think that's an especially interesting case to check for.
I'll drop it.

> 
> - This one doesn't compile:

Oops, thanks, yes, I missed a caller that gets deleted in the next
patch.

> I think I will need a v3 of this series because 8/9 will have to
> be fixed up too. Please be sure to add the series version number
> when you repost.

OK!--b.

> 
> Thanks!
> 
> 
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> > fs/nfsd/nfs4state.c | 22 +++++++++-------------
> > 1 file changed, 9 insertions(+), 13 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index db10fef1c1d2..7c95f8808324 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -4648,8 +4648,7 @@ static struct nfs4_client *lookup_clientid(clientid_t *clid, bool sessions,
> > 
> > static __be32 set_client(clientid_t *clid,
> > 		struct nfsd4_compound_state *cstate,
> > -		struct nfsd_net *nn,
> > -		bool sessions)
> > +		struct nfsd_net *nn)
> > {
> > 	if (cstate->clp) {
> > 		if (!same_clid(&cstate->clp->cl_clientid, clid))
> > @@ -4658,13 +4657,10 @@ static __be32 set_client(clientid_t *clid,
> > 	}
> > 	if (STALE_CLIENTID(clid, nn))
> > 		return nfserr_stale_clientid;
> > -	/*
> > -	 * For v4.1+ we get the client in the SEQUENCE op. If we don't have one
> > -	 * cached already then we know this is for is for v4.0 and "sessions"
> > -	 * will be false.
> > -	 */
> > +	/* For v4.1+ we should have gotten the client in the SEQUENCE op: */
> > 	WARN_ON_ONCE(cstate->session);
> > -	cstate->clp = lookup_clientid(clid, sessions, nn);
> > +	/* So we're looking for a 4.0 client (sessions = false): */
> > +	cstate->clp = lookup_clientid(clid, false, nn);
> > 	if (!cstate->clp)
> > 		return nfserr_expired;
> > 	return nfs_ok;
> > @@ -4688,7 +4684,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
> > 	if (open->op_file == NULL)
> > 		return nfserr_jukebox;
> > 
> > -	status = set_client(clientid, cstate, nn, false);
> > +	status = set_client(clientid, cstate, nn);
> > 	if (status)
> > 		return status;
> > 	clp = cstate->clp;
> > @@ -5298,7 +5294,7 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> > 
> > 	trace_nfsd_clid_renew(clid);
> > -	status = set_client(clid, cstate, nn, false);
> > +	status = set_client(clid, cstate, nn);
> > 	if (status)
> > 		return status;
> > 	clp = cstate->clp;
> > @@ -5681,7 +5677,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> > 	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
> > 		CLOSE_STATEID(stateid))
> > 		return nfserr_bad_stateid;
> > -	status = set_client(&stateid->si_opaque.so_clid, cstate, nn, false);
> > +	status = set_client(&stateid->si_opaque.so_clid, cstate, nn);
> > 	if (status == nfserr_stale_clientid) {
> > 		if (cstate->session)
> > 			return nfserr_bad_stateid;
> > @@ -6905,7 +6901,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > 		 return nfserr_inval;
> > 
> > 	if (!nfsd4_has_session(cstate)) {
> > -		status = set_client(&lockt->lt_clientid, cstate, nn, false);
> > +		status = set_client(&lockt->lt_clientid, cstate, nn);
> > 		if (status)
> > 			goto out;
> > 	}
> > @@ -7089,7 +7085,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
> > 	dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n",
> > 		clid->cl_boot, clid->cl_id);
> > 
> > -	status = set_client(clid, cstate, nn, false);
> > +	status = set_client(clid, cstate, nn);
> > 	if (status)
> > 		return status;
> > 
> > -- 
> > 2.29.2
> > 
> 
> --
> Chuck Lever
> 
> 
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v3 1/9] nfsd4: simplify process_lookup1
  2021-01-21 20:42               ` J. Bruce Fields
@ 2021-01-21 22:57                 ` J. Bruce Fields
  2021-01-21 22:57                   ` [PATCH v3 2/9] nfsd: simplify process_lock J. Bruce Fields
                                     ` (7 more replies)
  0 siblings, 8 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 22:57 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

This STALE_CLIENTID check is redundant with the one in
lookup_clientid().

There's a difference in behavior is in case of memory allocation
failure, which I think isn't a big deal.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1d2cd6a88f61..f9f89229dba6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4680,8 +4680,6 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
 	struct nfs4_openowner *oo = NULL;
 	__be32 status;
 
-	if (STALE_CLIENTID(&open->op_clientid, nn))
-		return nfserr_stale_clientid;
 	/*
 	 * In case we need it later, after we've already created the
 	 * file and don't want to risk a further failure:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 2/9] nfsd: simplify process_lock
  2021-01-21 22:57                 ` [PATCH v3 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
@ 2021-01-21 22:57                   ` J. Bruce Fields
  2021-01-21 22:57                   ` [PATCH v3 3/9] nfsd: simplify nfsd_renew J. Bruce Fields
                                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 22:57 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

Similarly, this STALE_CLIENTID check is already handled by:

nfs4_preprocess_confirmed_seqid_op()->
        nfs4_preprocess_seqid_op()->
                nfsd4_lookup_stateid()->
                        set_client()->
                                STALE_CLIENTID()

(This may cause it to return a different error in some cases where
there are multiple things wrong; pynfs test SEQ10 regressed on this
commit because of that, but I think that's the test's fault, and I've
fixed it separately.)

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f9f89229dba6..7ea63d7cec4d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6697,10 +6697,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 				&cstate->session->se_client->cl_clientid,
 				sizeof(clientid_t));
 
-		status = nfserr_stale_clientid;
-		if (STALE_CLIENTID(&lock->lk_new_clientid, nn))
-			goto out;
-
 		/* validate and update open stateid and open seqid */
 		status = nfs4_preprocess_confirmed_seqid_op(cstate,
 				        lock->lk_new_open_seqid,
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 3/9] nfsd: simplify nfsd_renew
  2021-01-21 22:57                 ` [PATCH v3 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
  2021-01-21 22:57                   ` [PATCH v3 2/9] nfsd: simplify process_lock J. Bruce Fields
@ 2021-01-21 22:57                   ` J. Bruce Fields
  2021-01-21 22:57                   ` [PATCH v3 4/9] nfsd: rename lookup_clientid->set_client J. Bruce Fields
                                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 22:57 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

You can take the single-exit thing too far, I think.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7ea63d7cec4d..ba955bbf21df 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5300,15 +5300,12 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	trace_nfsd_clid_renew(clid);
 	status = lookup_clientid(clid, cstate, nn, false);
 	if (status)
-		goto out;
+		return status;
 	clp = cstate->clp;
-	status = nfserr_cb_path_down;
 	if (!list_empty(&clp->cl_delegations)
 			&& clp->cl_cb_state != NFSD4_CB_UP)
-		goto out;
-	status = nfs_ok;
-out:
-	return status;
+		return nfserr_cb_path_down;
+	return nfs_ok;
 }
 
 void
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 4/9] nfsd: rename lookup_clientid->set_client
  2021-01-21 22:57                 ` [PATCH v3 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
  2021-01-21 22:57                   ` [PATCH v3 2/9] nfsd: simplify process_lock J. Bruce Fields
  2021-01-21 22:57                   ` [PATCH v3 3/9] nfsd: simplify nfsd_renew J. Bruce Fields
@ 2021-01-21 22:57                   ` J. Bruce Fields
  2021-01-21 22:57                   ` [PATCH v3 5/9] nfsd: refactor set_client J. Bruce Fields
                                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 22:57 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

I think this is a better name, and I'm going to reuse elsewhere the code
that does the lookup itself.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ba955bbf21df..4bdd90074e24 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4633,7 +4633,7 @@ static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4
 	return nfserr_bad_seqid;
 }
 
-static __be32 lookup_clientid(clientid_t *clid,
+static __be32 set_client(clientid_t *clid,
 		struct nfsd4_compound_state *cstate,
 		struct nfsd_net *nn,
 		bool sessions)
@@ -4688,7 +4688,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
 	if (open->op_file == NULL)
 		return nfserr_jukebox;
 
-	status = lookup_clientid(clientid, cstate, nn, false);
+	status = set_client(clientid, cstate, nn, false);
 	if (status)
 		return status;
 	clp = cstate->clp;
@@ -5298,7 +5298,7 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 
 	trace_nfsd_clid_renew(clid);
-	status = lookup_clientid(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn, false);
 	if (status)
 		return status;
 	clp = cstate->clp;
@@ -5681,8 +5681,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
 		CLOSE_STATEID(stateid))
 		return nfserr_bad_stateid;
-	status = lookup_clientid(&stateid->si_opaque.so_clid, cstate, nn,
-				 false);
+	status = set_client(&stateid->si_opaque.so_clid, cstate, nn, false);
 	if (status == nfserr_stale_clientid) {
 		if (cstate->session)
 			return nfserr_bad_stateid;
@@ -5821,7 +5820,7 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 
 	cps->cpntf_time = ktime_get_boottime_seconds();
 	memset(&cstate, 0, sizeof(cstate));
-	status = lookup_clientid(&cps->cp_p_clid, &cstate, nn, true);
+	status = set_client(&cps->cp_p_clid, &cstate, nn, true);
 	if (status)
 		goto out;
 	status = nfsd4_lookup_stateid(&cstate, &cps->cp_p_stateid,
@@ -6900,8 +6899,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		 return nfserr_inval;
 
 	if (!nfsd4_has_session(cstate)) {
-		status = lookup_clientid(&lockt->lt_clientid, cstate, nn,
-					 false);
+		status = set_client(&lockt->lt_clientid, cstate, nn, false);
 		if (status)
 			goto out;
 	}
@@ -7085,7 +7083,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
 	dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n",
 		clid->cl_boot, clid->cl_id);
 
-	status = lookup_clientid(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn, false);
 	if (status)
 		return status;
 
@@ -7232,7 +7230,7 @@ nfs4_check_open_reclaim(clientid_t *clid,
 	__be32 status;
 
 	/* find clientid in conf_id_hashtbl */
-	status = lookup_clientid(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn, false);
 	if (status)
 		return nfserr_reclaim_bad;
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 5/9] nfsd: refactor set_client
  2021-01-21 22:57                 ` [PATCH v3 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
                                     ` (2 preceding siblings ...)
  2021-01-21 22:57                   ` [PATCH v3 4/9] nfsd: rename lookup_clientid->set_client J. Bruce Fields
@ 2021-01-21 22:57                   ` J. Bruce Fields
  2021-01-21 22:57                   ` [PATCH v3 6/9] nfsd: find_cpntf_state cleanup J. Bruce Fields
                                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 22:57 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

This'll be useful elsewhere.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 4bdd90074e24..c74bf3b5b0de 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4633,40 +4633,40 @@ static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4
 	return nfserr_bad_seqid;
 }
 
+static struct nfs4_client *lookup_clientid(clientid_t *clid, bool sessions,
+						struct nfsd_net *nn)
+{
+	struct nfs4_client *found;
+
+	spin_lock(&nn->client_lock);
+	found = find_confirmed_client(clid, sessions, nn);
+	if (found)
+		atomic_inc(&found->cl_rpc_users);
+	spin_unlock(&nn->client_lock);
+	return found;
+}
+
 static __be32 set_client(clientid_t *clid,
 		struct nfsd4_compound_state *cstate,
 		struct nfsd_net *nn,
 		bool sessions)
 {
-	struct nfs4_client *found;
-
 	if (cstate->clp) {
-		found = cstate->clp;
-		if (!same_clid(&found->cl_clientid, clid))
+		if (!same_clid(&cstate->clp->cl_clientid, clid))
 			return nfserr_stale_clientid;
 		return nfs_ok;
 	}
-
 	if (STALE_CLIENTID(clid, nn))
 		return nfserr_stale_clientid;
-
 	/*
 	 * For v4.1+ we get the client in the SEQUENCE op. If we don't have one
 	 * cached already then we know this is for is for v4.0 and "sessions"
 	 * will be false.
 	 */
 	WARN_ON_ONCE(cstate->session);
-	spin_lock(&nn->client_lock);
-	found = find_confirmed_client(clid, sessions, nn);
-	if (!found) {
-		spin_unlock(&nn->client_lock);
+	cstate->clp = lookup_clientid(clid, sessions, nn);
+	if (!cstate->clp)
 		return nfserr_expired;
-	}
-	atomic_inc(&found->cl_rpc_users);
-	spin_unlock(&nn->client_lock);
-
-	/* Cache the nfs4_client in cstate! */
-	cstate->clp = found;
 	return nfs_ok;
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 6/9] nfsd: find_cpntf_state cleanup
  2021-01-21 22:57                 ` [PATCH v3 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
                                     ` (3 preceding siblings ...)
  2021-01-21 22:57                   ` [PATCH v3 5/9] nfsd: refactor set_client J. Bruce Fields
@ 2021-01-21 22:57                   ` J. Bruce Fields
  2021-01-21 22:57                   ` [PATCH v3 7/9] nfsd: remove unused set_client argument J. Bruce Fields
                                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 22:57 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

I think this unusual use of struct compound_state could cause confusion.

It's not that much more complicated just to open-code this stateid
lookup.

The only change in behavior should be a different error return in the
case the copy is using a source stateid that is a revoked delegation,
but I doubt that matters.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c74bf3b5b0de..db10fef1c1d2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5812,21 +5812,27 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 {
 	__be32 status;
 	struct nfs4_cpntf_state *cps = NULL;
-	struct nfsd4_compound_state cstate;
+	struct nfs4_client *found;
 
 	status = manage_cpntf_state(nn, st, NULL, &cps);
 	if (status)
 		return status;
 
 	cps->cpntf_time = ktime_get_boottime_seconds();
-	memset(&cstate, 0, sizeof(cstate));
-	status = set_client(&cps->cp_p_clid, &cstate, nn, true);
-	if (status)
+
+	status = nfserr_expired;
+	found = lookup_clientid(&cps->cp_p_clid, true, nn);
+	if (!found)
 		goto out;
-	status = nfsd4_lookup_stateid(&cstate, &cps->cp_p_stateid,
-				NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID,
-				stid, nn);
-	put_client_renew(cstate.clp);
+
+	*stid = find_stateid_by_type(found, &cps->cp_p_stateid,
+			NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID);
+	if (stid)
+		status = nfs_ok;
+	else
+		status = nfserr_bad_stateid;
+
+	put_client_renew(found);
 out:
 	nfs4_put_cpntf_state(nn, cps);
 	return status;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 7/9] nfsd: remove unused set_client argument
  2021-01-21 22:57                 ` [PATCH v3 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
                                     ` (4 preceding siblings ...)
  2021-01-21 22:57                   ` [PATCH v3 6/9] nfsd: find_cpntf_state cleanup J. Bruce Fields
@ 2021-01-21 22:57                   ` J. Bruce Fields
  2021-01-21 22:57                   ` [PATCH v3 8/9] nfsd: simplify nfsd4_check_open_reclaim J. Bruce Fields
  2021-01-21 22:57                   ` [PATCH v3 9/9] nfsd: cstate->session->se_client -> cstate->clp J. Bruce Fields
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 22:57 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

Every caller is setting this argument to false, so we don't need it.

Also cut this comment a bit and remove an unnecessary warning.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index db10fef1c1d2..6d4b77b536fb 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4648,8 +4648,7 @@ static struct nfs4_client *lookup_clientid(clientid_t *clid, bool sessions,
 
 static __be32 set_client(clientid_t *clid,
 		struct nfsd4_compound_state *cstate,
-		struct nfsd_net *nn,
-		bool sessions)
+		struct nfsd_net *nn)
 {
 	if (cstate->clp) {
 		if (!same_clid(&cstate->clp->cl_clientid, clid))
@@ -4659,12 +4658,10 @@ static __be32 set_client(clientid_t *clid,
 	if (STALE_CLIENTID(clid, nn))
 		return nfserr_stale_clientid;
 	/*
-	 * For v4.1+ we get the client in the SEQUENCE op. If we don't have one
-	 * cached already then we know this is for is for v4.0 and "sessions"
-	 * will be false.
+	 * We're in the 4.0 case (otherwise the SEQUENCE op would have
+	 * set cstate->clp), so session = false:
 	 */
-	WARN_ON_ONCE(cstate->session);
-	cstate->clp = lookup_clientid(clid, sessions, nn);
+	cstate->clp = lookup_clientid(clid, false, nn);
 	if (!cstate->clp)
 		return nfserr_expired;
 	return nfs_ok;
@@ -4688,7 +4685,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
 	if (open->op_file == NULL)
 		return nfserr_jukebox;
 
-	status = set_client(clientid, cstate, nn, false);
+	status = set_client(clientid, cstate, nn);
 	if (status)
 		return status;
 	clp = cstate->clp;
@@ -5298,7 +5295,7 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 
 	trace_nfsd_clid_renew(clid);
-	status = set_client(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn);
 	if (status)
 		return status;
 	clp = cstate->clp;
@@ -5681,7 +5678,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
 		CLOSE_STATEID(stateid))
 		return nfserr_bad_stateid;
-	status = set_client(&stateid->si_opaque.so_clid, cstate, nn, false);
+	status = set_client(&stateid->si_opaque.so_clid, cstate, nn);
 	if (status == nfserr_stale_clientid) {
 		if (cstate->session)
 			return nfserr_bad_stateid;
@@ -6905,7 +6902,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		 return nfserr_inval;
 
 	if (!nfsd4_has_session(cstate)) {
-		status = set_client(&lockt->lt_clientid, cstate, nn, false);
+		status = set_client(&lockt->lt_clientid, cstate, nn);
 		if (status)
 			goto out;
 	}
@@ -7089,7 +7086,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
 	dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n",
 		clid->cl_boot, clid->cl_id);
 
-	status = set_client(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn);
 	if (status)
 		return status;
 
@@ -7236,7 +7233,7 @@ nfs4_check_open_reclaim(clientid_t *clid,
 	__be32 status;
 
 	/* find clientid in conf_id_hashtbl */
-	status = set_client(clid, cstate, nn, false);
+	status = set_client(clid, cstate, nn);
 	if (status)
 		return nfserr_reclaim_bad;
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 8/9] nfsd: simplify nfsd4_check_open_reclaim
  2021-01-21 22:57                 ` [PATCH v3 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
                                     ` (5 preceding siblings ...)
  2021-01-21 22:57                   ` [PATCH v3 7/9] nfsd: remove unused set_client argument J. Bruce Fields
@ 2021-01-21 22:57                   ` J. Bruce Fields
  2021-01-21 22:57                   ` [PATCH v3 9/9] nfsd: cstate->session->se_client -> cstate->clp J. Bruce Fields
  7 siblings, 0 replies; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 22:57 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

The set_client() was already taken care of by process_open1().

The comments here are mostly redundant with the code.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c  |  3 +--
 fs/nfsd/nfs4state.c | 18 +++---------------
 fs/nfsd/state.h     |  3 +--
 3 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4727b7f03c5b..567af1f10d2c 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -423,8 +423,7 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 				goto out;
 			break;
 		case NFS4_OPEN_CLAIM_PREVIOUS:
-			status = nfs4_check_open_reclaim(&open->op_clientid,
-							 cstate, nn);
+			status = nfs4_check_open_reclaim(cstate->clp);
 			if (status)
 				goto out;
 			open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 6d4b77b536fb..389456937bbe 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7222,25 +7222,13 @@ nfsd4_find_reclaim_client(struct xdr_netobj name, struct nfsd_net *nn)
 	return NULL;
 }
 
-/*
-* Called from OPEN. Look for clientid in reclaim list.
-*/
 __be32
-nfs4_check_open_reclaim(clientid_t *clid,
-		struct nfsd4_compound_state *cstate,
-		struct nfsd_net *nn)
+nfs4_check_open_reclaim(struct nfs4_client *clp)
 {
-	__be32 status;
-
-	/* find clientid in conf_id_hashtbl */
-	status = set_client(clid, cstate, nn);
-	if (status)
-		return nfserr_reclaim_bad;
-
-	if (test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &cstate->clp->cl_flags))
+	if (test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &clp->cl_flags))
 		return nfserr_no_grace;
 
-	if (nfsd4_client_record_check(cstate->clp))
+	if (nfsd4_client_record_check(clp))
 		return nfserr_reclaim_bad;
 
 	return nfs_ok;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 9eae11a9d21c..73deea353169 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -649,8 +649,7 @@ void nfs4_remove_reclaim_record(struct nfs4_client_reclaim *, struct nfsd_net *)
 extern void nfs4_release_reclaim(struct nfsd_net *);
 extern struct nfs4_client_reclaim *nfsd4_find_reclaim_client(struct xdr_netobj name,
 							struct nfsd_net *nn);
-extern __be32 nfs4_check_open_reclaim(clientid_t *clid,
-		struct nfsd4_compound_state *cstate, struct nfsd_net *nn);
+extern __be32 nfs4_check_open_reclaim(struct nfs4_client *);
 extern void nfsd4_probe_callback(struct nfs4_client *clp);
 extern void nfsd4_probe_callback_sync(struct nfs4_client *clp);
 extern void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 9/9] nfsd: cstate->session->se_client -> cstate->clp
  2021-01-21 22:57                 ` [PATCH v3 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
                                     ` (6 preceding siblings ...)
  2021-01-21 22:57                   ` [PATCH v3 8/9] nfsd: simplify nfsd4_check_open_reclaim J. Bruce Fields
@ 2021-01-21 22:57                   ` J. Bruce Fields
  2021-01-21 23:34                     ` Chuck Lever
  7 siblings, 1 reply; 35+ messages in thread
From: J. Bruce Fields @ 2021-01-21 22:57 UTC (permalink / raw)
  To: Chuck Lever; +Cc: linux-nfs, J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

I'm not sure why we're writing this out the hard way in so many places.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c  |  5 ++---
 fs/nfsd/nfs4state.c | 16 ++++++++--------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 567af1f10d2c..f63a12a5278a 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -373,8 +373,7 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	 * Before RECLAIM_COMPLETE done, server should deny new lock
 	 */
 	if (nfsd4_has_session(cstate) &&
-	    !test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE,
-		      &cstate->session->se_client->cl_flags) &&
+	    !test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &cstate->clp->cl_flags) &&
 	    open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
 		return nfserr_grace;
 
@@ -1882,7 +1881,7 @@ nfsd4_getdeviceinfo(struct svc_rqst *rqstp,
 	nfserr = nfs_ok;
 	if (gdp->gd_maxcount != 0) {
 		nfserr = ops->proc_getdeviceinfo(exp->ex_path.mnt->mnt_sb,
-				rqstp, cstate->session->se_client, gdp);
+				rqstp, cstate->clp, gdp);
 	}
 
 	gdp->gd_notify_types &= ops->notify_types;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 389456937bbe..f554e3480bb1 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3891,6 +3891,7 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *cstate, union nfsd4_op_u *u)
 {
 	struct nfsd4_reclaim_complete *rc = &u->reclaim_complete;
+	struct nfs4_client *clp = cstate->clp;
 	__be32 status = 0;
 
 	if (rc->rca_one_fs) {
@@ -3904,12 +3905,11 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp,
 	}
 
 	status = nfserr_complete_already;
-	if (test_and_set_bit(NFSD4_CLIENT_RECLAIM_COMPLETE,
-			     &cstate->session->se_client->cl_flags))
+	if (test_and_set_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &clp->cl_flags))
 		goto out;
 
 	status = nfserr_stale_clientid;
-	if (is_client_expired(cstate->session->se_client))
+	if (is_client_expired(clp))
 		/*
 		 * The following error isn't really legal.
 		 * But we only get here if the client just explicitly
@@ -3920,8 +3920,8 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp,
 		goto out;
 
 	status = nfs_ok;
-	nfsd4_client_record_create(cstate->session->se_client);
-	inc_reclaim_complete(cstate->session->se_client);
+	nfsd4_client_record_create(clp);
+	inc_reclaim_complete(clp);
 out:
 	return status;
 }
@@ -5918,7 +5918,7 @@ nfsd4_test_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 {
 	struct nfsd4_test_stateid *test_stateid = &u->test_stateid;
 	struct nfsd4_test_stateid_id *stateid;
-	struct nfs4_client *cl = cstate->session->se_client;
+	struct nfs4_client *cl = cstate->clp;
 
 	list_for_each_entry(stateid, &test_stateid->ts_stateid_list, ts_id_list)
 		stateid->ts_id_status =
@@ -5964,7 +5964,7 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	stateid_t *stateid = &free_stateid->fr_stateid;
 	struct nfs4_stid *s;
 	struct nfs4_delegation *dp;
-	struct nfs4_client *cl = cstate->session->se_client;
+	struct nfs4_client *cl = cstate->clp;
 	__be32 ret = nfserr_bad_stateid;
 
 	spin_lock(&cl->cl_lock);
@@ -6693,7 +6693,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		if (nfsd4_has_session(cstate))
 			/* See rfc 5661 18.10.3: given clientid is ignored: */
 			memcpy(&lock->lk_new_clientid,
-				&cstate->session->se_client->cl_clientid,
+				&cstate->clp->cl_clientid,
 				sizeof(clientid_t));
 
 		/* validate and update open stateid and open seqid */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 9/9] nfsd: cstate->session->se_client -> cstate->clp
  2021-01-21 22:57                   ` [PATCH v3 9/9] nfsd: cstate->session->se_client -> cstate->clp J. Bruce Fields
@ 2021-01-21 23:34                     ` Chuck Lever
  0 siblings, 0 replies; 35+ messages in thread
From: Chuck Lever @ 2021-01-21 23:34 UTC (permalink / raw)
  To: Bruce Fields; +Cc: Linux NFS Mailing List

Hi Bruce-

> On Jan 21, 2021, at 5:57 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> 
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> I'm not sure why we're writing this out the hard way in so many places.
> 
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>

Thanks. v3 of this series has been committed to the for-next branch at

git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git

in preparation for the v5.12 merge window.


> ---
> fs/nfsd/nfs4proc.c  |  5 ++---
> fs/nfsd/nfs4state.c | 16 ++++++++--------
> 2 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 567af1f10d2c..f63a12a5278a 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -373,8 +373,7 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 	 * Before RECLAIM_COMPLETE done, server should deny new lock
> 	 */
> 	if (nfsd4_has_session(cstate) &&
> -	    !test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE,
> -		      &cstate->session->se_client->cl_flags) &&
> +	    !test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &cstate->clp->cl_flags) &&
> 	    open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
> 		return nfserr_grace;
> 
> @@ -1882,7 +1881,7 @@ nfsd4_getdeviceinfo(struct svc_rqst *rqstp,
> 	nfserr = nfs_ok;
> 	if (gdp->gd_maxcount != 0) {
> 		nfserr = ops->proc_getdeviceinfo(exp->ex_path.mnt->mnt_sb,
> -				rqstp, cstate->session->se_client, gdp);
> +				rqstp, cstate->clp, gdp);
> 	}
> 
> 	gdp->gd_notify_types &= ops->notify_types;
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 389456937bbe..f554e3480bb1 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -3891,6 +3891,7 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp,
> 		struct nfsd4_compound_state *cstate, union nfsd4_op_u *u)
> {
> 	struct nfsd4_reclaim_complete *rc = &u->reclaim_complete;
> +	struct nfs4_client *clp = cstate->clp;
> 	__be32 status = 0;
> 
> 	if (rc->rca_one_fs) {
> @@ -3904,12 +3905,11 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp,
> 	}
> 
> 	status = nfserr_complete_already;
> -	if (test_and_set_bit(NFSD4_CLIENT_RECLAIM_COMPLETE,
> -			     &cstate->session->se_client->cl_flags))
> +	if (test_and_set_bit(NFSD4_CLIENT_RECLAIM_COMPLETE, &clp->cl_flags))
> 		goto out;
> 
> 	status = nfserr_stale_clientid;
> -	if (is_client_expired(cstate->session->se_client))
> +	if (is_client_expired(clp))
> 		/*
> 		 * The following error isn't really legal.
> 		 * But we only get here if the client just explicitly
> @@ -3920,8 +3920,8 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp,
> 		goto out;
> 
> 	status = nfs_ok;
> -	nfsd4_client_record_create(cstate->session->se_client);
> -	inc_reclaim_complete(cstate->session->se_client);
> +	nfsd4_client_record_create(clp);
> +	inc_reclaim_complete(clp);
> out:
> 	return status;
> }
> @@ -5918,7 +5918,7 @@ nfsd4_test_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> {
> 	struct nfsd4_test_stateid *test_stateid = &u->test_stateid;
> 	struct nfsd4_test_stateid_id *stateid;
> -	struct nfs4_client *cl = cstate->session->se_client;
> +	struct nfs4_client *cl = cstate->clp;
> 
> 	list_for_each_entry(stateid, &test_stateid->ts_stateid_list, ts_id_list)
> 		stateid->ts_id_status =
> @@ -5964,7 +5964,7 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 	stateid_t *stateid = &free_stateid->fr_stateid;
> 	struct nfs4_stid *s;
> 	struct nfs4_delegation *dp;
> -	struct nfs4_client *cl = cstate->session->se_client;
> +	struct nfs4_client *cl = cstate->clp;
> 	__be32 ret = nfserr_bad_stateid;
> 
> 	spin_lock(&cl->cl_lock);
> @@ -6693,7 +6693,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 		if (nfsd4_has_session(cstate))
> 			/* See rfc 5661 18.10.3: given clientid is ignored: */
> 			memcpy(&lock->lk_new_clientid,
> -				&cstate->session->se_client->cl_clientid,
> +				&cstate->clp->cl_clientid,
> 				sizeof(clientid_t));
> 
> 		/* validate and update open stateid and open seqid */
> -- 
> 2.29.2
> 

--
Chuck Lever




^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2021-01-21 23:35 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-19 22:35 [PATCH 0/8] miscellaneous nfsd4 state cleanup J. Bruce Fields
2021-01-19 22:35 ` [PATCH 1/8] nfsd4: simplify process_lookup1 J. Bruce Fields
2021-01-19 22:35 ` [PATCH 2/8] nfsd: simplify process_lock J. Bruce Fields
2021-01-20 21:01   ` Chuck Lever
2021-01-20 21:25     ` J. Bruce Fields
2021-01-19 22:35 ` [PATCH 3/8] nfsd: simplify nfsd_renew J. Bruce Fields
2021-01-19 22:35 ` [PATCH 4/8] nfsd: refactor lookup_clientid J. Bruce Fields
2021-01-20 21:02   ` Chuck Lever
2021-01-20 23:01     ` J. Bruce Fields
2021-01-21  1:54       ` Chuck Lever
2021-01-21 18:49         ` [PATCH 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
2021-01-21 18:49           ` [PATCH 2/9] nfsd: simplify process_lock J. Bruce Fields
2021-01-21 18:49           ` [PATCH 3/9] nfsd: simplify nfsd_renew J. Bruce Fields
2021-01-21 18:49           ` [PATCH 4/9] nfsd: rename lookup_clientid->set_client J. Bruce Fields
2021-01-21 18:49           ` [PATCH 5/9] nfsd: refactor lookup_clientid J. Bruce Fields
2021-01-21 18:49           ` [PATCH 6/9] nfsd: find_cpntf_state cleanup J. Bruce Fields
2021-01-21 18:49           ` [PATCH 7/9] nfsd: remove unused set_clientid argument J. Bruce Fields
2021-01-21 20:20             ` Chuck Lever
2021-01-21 20:42               ` J. Bruce Fields
2021-01-21 22:57                 ` [PATCH v3 1/9] nfsd4: simplify process_lookup1 J. Bruce Fields
2021-01-21 22:57                   ` [PATCH v3 2/9] nfsd: simplify process_lock J. Bruce Fields
2021-01-21 22:57                   ` [PATCH v3 3/9] nfsd: simplify nfsd_renew J. Bruce Fields
2021-01-21 22:57                   ` [PATCH v3 4/9] nfsd: rename lookup_clientid->set_client J. Bruce Fields
2021-01-21 22:57                   ` [PATCH v3 5/9] nfsd: refactor set_client J. Bruce Fields
2021-01-21 22:57                   ` [PATCH v3 6/9] nfsd: find_cpntf_state cleanup J. Bruce Fields
2021-01-21 22:57                   ` [PATCH v3 7/9] nfsd: remove unused set_client argument J. Bruce Fields
2021-01-21 22:57                   ` [PATCH v3 8/9] nfsd: simplify nfsd4_check_open_reclaim J. Bruce Fields
2021-01-21 22:57                   ` [PATCH v3 9/9] nfsd: cstate->session->se_client -> cstate->clp J. Bruce Fields
2021-01-21 23:34                     ` Chuck Lever
2021-01-21 18:49           ` [PATCH 8/9] nfsd: simplify nfsd4_check_open_reclaim J. Bruce Fields
2021-01-21 18:49           ` [PATCH 9/9] nfsd: cstate->session->se_client -> cstate->clp J. Bruce Fields
2021-01-19 22:35 ` [PATCH 5/8] nfsd: find_cpntf_state cleanup J. Bruce Fields
2021-01-19 22:35 ` [PATCH 6/8] nfsd: remove unused set_clientid argument J. Bruce Fields
2021-01-19 22:35 ` [PATCH 7/8] nfsd: simplify nfsd4_check_open_reclaim J. Bruce Fields
2021-01-19 22:35 ` [PATCH 8/8] nfsd: cstate->session->se_client -> cstate->clp J. Bruce Fields

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.