public inbox for ecryptfs@vger.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@ownmail.net>
To: "Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Christian Brauner" <brauner@kernel.org>,
	"Amir Goldstein" <amir73il@gmail.com>
Cc: "Jan Kara" <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org, Jeff Layton <jlayton@kernel.org>,
	Chris Mason <clm@fb.com>, David Sterba <dsterba@suse.com>,
	David Howells <dhowells@redhat.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Danilo Krummrich <dakr@kernel.org>,
	Tyler Hicks <code@tyhicks.com>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Chuck Lever <chuck.lever@oracle.com>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <Dai.Ngo@oracle.com>, Namjae Jeon <linkinjeon@kernel.org>,
	Steve French <smfrench@gmail.com>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Carlos Maiolino <cem@kernel.org>,
	John Johansen <john.johansen@canonical.com>,
	Paul Moore <paul@paul-moore.com>,
	James Morris <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	Stephen Smalley <stephen.smalley.work@gmail.com>,
	Ondrej Mosnacek <omosnace@redhat.com>,
	Mateusz Guzik <mjguzik@gmail.com>,
	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	Stefan Berger <stefanb@linux.ibm.com>,
	"Darrick J. Wong" <djwong@kernel.org>,
	linux-kernel@vger.kernel.org, netfs@lists.linux.dev,
	ecryptfs@vger.kernel.org, linux-nfs@vger.kernel.org,
	linux-unionfs@vger.kernel.org, linux-cifs@vger.kernel.org,
	linux-xfs@vger.kernel.org, linux-security-module@vger.kernel.org,
	selinux@vger.kernel.org
Subject: [PATCH v5 03/14] VFS: tidy up do_unlinkat()
Date: Thu,  6 Nov 2025 11:50:47 +1100	[thread overview]
Message-ID: <20251106005333.956321-4-neilb@ownmail.net> (raw)
In-Reply-To: <20251106005333.956321-1-neilb@ownmail.net>

From: NeilBrown <neil@brown.name>

The simplification of locking in the previous patch opens up some room
for tidying up do_unlinkat()

- change all "exit" labels to describe what will happen at the label.
- always goto an exit label on an error - unwrap the "if (!IS_ERR())" branch.
- Move the "slashes" handing inline, but mark it as unlikely()
- simplify use of the "inode" variable - we no longer need to test for NULL.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/namei.c | 55 ++++++++++++++++++++++++++----------------------------
 1 file changed, 26 insertions(+), 29 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 231e1ffd4b8d..93c5fce2d814 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4755,65 +4755,62 @@ int do_unlinkat(int dfd, struct filename *name)
 	struct path path;
 	struct qstr last;
 	int type;
-	struct inode *inode = NULL;
+	struct inode *inode;
 	struct inode *delegated_inode = NULL;
 	unsigned int lookup_flags = 0;
 retry:
 	error = filename_parentat(dfd, name, lookup_flags, &path, &last, &type);
 	if (error)
-		goto exit1;
+		goto exit_putname;
 
 	error = -EISDIR;
 	if (type != LAST_NORM)
-		goto exit2;
+		goto exit_path_put;
 
 	error = mnt_want_write(path.mnt);
 	if (error)
-		goto exit2;
+		goto exit_path_put;
 retry_deleg:
 	dentry = start_dirop(path.dentry, &last, lookup_flags);
 	error = PTR_ERR(dentry);
-	if (!IS_ERR(dentry)) {
+	if (IS_ERR(dentry))
+		goto exit_drop_write;
 
-		/* Why not before? Because we want correct error value */
-		if (last.name[last.len])
-			goto slashes;
-		inode = dentry->d_inode;
-		ihold(inode);
-		error = security_path_unlink(&path, dentry);
-		if (error)
-			goto exit3;
-		error = vfs_unlink(mnt_idmap(path.mnt), path.dentry->d_inode,
-				   dentry, &delegated_inode);
-exit3:
+	/* Why not before? Because we want correct error value */
+	if (unlikely(last.name[last.len])) {
+		if (d_is_dir(dentry))
+			error = -EISDIR;
+		else
+			error = -ENOTDIR;
 		end_dirop(dentry);
+		goto exit_drop_write;
 	}
-	if (inode)
-		iput(inode);	/* truncate the inode here */
-	inode = NULL;
+	inode = dentry->d_inode;
+	ihold(inode);
+	error = security_path_unlink(&path, dentry);
+	if (error)
+		goto exit_end_dirop;
+	error = vfs_unlink(mnt_idmap(path.mnt), path.dentry->d_inode,
+			   dentry, &delegated_inode);
+exit_end_dirop:
+	end_dirop(dentry);
+	iput(inode);	/* truncate the inode here */
 	if (delegated_inode) {
 		error = break_deleg_wait(&delegated_inode);
 		if (!error)
 			goto retry_deleg;
 	}
+exit_drop_write:
 	mnt_drop_write(path.mnt);
-exit2:
+exit_path_put:
 	path_put(&path);
 	if (retry_estale(error, lookup_flags)) {
 		lookup_flags |= LOOKUP_REVAL;
-		inode = NULL;
 		goto retry;
 	}
-exit1:
+exit_putname:
 	putname(name);
 	return error;
-
-slashes:
-	if (d_is_dir(dentry))
-		error = -EISDIR;
-	else
-		error = -ENOTDIR;
-	goto exit3;
 }
 
 SYSCALL_DEFINE3(unlinkat, int, dfd, const char __user *, pathname, int, flag)
