All of lore.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Chuck Lever III <chuck.lever@oracle.com>,
	Jeff Layton <jlayton@kernel.org>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: [PATCH 07/13] NFSD: change nfsd_create()/nfsd_symlink() to unlock directory before returning.
Date: Tue, 26 Jul 2022 16:45:30 +1000	[thread overview]
Message-ID: <165881793057.21666.9881237501107008368.stgit@noble.brown> (raw)
In-Reply-To: <165881740958.21666.5904057696047278505.stgit@noble.brown>

nfsd_create() usually returns with the directory still locked.
nfsd_symlink() usually returns with it unlocked.  This is clumsy.

Until recently nfsd_create() needed to keep the directory locked until
ACLs and security label had been set.  These are now set inside
nfsd_create() (in nfsd_setattr()) so this need is gone.

So change nfsd_create() and nfsd_symlink() to always unlock, and remove
any fh_unlock() calls that follow calls to these functions.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 fs/nfsd/nfs3proc.c |    2 --
 fs/nfsd/nfs4proc.c |    2 --
 fs/nfsd/vfs.c      |   38 +++++++++++++++++++++-----------------
 3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 5e369096e42f..0060a89997d4 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -382,7 +382,6 @@ nfsd3_proc_mkdir(struct svc_rqst *rqstp)
 	fh_init(&resp->fh, NFS3_FHSIZE);
 	resp->status = nfsd_create(rqstp, &resp->dirfh, argp->name, argp->len,
 				   &attrs, S_IFDIR, 0, &resp->fh);
-	fh_unlock(&resp->dirfh);
 	return rpc_success;
 }
 
@@ -459,7 +458,6 @@ nfsd3_proc_mknod(struct svc_rqst *rqstp)
 	type = nfs3_ftypes[argp->ftype];
 	resp->status = nfsd_create(rqstp, &resp->dirfh, argp->name, argp->len,
 				   &attrs, type, rdev, &resp->fh);
