All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: NeilBrown <neil@brown.name>,
	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>,
	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, apparmor@lists.ubuntu.com,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org
Subject: Re: [PATCH v4 09/14] VFS/nfsd/ovl: introduce start_renaming() and end_renaming()
Date: Thu, 30 Oct 2025 09:22:29 -0400	[thread overview]
Message-ID: <c20d10da-08ef-4e9a-aff9-a7a9e6efe82b@oracle.com> (raw)
In-Reply-To: <20251029234353.1321957-10-neilb@ownmail.net>

On 10/29/25 7:31 PM, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
> 
> start_renaming() combines name lookup and locking to prepare for rename.
> It is used when two names need to be looked up as in nfsd and overlayfs -
> cases where one or both dentries are already available will be handled
> separately.
> 
> __start_renaming() avoids the inode_permission check and hash
> calculation and is suitable after filename_parentat() in do_renameat2().
> It subsumes quite a bit of code from that function.
> 
> start_renaming() does calculate the hash and check X permission and is
> suitable elsewhere:
> - nfsd_rename()
> - ovl_rename()
> 
> In ovl, ovl_do_rename_rd() is factored out of ovl_do_rename(), which
> itself will be gone by the end of the series.
> 
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: NeilBrown <neil@brown.name>
> 
> --
> Changes since v3:
>  - added missig dput() in ovl_rename when "whiteout" is not-NULL.
> 
> Changes since v2:
>  - in __start_renaming() some label have been renamed, and err
>    is always set before a "goto out_foo" rather than passing the
>    error in a dentry*.
>  - ovl_do_rename() changed to call the new ovl_do_rename_rd() rather
>    than keeping duplicate code
>  - code around ovl_cleanup() call in ovl_rename() restructured.
> ---
>  fs/namei.c               | 197 ++++++++++++++++++++++++++++-----------
>  fs/nfsd/vfs.c            |  73 +++++----------
>  fs/overlayfs/dir.c       |  74 +++++++--------
>  fs/overlayfs/overlayfs.h |  23 +++--
>  include/linux/namei.h    |   3 +
>  5 files changed, 218 insertions(+), 152 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 04d2819bd351..0ee0a110b088 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3667,6 +3667,129 @@ void unlock_rename(struct dentry *p1, struct dentry *p2)
>  }
>  EXPORT_SYMBOL(unlock_rename);
>  
> +/**
> + * __start_renaming - lookup and lock names for rename
> + * @rd:           rename data containing parent and flags, and
> + *                for receiving found dentries
> + * @lookup_flags: extra flags to pass to ->lookup (e.g. LOOKUP_REVAL,
> + *                LOOKUP_NO_SYMLINKS etc).
> + * @old_last:     name of object in @rd.old_parent
> + * @new_last:     name of object in @rd.new_parent
> + *
> + * Look up two names and ensure locks are in place for
> + * rename.
> + *
> + * On success the found dentries are stored in @rd.old_dentry,
> + * @rd.new_dentry.  These references and the lock are dropped by
> + * end_renaming().
> + *
> + * The passed in qstrs must have the hash calculated, and no permission
> + * checking is performed.
> + *
> + * Returns: zero or an error.
> + */
> +static int
> +__start_renaming(struct renamedata *rd, int lookup_flags,
> +		 struct qstr *old_last, struct qstr *new_last)
> +{
> +	struct dentry *trap;
> +	struct dentry *d1, *d2;
> +	int target_flags = LOOKUP_RENAME_TARGET | LOOKUP_CREATE;
> +	int err;
> +
> +	if (rd->flags & RENAME_EXCHANGE)
> +		target_flags = 0;
> +	if (rd->flags & RENAME_NOREPLACE)
> +		target_flags |= LOOKUP_EXCL;
> +
> +	trap = lock_rename(rd->old_parent, rd->new_parent);
> +	if (IS_ERR(trap))
> +		return PTR_ERR(trap);
> +
> +	d1 = lookup_one_qstr_excl(old_last, rd->old_parent,
> +				  lookup_flags);
> +	err = PTR_ERR(d1);
> +	if (IS_ERR(d1))
> +		goto out_unlock;
> +
> +	d2 = lookup_one_qstr_excl(new_last, rd->new_parent,
> +				  lookup_flags | target_flags);
> +	err = PTR_ERR(d2);
> +	if (IS_ERR(d2))
> +		goto out_dput_d1;
> +
> +	if (d1 == trap) {
> +		/* source is an ancestor of target */
> +		err = -EINVAL;
> +		goto out_dput_d2;
> +	}
> +
> +	if (d2 == trap) {
> +		/* target is an ancestor of source */
> +		if (rd->flags & RENAME_EXCHANGE)
> +			err = -EINVAL;
> +		else
> +			err = -ENOTEMPTY;
> +		goto out_dput_d2;
> +	}
> +
> +	rd->old_dentry = d1;
> +	rd->new_dentry = d2;
> +	return 0;
> +
> +out_dput_d2:
> +	dput(d2);
> +out_dput_d1:
> +	dput(d1);
> +out_unlock:
> +	unlock_rename(rd->old_parent, rd->new_parent);
> +	return err;
> +}
> +
> +/**
> + * start_renaming - lookup and lock names for rename with permission checking
> + * @rd:           rename data containing parent and flags, and
> + *                for receiving found dentries
> + * @lookup_flags: extra flags to pass to ->lookup (e.g. LOOKUP_REVAL,
> + *                LOOKUP_NO_SYMLINKS etc).
> + * @old_last:     name of object in @rd.old_parent
> + * @new_last:     name of object in @rd.new_parent
> + *
> + * Look up two names and ensure locks are in place for
> + * rename.
> + *
> + * On success the found dentries are stored in @rd.old_dentry,
> + * @rd.new_dentry.  These references and the lock are dropped by
> + * end_renaming().
> + *
> + * The passed in qstrs need not have the hash calculated, and basic
> + * eXecute permission checking is performed against @rd.mnt_idmap.
> + *
> + * Returns: zero or an error.
> + */
> +int start_renaming(struct renamedata *rd, int lookup_flags,
> +		   struct qstr *old_last, struct qstr *new_last)
> +{
> +	int err;
> +
> +	err = lookup_one_common(rd->mnt_idmap, old_last, rd->old_parent);
> +	if (err)
> +		return err;
> +	err = lookup_one_common(rd->mnt_idmap, new_last, rd->new_parent);
> +	if (err)
> +		return err;
> +	return __start_renaming(rd, lookup_flags, old_last, new_last);
> +}
> +EXPORT_SYMBOL(start_renaming);
> +
> +void end_renaming(struct renamedata *rd)
> +{
> +	unlock_rename(rd->old_parent, rd->new_parent);
> +	dput(rd->old_dentry);
> +	dput(rd->new_dentry);
> +}
> +EXPORT_SYMBOL(end_renaming);
> +
>  /**
>   * vfs_prepare_mode - prepare the mode to be used for a new inode
>   * @idmap:	idmap of the mount the inode was found from
> @@ -5504,14 +5627,11 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
>  		 struct filename *to, unsigned int flags)
>  {
>  	struct renamedata rd;
> -	struct dentry *old_dentry, *new_dentry;
> -	struct dentry *trap;
>  	struct path old_path, new_path;
>  	struct qstr old_last, new_last;
>  	int old_type, new_type;
>  	struct inode *delegated_inode = NULL;
> -	unsigned int lookup_flags = 0, target_flags =
> -		LOOKUP_RENAME_TARGET | LOOKUP_CREATE;
> +	unsigned int lookup_flags = 0;
>  	bool should_retry = false;
>  	int error = -EINVAL;
>  
> @@ -5522,11 +5642,6 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
>  	    (flags & RENAME_EXCHANGE))
>  		goto put_names;
>  
> -	if (flags & RENAME_EXCHANGE)
> -		target_flags = 0;
> -	if (flags & RENAME_NOREPLACE)
> -		target_flags |= LOOKUP_EXCL;
> -
>  retry:
>  	error = filename_parentat(olddfd, from, lookup_flags, &old_path,
>  				  &old_last, &old_type);
> @@ -5556,66 +5671,40 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
>  		goto exit2;
>  
>  retry_deleg:
> -	trap = lock_rename(new_path.dentry, old_path.dentry);
> -	if (IS_ERR(trap)) {
> -		error = PTR_ERR(trap);
> +	rd.old_parent	   = old_path.dentry;
> +	rd.mnt_idmap	   = mnt_idmap(old_path.mnt);
> +	rd.new_parent	   = new_path.dentry;
> +	rd.delegated_inode = &delegated_inode;
> +	rd.flags	   = flags;
> +
> +	error = __start_renaming(&rd, lookup_flags, &old_last, &new_last);
> +	if (error)
>  		goto exit_lock_rename;
> -	}
>  
> -	old_dentry = lookup_one_qstr_excl(&old_last, old_path.dentry,
> -					  lookup_flags);
> -	error = PTR_ERR(old_dentry);
> -	if (IS_ERR(old_dentry))
> -		goto exit3;
> -	new_dentry = lookup_one_qstr_excl(&new_last, new_path.dentry,
> -					  lookup_flags | target_flags);
> -	error = PTR_ERR(new_dentry);
> -	if (IS_ERR(new_dentry))
> -		goto exit4;
>  	if (flags & RENAME_EXCHANGE) {
> -		if (!d_is_dir(new_dentry)) {
> +		if (!d_is_dir(rd.new_dentry)) {
>  			error = -ENOTDIR;
>  			if (new_last.name[new_last.len])
> -				goto exit5;
> +				goto exit_unlock;
>  		}
>  	}
>  	/* unless the source is a directory trailing slashes give -ENOTDIR */
> -	if (!d_is_dir(old_dentry)) {
> +	if (!d_is_dir(rd.old_dentry)) {
>  		error = -ENOTDIR;
>  		if (old_last.name[old_last.len])
> -			goto exit5;
> +			goto exit_unlock;
>  		if (!(flags & RENAME_EXCHANGE) && new_last.name[new_last.len])
> -			goto exit5;
> -	}
> -	/* source should not be ancestor of target */
> -	error = -EINVAL;
> -	if (old_dentry == trap)
> -		goto exit5;
> -	/* target should not be an ancestor of source */
> -	if (!(flags & RENAME_EXCHANGE))
> -		error = -ENOTEMPTY;
> -	if (new_dentry == trap)
> -		goto exit5;
> +			goto exit_unlock;
> +	}
>  
> -	error = security_path_rename(&old_path, old_dentry,
> -				     &new_path, new_dentry, flags);
> +	error = security_path_rename(&old_path, rd.old_dentry,
> +				     &new_path, rd.new_dentry, flags);
>  	if (error)
> -		goto exit5;
> +		goto exit_unlock;
>  
> -	rd.old_parent	   = old_path.dentry;
> -	rd.old_dentry	   = old_dentry;
> -	rd.mnt_idmap	   = mnt_idmap(old_path.mnt);
> -	rd.new_parent	   = new_path.dentry;
> -	rd.new_dentry	   = new_dentry;
> -	rd.delegated_inode = &delegated_inode;
> -	rd.flags	   = flags;
>  	error = vfs_rename(&rd);
> -exit5:
> -	dput(new_dentry);
> -exit4:
> -	dput(old_dentry);
> -exit3:
> -	unlock_rename(new_path.dentry, old_path.dentry);
> +exit_unlock:
> +	end_renaming(&rd);
>  exit_lock_rename:
>  	if (delegated_inode) {
>  		error = break_deleg_wait(&delegated_inode);
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index cd64ffe12e0b..62109885d4db 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1885,11 +1885,12 @@ __be32
>  nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
>  			    struct svc_fh *tfhp, char *tname, int tlen)
>  {
> -	struct dentry	*fdentry, *tdentry, *odentry, *ndentry, *trap;
> +	struct dentry	*fdentry, *tdentry;
>  	int		type = S_IFDIR;
> +	struct renamedata rd = {};
>  	__be32		err;
>  	int		host_err;
> -	bool		close_cached = false;
> +	struct dentry	*close_cached;
>  
>  	trace_nfsd_vfs_rename(rqstp, ffhp, tfhp, fname, flen, tname, tlen);
>  
> @@ -1915,15 +1916,22 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
>  		goto out;
>  
>  retry:
> +	close_cached = NULL;
>  	host_err = fh_want_write(ffhp);
>  	if (host_err) {
>  		err = nfserrno(host_err);
>  		goto out;
>  	}
>  
> -	trap = lock_rename(tdentry, fdentry);
> -	if (IS_ERR(trap)) {
> -		err = nfserr_xdev;
> +	rd.mnt_idmap	= &nop_mnt_idmap;
> +	rd.old_parent	= fdentry;
> +	rd.new_parent	= tdentry;
> +
> +	host_err = start_renaming(&rd, 0, &QSTR_LEN(fname, flen),
> +				  &QSTR_LEN(tname, tlen));
> +
> +	if (host_err) {
> +		err = nfserrno(host_err);
>  		goto out_want_write;
>  	}
>  	err = fh_fill_pre_attrs(ffhp);
> @@ -1933,48 +1941,23 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
>  	if (err != nfs_ok)
>  		goto out_unlock;
>  
> -	odentry = lookup_one(&nop_mnt_idmap, &QSTR_LEN(fname, flen), fdentry);
> -	host_err = PTR_ERR(odentry);
> -	if (IS_ERR(odentry))
> -		goto out_nfserr;
> +	type = d_inode(rd.old_dentry)->i_mode & S_IFMT;
> +
> +	if (d_inode(rd.new_dentry))
> +		type = d_inode(rd.new_dentry)->i_mode & S_IFMT;
>  
> -	host_err = -ENOENT;
> -	if (d_really_is_negative(odentry))
> -		goto out_dput_old;
> -	host_err = -EINVAL;
> -	if (odentry == trap)
> -		goto out_dput_old;
> -	type = d_inode(odentry)->i_mode & S_IFMT;
> -
> -	ndentry = lookup_one(&nop_mnt_idmap, &QSTR_LEN(tname, tlen), tdentry);
> -	host_err = PTR_ERR(ndentry);
> -	if (IS_ERR(ndentry))
> -		goto out_dput_old;
> -	if (d_inode(ndentry))
> -		type = d_inode(ndentry)->i_mode & S_IFMT;
> -	host_err = -ENOTEMPTY;
> -	if (ndentry == trap)
> -		goto out_dput_new;
> -
> -	if ((ndentry->d_sb->s_export_op->flags & EXPORT_OP_CLOSE_BEFORE_UNLINK) &&
> -	    nfsd_has_cached_files(ndentry)) {
> -		close_cached = true;
> -		goto out_dput_old;
> +	if ((rd.new_dentry->d_sb->s_export_op->flags & EXPORT_OP_CLOSE_BEFORE_UNLINK) &&
> +	    nfsd_has_cached_files(rd.new_dentry)) {
> +		close_cached = dget(rd.new_dentry);
> +		goto out_unlock;
>  	} else {
> -		struct renamedata rd = {
> -			.mnt_idmap	= &nop_mnt_idmap,
> -			.old_parent	= fdentry,
> -			.old_dentry	= odentry,
> -			.new_parent	= tdentry,
> -			.new_dentry	= ndentry,
> -		};
>  		int retries;
>  
>  		for (retries = 1;;) {
>  			host_err = vfs_rename(&rd);
>  			if (host_err != -EAGAIN || !retries--)
>  				break;
> -			if (!nfsd_wait_for_delegreturn(rqstp, d_inode(odentry)))
> +			if (!nfsd_wait_for_delegreturn(rqstp, d_inode(rd.old_dentry)))
>  				break;
>  		}
>  		if (!host_err) {
> @@ -1983,11 +1966,6 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
>  				host_err = commit_metadata(ffhp);
>  		}
>  	}
> - out_dput_new:
> -	dput(ndentry);
> - out_dput_old:
> -	dput(odentry);
> - out_nfserr:
>  	if (host_err == -EBUSY) {
>  		/*
>  		 * See RFC 8881 Section 18.26.4 para 1-3: NFSv4 RENAME
> @@ -2006,7 +1984,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
>  		fh_fill_post_attrs(tfhp);
>  	}
>  out_unlock:
> -	unlock_rename(tdentry, fdentry);
> +	end_renaming(&rd);
>  out_want_write:
>  	fh_drop_write(ffhp);
>  
> @@ -2017,9 +1995,8 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
>  	 * until this point and then reattempt the whole shebang.
>  	 */
>  	if (close_cached) {
> -		close_cached = false;
> -		nfsd_close_cached_files(ndentry);
> -		dput(ndentry);
> +		nfsd_close_cached_files(close_cached);
> +		dput(close_cached);
>  		goto retry;
>  	}
>  out:
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index c8d0885ee5e0..0f2c2da68433 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -1124,9 +1124,7 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>  	int err;
>  	struct dentry *old_upperdir;
>  	struct dentry *new_upperdir;
> -	struct dentry *olddentry = NULL;
> -	struct dentry *newdentry = NULL;
> -	struct dentry *trap, *de;
> +	struct renamedata rd = {};
>  	bool old_opaque;
>  	bool new_opaque;
>  	bool cleanup_whiteout = false;
> @@ -1136,6 +1134,7 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>  	bool new_is_dir = d_is_dir(new);
>  	bool samedir = olddir == newdir;
>  	struct dentry *opaquedir = NULL;
> +	struct dentry *whiteout = NULL;
>  	const struct cred *old_cred = NULL;
>  	struct ovl_fs *ofs = OVL_FS(old->d_sb);
>  	LIST_HEAD(list);
> @@ -1233,29 +1232,21 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>  		}
>  	}
>  
> -	trap = lock_rename(new_upperdir, old_upperdir);
> -	if (IS_ERR(trap)) {
> -		err = PTR_ERR(trap);
> -		goto out_revert_creds;
> -	}
> +	rd.mnt_idmap = ovl_upper_mnt_idmap(ofs);
> +	rd.old_parent = old_upperdir;
> +	rd.new_parent = new_upperdir;
> +	rd.flags = flags;
>  
> -	de = ovl_lookup_upper(ofs, old->d_name.name, old_upperdir,
> -			      old->d_name.len);
> -	err = PTR_ERR(de);
> -	if (IS_ERR(de))
> -		goto out_unlock;
> -	olddentry = de;
> +	err = start_renaming(&rd, 0,
> +			     &QSTR_LEN(old->d_name.name, old->d_name.len),
> +			     &QSTR_LEN(new->d_name.name, new->d_name.len));
>  
> -	err = -ESTALE;
> -	if (!ovl_matches_upper(old, olddentry))
> -		goto out_unlock;
> +	if (err)
> +		goto out_revert_creds;
>  
> -	de = ovl_lookup_upper(ofs, new->d_name.name, new_upperdir,
> -			      new->d_name.len);
> -	err = PTR_ERR(de);
> -	if (IS_ERR(de))
> +	err = -ESTALE;
> +	if (!ovl_matches_upper(old, rd.old_dentry))
>  		goto out_unlock;
> -	newdentry = de;
>  
>  	old_opaque = ovl_dentry_is_opaque(old);
>  	new_opaque = ovl_dentry_is_opaque(new);
> @@ -1263,15 +1254,15 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>  	err = -ESTALE;
>  	if (d_inode(new) && ovl_dentry_upper(new)) {
>  		if (opaquedir) {
> -			if (newdentry != opaquedir)
> +			if (rd.new_dentry != opaquedir)
>  				goto out_unlock;
>  		} else {
> -			if (!ovl_matches_upper(new, newdentry))
> +			if (!ovl_matches_upper(new, rd.new_dentry))
>  				goto out_unlock;
>  		}
>  	} else {
> -		if (!d_is_negative(newdentry)) {
> -			if (!new_opaque || !ovl_upper_is_whiteout(ofs, newdentry))
> +		if (!d_is_negative(rd.new_dentry)) {
> +			if (!new_opaque || !ovl_upper_is_whiteout(ofs, rd.new_dentry))
>  				goto out_unlock;
>  		} else {
>  			if (flags & RENAME_EXCHANGE)
> @@ -1279,19 +1270,14 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>  		}
>  	}
>  
> -	if (olddentry == trap)
> -		goto out_unlock;
> -	if (newdentry == trap)
> -		goto out_unlock;
> -
> -	if (olddentry->d_inode == newdentry->d_inode)
> +	if (rd.old_dentry->d_inode == rd.new_dentry->d_inode)
>  		goto out_unlock;
>  
>  	err = 0;
>  	if (ovl_type_merge_or_lower(old))
>  		err = ovl_set_redirect(old, samedir);
>  	else if (is_dir && !old_opaque && ovl_type_merge(new->d_parent))
> -		err = ovl_set_opaque_xerr(old, olddentry, -EXDEV);
> +		err = ovl_set_opaque_xerr(old, rd.old_dentry, -EXDEV);
>  	if (err)
>  		goto out_unlock;
>  
> @@ -1299,18 +1285,24 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>  		err = ovl_set_redirect(new, samedir);
>  	else if (!overwrite && new_is_dir && !new_opaque &&
>  		 ovl_type_merge(old->d_parent))
> -		err = ovl_set_opaque_xerr(new, newdentry, -EXDEV);
> +		err = ovl_set_opaque_xerr(new, rd.new_dentry, -EXDEV);
>  	if (err)
>  		goto out_unlock;
>  
> -	err = ovl_do_rename(ofs, old_upperdir, olddentry,
> -			    new_upperdir, newdentry, flags);
> -	unlock_rename(new_upperdir, old_upperdir);
> +	err = ovl_do_rename_rd(&rd);
> +
> +	if (!err && cleanup_whiteout)
> +		whiteout = dget(rd.new_dentry);
> +
> +	end_renaming(&rd);
> +
>  	if (err)
>  		goto out_revert_creds;
>  
> -	if (cleanup_whiteout)
> -		ovl_cleanup(ofs, old_upperdir, newdentry);
> +	if (whiteout) {
> +		ovl_cleanup(ofs, old_upperdir, whiteout);
> +		dput(whiteout);
> +	}
>  
>  	if (overwrite && d_inode(new)) {
>  		if (new_is_dir)
> @@ -1336,14 +1328,12 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>  	else
>  		ovl_drop_write(old);
>  out:
> -	dput(newdentry);
> -	dput(olddentry);
>  	dput(opaquedir);
>  	ovl_cache_free(&list);
>  	return err;
>  
>  out_unlock:
> -	unlock_rename(new_upperdir, old_upperdir);
> +	end_renaming(&rd);
>  	goto out_revert_creds;
>  }
>  
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 49ad65f829dc..3cc85a893b5c 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -355,11 +355,24 @@ static inline int ovl_do_remove_acl(struct ovl_fs *ofs, struct dentry *dentry,
>  	return vfs_remove_acl(ovl_upper_mnt_idmap(ofs), dentry, acl_name);
>  }
>  
> +static inline int ovl_do_rename_rd(struct renamedata *rd)
> +{
> +	int err;
> +
> +	pr_debug("rename(%pd2, %pd2, 0x%x)\n", rd->old_dentry, rd->new_dentry,
> +		 rd->flags);
> +	err = vfs_rename(rd);
> +	if (err) {
> +		pr_debug("...rename(%pd2, %pd2, ...) = %i\n",
> +			 rd->old_dentry, rd->new_dentry, err);
> +	}
> +	return err;
> +}
> +
>  static inline int ovl_do_rename(struct ovl_fs *ofs, struct dentry *olddir,
>  				struct dentry *olddentry, struct dentry *newdir,
>  				struct dentry *newdentry, unsigned int flags)
>  {
> -	int err;
>  	struct renamedata rd = {
>  		.mnt_idmap	= ovl_upper_mnt_idmap(ofs),
>  		.old_parent	= olddir,
> @@ -369,13 +382,7 @@ static inline int ovl_do_rename(struct ovl_fs *ofs, struct dentry *olddir,
>  		.flags		= flags,
>  	};
>  
> -	pr_debug("rename(%pd2, %pd2, 0x%x)\n", olddentry, newdentry, flags);
> -	err = vfs_rename(&rd);
> -	if (err) {
> -		pr_debug("...rename(%pd2, %pd2, ...) = %i\n",
> -			 olddentry, newdentry, err);
> -	}
> -	return err;
> +	return ovl_do_rename_rd(&rd);
>  }
>  
>  static inline int ovl_do_whiteout(struct ovl_fs *ofs,
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index e5cff89679df..19c3d8e336d5 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -156,6 +156,9 @@ extern int follow_up(struct path *);
>  extern struct dentry *lock_rename(struct dentry *, struct dentry *);
>  extern struct dentry *lock_rename_child(struct dentry *, struct dentry *);
>  extern void unlock_rename(struct dentry *, struct dentry *);
> +int start_renaming(struct renamedata *rd, int lookup_flags,
> +		   struct qstr *old_last, struct qstr *new_last);
> +void end_renaming(struct renamedata *rd);
>  
>  /**
>   * mode_strip_umask - handle vfs umask stripping

For the fs/nfsd/vfs.c hunks:

Acked-by: Chuck Lever <chuck.lever@oracle.com>


-- 
Chuck Lever

  reply	other threads:[~2025-10-30 13:23 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-29 23:31 [PATCH v4 00/14] Create and use APIs to centralise locking for directory ops NeilBrown
2025-10-29 23:31 ` [PATCH v4 01/14] debugfs: rename end_creating() to debugfs_end_creating() NeilBrown
2025-10-30  5:32   ` Greg Kroah-Hartman
2025-10-29 23:31 ` [PATCH v4 02/14] VFS: introduce start_dirop() and end_dirop() NeilBrown
2025-10-29 23:31 ` [PATCH v4 03/14] VFS: tidy up do_unlinkat() NeilBrown
2025-10-29 23:31 ` [PATCH v4 04/14] VFS/nfsd/cachefiles/ovl: add start_creating() and end_creating() NeilBrown
2025-11-02  7:30   ` kernel test robot
2025-11-02  9:08     ` Dan Carpenter
2025-11-02  9:10     ` Dan Carpenter
2025-11-02  9:59     ` Amir Goldstein
2025-11-06  0:53       ` NeilBrown
2025-10-29 23:31 ` [PATCH v4 05/14] VFS/nfsd/cachefiles/ovl: introduce start_removing() and end_removing() NeilBrown
2025-10-29 23:31 ` [PATCH v4 06/14] VFS: introduce start_creating_noperm() and start_removing_noperm() NeilBrown
2025-10-29 23:31 ` [PATCH v4 07/14] VFS: introduce start_removing_dentry() NeilBrown
2025-10-30  6:11   ` Al Viro
2025-10-30 23:22     ` NeilBrown
2025-10-29 23:31 ` [PATCH v4 08/14] VFS: add start_creating_killable() and start_removing_killable() NeilBrown
2025-10-29 23:31 ` [PATCH v4 09/14] VFS/nfsd/ovl: introduce start_renaming() and end_renaming() NeilBrown
2025-10-30 13:22   ` Chuck Lever [this message]
2025-10-29 23:31 ` [PATCH v4 10/14] VFS/ovl/smb: introduce start_renaming_dentry() NeilBrown
2025-10-30  0:02   ` Namjae Jeon
2025-10-29 23:31 ` [PATCH v4 11/14] Add start_renaming_two_dentries() NeilBrown
2025-10-30  6:22   ` Al Viro
2025-10-30 23:37     ` NeilBrown
2025-10-29 23:31 ` [PATCH v4 12/14] ecryptfs: use new start_creating/start_removing APIs NeilBrown
2025-10-30  6:24   ` Al Viro
2025-10-30 23:41     ` NeilBrown
2025-10-29 23:31 ` [PATCH v4 13/14] VFS: change vfs_mkdir() to unlock on failure NeilBrown
2025-10-29 23:31 ` [PATCH v4 14/14] VFS: introduce end_creating_keep() 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=c20d10da-08ef-4e9a-aff9-a7a9e6efe82b@oracle.com \
    --to=chuck.lever@oracle.com \
    --cc=Dai.Ngo@oracle.com \
    --cc=amir73il@gmail.com \
    --cc=apparmor@lists.ubuntu.com \
    --cc=brauner@kernel.org \
    --cc=cem@kernel.org \
    --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 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.