-- 
2.50.0.107.gf914562f5916.dirty


  parent reply	other threads:[~2025-11-06  0:54 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-06  0:50 [PATCH v5 00/14] Create and use APIs to centralise locking for directory ops NeilBrown
2025-11-06  0:50 ` [PATCH v5 01/14] debugfs: rename end_creating() to debugfs_end_creating() NeilBrown
2025-11-06  0:50 ` [PATCH v5 02/14] VFS: introduce start_dirop() and end_dirop() NeilBrown
2025-11-12 14:46   ` Jeff Layton
2025-11-06  0:50 ` NeilBrown [this message]
2025-11-12 14:47   ` [PATCH v5 03/14] VFS: tidy up do_unlinkat() Jeff Layton
2025-11-06  0:50 ` [PATCH v5 04/14] VFS/nfsd/cachefiles/ovl: add start_creating() and end_creating() NeilBrown
2025-11-06  0:50 ` [PATCH v5 05/14] VFS/nfsd/cachefiles/ovl: introduce start_removing() and end_removing() NeilBrown
2025-11-12 14:51   ` Jeff Layton
2025-11-12 23:51     ` NeilBrown
2025-11-06  0:50 ` [PATCH v5 06/14] VFS: introduce start_creating_noperm() and start_removing_noperm() NeilBrown
2025-11-06  0:50 ` [PATCH v5 07/14] VFS: introduce start_removing_dentry() NeilBrown
2025-11-06  1:56   ` Namjae Jeon
2025-11-12 14:58   ` Jeff Layton
2025-11-06  0:50 ` [PATCH v5 08/14] VFS: add start_creating_killable() and start_removing_killable() NeilBrown
2025-11-12 15:01   ` Jeff Layton
2025-11-06  0:50 ` [PATCH v5 09/14] VFS/nfsd/ovl: introduce start_renaming() and end_renaming() NeilBrown
2025-11-12 15:06   ` Jeff Layton
2025-11-06  0:50 ` [PATCH v5 10/14] VFS/ovl/smb: introduce start_renaming_dentry() NeilBrown
2025-11-12 19:36   ` Jeff Layton
2025-11-06  0:50 ` [PATCH v5 11/14] Add start_renaming_two_dentries() NeilBrown
2025-11-10 16:08   ` Stephen Smalley
2025-11-10 17:30     ` Stephen Smalley
2025-11-12 23:37     ` NeilBrown
2025-11-12 19:38   ` Jeff Layton
2025-11-06  0:50 ` [PATCH v5 12/14] ecryptfs: use new start_creating/start_removing APIs NeilBrown
2025-11-12 19:41   ` Jeff Layton
2025-11-06  0:50 ` [PATCH v5 13/14] VFS: change vfs_mkdir() to unlock on failure NeilBrown
2025-11-12 19:45   ` Jeff Layton
2025-11-06  0:50 ` [PATCH v5 14/14] VFS: introduce end_creating_keep() NeilBrown
2025-11-12 19:46   ` Jeff Layton
2025-11-06  9:02 ` [syzbot ci] Re: Create and use APIs to centralise locking for directory ops syzbot ci
2025-11-12 22:50   ` 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=20251106005333.956321-4-neilb@ownmail.net \
    --to=neilb@ownmail.net \
    --cc=Dai.Ngo@oracle.com \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=cem@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=clm@fb.com \
    --cc=code@tyhicks.com \
    --cc=dakr@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=djwong@kernel.org \
    --cc=dsterba@suse.com \
    --cc=ecryptfs@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jack@suse.cz \
    --cc=jlayton@kernel.org \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=miklos@szeredi.hu \
    --cc=mjguzik@gmail.com \
    --cc=neil@brown.name \
    --cc=netfs@lists.linux.dev \
    --cc=okorniev@redhat.com \
    --cc=omosnace@redhat.com \
    --cc=paul@paul-moore.com \
    --cc=rafael@kernel.org \
    --cc=selinux@vger.kernel.org \
    --cc=senozhatsky@chromium.org \
    --cc=serge@hallyn.com \
    --cc=smfrench@gmail.com \
    --cc=stefanb@linux.ibm.com \
    --cc=stephen.smalley.work@gmail.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

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

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