[PATCH] nfsd: handle OPEN_XOR_DELEGATION outside of st_mutex in open codepath

[Jeff Layton]

When I originally wrote these patches, I was under the mistaken
impression that I didn't need to increment the stateid in the case of an
existing stateid (because we weren't returning it). After some
discussion upstream, it turns out that the server should ignore the
WANT_OPEN_XOR_DELEGATION flag if there is an outstanding open stateid.

Given that, there is no need to expand the scope of the st_mutex to
cover acquiring the delegation. The server may end up bumping the seqid
in a brand new open stateid that it ends up discarding, but that's not a
problem.

This also seems to lower the "App Overhead" on the fs_mark test that
the kernel test robot reported.

Reported-by: kernel test robot <oliver.sang@xxxxxxxxx>
Closes: https://lore.kernel.org/oe-lkp/202409161645.d44bced5-oliver.sang@xxxxxxxxx
Fixes: e816ca3f9ee0 ("nfsd: implement OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION")
Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
We had a report of a performance regression (in the form of higer "App
Overhead") in fs_mark. After some experimentation, I found that the
cause seemed to be the change in how the mutex is handled in e816ca3f9ee0.

This patch restores the App Overhead back to its previous levels (and
may even improve it a bit -- go figure). Chuck, this should probably be
squashed into e816ca3f9ee0.
---
 fs/nfsd/nfs4state.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9c2b1d251ab31b4e504cf301d1deaa4945bd244f..73c4b983c048c101d16ec146b3f80922bcca3c69 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6120,7 +6120,6 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 	struct nfs4_delegation *dp = NULL;
 	__be32 status;
 	bool new_stp = false;
-	bool deleg_only = false;
 
 	/*
 	 * Lookup file; if found, lookup stateid and check open request,
@@ -6175,6 +6174,9 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 			open->op_odstate = NULL;
 	}
 
+	nfs4_inc_and_copy_stateid(&open->op_stateid, &stp->st_stid);
+	mutex_unlock(&stp->st_mutex);
+
 	if (nfsd4_has_session(&resp->cstate)) {
 		if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
 			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
@@ -6194,17 +6196,12 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 	 * returned. Only respect WANT_OPEN_XOR_DELEGATION when a new
 	 * open stateid would have to be created.
 	 */
-	deleg_only = new_stp && open_xor_delegation(open);
-nodeleg:
-	if (deleg_only) {
+	if (new_stp && open_xor_delegation(open)) {
 		memcpy(&open->op_stateid, &zero_stateid, sizeof(open->op_stateid));
 		open->op_rflags |= OPEN4_RESULT_NO_OPEN_STATEID;
 		release_open_stateid(stp);
-	} else {
-		nfs4_inc_and_copy_stateid(&open->op_stateid, &stp->st_stid);
 	}
-	mutex_unlock(&stp->st_mutex);
-
+nodeleg:
 	status = nfs_ok;
 	trace_nfsd_open(&stp->st_stid.sc_stateid);
 out:

---
base-commit: 144cb1225cd863e1bd3ae3d577d86e1531afd932
change-id: 20241010-delstid-db2a12f242f3

Best regards,
-- 
Jeff Layton <jlayton@xxxxxxxxxx>