-	fh_unlock(&resp->dirfh);
 out:
 	return rpc_success;
 }
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index c49cd04cb567..915c9457a571 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -823,8 +823,6 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		create->cr_bmval[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
 	if (attrs.label_failed)
 		create->cr_bmval[0] &= ~FATTR4_WORD0_ACL;
-
-	fh_unlock(&cstate->current_fh);
 	set_change_info(&create->cr_cinfo, &cstate->current_fh);
 	fh_dup2(&cstate->current_fh, &resfh);
 out:
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 4bb05586a258..877331fabae0 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1378,8 +1378,10 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	fh_lock_nested(fhp, I_MUTEX_PARENT);
 	dchild = lookup_one_len(fname, dentry, flen);
 	host_err = PTR_ERR(dchild);
-	if (IS_ERR(dchild))
-		return nfserrno(host_err);
+	if (IS_ERR(dchild)) {
+		err = nfserrno(host_err);
+		goto out_unlock;
+	}
 	err = fh_compose(resfhp, fhp->fh_export, dchild, fhp);
 	/*
 	 * We unconditionally drop our ref to dchild as fh_compose will have
@@ -1387,9 +1389,12 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	 */
 	dput(dchild);
 	if (err)
-		return err;
-	return nfsd_create_locked(rqstp, fhp, fname, flen, attrs, type,
-					rdev, resfhp);
+		goto out_unlock;
+	err = nfsd_create_locked(rqstp, fhp, fname, flen, attrs, type,
+				 rdev, resfhp);
+out_unlock:
+	fh_unlock(fhp);
+	return err;
 }
 
 /*
@@ -1456,16 +1461,19 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		goto out;
 
 	host_err = fh_want_write(fhp);
-	if (host_err)
-		goto out_nfserr;
+	if (host_err) {
+		err = nfserrno(host_err);
+		goto out;
+	}
 
 	fh_lock(fhp);
 	dentry = fhp->fh_dentry;
 	dnew = lookup_one_len(fname, dentry, flen);
-	host_err = PTR_ERR(dnew);
-	if (IS_ERR(dnew))
-		goto out_nfserr;
-
+	if (IS_ERR(dnew)) {
+		err = nfserrno(PTR_ERR(dnew));
+		fh_unlock(fhp);
+		goto out_drop_write;
+	}
 	host_err = vfs_symlink(&init_user_ns, d_inode(dentry), dnew, path);
 	err = nfserrno(host_err);
 	cerr = fh_compose(resfhp, fhp->fh_export, dnew, fhp);
@@ -1474,16 +1482,12 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	fh_unlock(fhp);
 	if (!err)
 		err = nfserrno(commit_metadata(fhp));
-	fh_drop_write(fhp);
-
 	dput(dnew);
 	if (err==0) err = cerr;
+out_drop_write:
+	fh_drop_write(fhp);
 out:
 	return err;
-
-out_nfserr:
-	err = nfserrno(host_err);
-	goto out;
 }
 
 /*



  parent reply	other threads:[~2022-07-26  6:48 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-26  6:45 [PATCH 00/13] NFSD: clean up locking NeilBrown
2022-07-26  6:45 ` [PATCH 13/13] NFSD: discard fh_locked flag and fh_lock/fh_unlock NeilBrown
2022-07-26  6:45 ` [PATCH 01/13] NFSD: drop fh argument from alloc_init_deleg NeilBrown
2022-07-26  6:45 ` [PATCH 12/13] NFSD: use (un)lock_inode instead of fh_(un)lock for file operations NeilBrown
2022-07-28 14:55   ` Chuck Lever III
2022-07-26  6:45 ` [PATCH 04/13] NFSD: set attributes when creating symlinks NeilBrown
2022-07-26 12:40   ` Chuck Lever III
2022-07-26 12:53     ` Jeff Layton
2022-07-26 13:02       ` Chuck Lever III
2022-07-26 22:01     ` NeilBrown
2022-07-28 14:53   ` Chuck Lever III
2022-07-29  1:09     ` NeilBrown
2022-07-26  6:45 ` [PATCH 11/13] NFSD: use explicit lock/unlock for directory ops NeilBrown
2022-07-28 14:54   ` Chuck Lever III
2022-07-29  1:27     ` NeilBrown
2022-07-29 14:31       ` Chuck Lever III
2022-07-29 14:34         ` Chuck Lever III
2022-07-26  6:45 ` [PATCH 02/13] NFSD: verify the opened dentry after setting a delegation NeilBrown
2022-07-27 15:44   ` Jeff Layton
2022-07-26  6:45 ` [PATCH 09/13] NFSD: only call fh_unlock() once in nfsd_link() NeilBrown
2022-07-26  6:45 ` [PATCH 06/13] NFSD: add posix ACLs to struct nfsd_attrs NeilBrown
2022-07-29 14:24   ` Chuck Lever III
2022-08-01  0:13     ` NeilBrown
2022-07-26  6:45 ` [PATCH 10/13] NFSD: reduce locking in nfsd_lookup() NeilBrown
2022-07-28 14:54   ` Chuck Lever III
2022-07-26  6:45 ` [PATCH 03/13] NFSD: introduce struct nfsd_attrs NeilBrown
2022-07-28 14:53   ` Chuck Lever III
2022-07-26  6:45 ` [PATCH 05/13] NFSD: add security label to " NeilBrown
2022-07-28 14:54   ` Chuck Lever III
2022-07-29  1:11     ` NeilBrown
2022-07-26  6:45 ` NeilBrown [this message]
2022-07-26  6:45 ` [PATCH 08/13] NFSD: always drop directory lock in nfsd_unlink() NeilBrown
2022-07-27 15:50 ` [PATCH 00/13] NFSD: clean up locking Jeff Layton
2022-07-27 17:12   ` Chuck Lever III
2022-07-27 23:48   ` NeilBrown
2022-07-28 14:52 ` Chuck Lever III
2022-07-29  1:29   ` NeilBrown

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=165881793057.21666.9881237501107008368.stgit@noble.brown \
    --to=neilb@suse.de \
    --cc=chuck.lever@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is 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.