From: Jeff Layton <jlayton@kernel.org>
To: NeilBrown <neil@brown.name>,
Linus Torvalds <torvalds@linux-foundation.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/7] VFS: unify old_mnt_idmap and new_mnt_idmap in renamedata
Date: Mon, 21 Jul 2025 09:06:10 -0400 [thread overview]
Message-ID: <c3cd7afa8a319d7e45266d30741acb582b0085b0.camel@kernel.org> (raw)
In-Reply-To: <20250721084412.370258-2-neil@brown.name>
On Mon, 2025-07-21 at 17:59 +1000, NeilBrown wrote:
> A rename can only rename with a single mount. Callers of vfs_rename()
> must and do ensure this is the case.
>
> So there is no point in having two mnt_idmaps in renamedata as they are
> always the same. Only of of them is passed to ->rename in any case.
>
> This patch replaces both with a single "mnt_idmap" and changes all
> callers.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/cachefiles/namei.c | 3 +--
> fs/ecryptfs/inode.c | 3 +--
> fs/namei.c | 17 ++++++++---------
> fs/nfsd/vfs.c | 3 +--
> fs/overlayfs/overlayfs.h | 3 +--
> fs/smb/server/vfs.c | 3 +--
> include/linux/fs.h | 6 ++----
> 7 files changed, 15 insertions(+), 23 deletions(-)
>
> diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> index 91dfd0231877..d1edb2ac3837 100644
> --- a/fs/cachefiles/namei.c
> +++ b/fs/cachefiles/namei.c
> @@ -387,10 +387,9 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> cachefiles_io_error(cache, "Rename security error %d", ret);
> } else {
> struct renamedata rd = {
> - .old_mnt_idmap = &nop_mnt_idmap,
> + .mnt_idmap = &nop_mnt_idmap,
> .old_parent = dir,
> .old_dentry = rep,
> - .new_mnt_idmap = &nop_mnt_idmap,
> .new_parent = cache->graveyard,
> .new_dentry = grave,
> };
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 72fbe1316ab8..abd954c6a14e 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -634,10 +634,9 @@ ecryptfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
> goto out_lock;
> }
>
> - rd.old_mnt_idmap = &nop_mnt_idmap;
> + rd.mnt_idmap = &nop_mnt_idmap;
> rd.old_parent = lower_old_dir_dentry;
> rd.old_dentry = lower_old_dentry;
> - rd.new_mnt_idmap = &nop_mnt_idmap;
> rd.new_parent = lower_new_dir_dentry;
> rd.new_dentry = lower_new_dentry;
> rc = vfs_rename(&rd);
> diff --git a/fs/namei.c b/fs/namei.c
> index cd43ff89fbaa..1c80445693d4 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -5024,20 +5024,20 @@ int vfs_rename(struct renamedata *rd)
> if (source == target)
> return 0;
>
> - error = may_delete(rd->old_mnt_idmap, old_dir, old_dentry, is_dir);
> + error = may_delete(rd->mnt_idmap, old_dir, old_dentry, is_dir);
> if (error)
> return error;
>
> if (!target) {
> - error = may_create(rd->new_mnt_idmap, new_dir, new_dentry);
> + error = may_create(rd->mnt_idmap, new_dir, new_dentry);
> } else {
> new_is_dir = d_is_dir(new_dentry);
>
> if (!(flags & RENAME_EXCHANGE))
> - error = may_delete(rd->new_mnt_idmap, new_dir,
> + error = may_delete(rd->mnt_idmap, new_dir,
> new_dentry, is_dir);
> else
> - error = may_delete(rd->new_mnt_idmap, new_dir,
> + error = may_delete(rd->mnt_idmap, new_dir,
> new_dentry, new_is_dir);
> }
> if (error)
> @@ -5052,13 +5052,13 @@ int vfs_rename(struct renamedata *rd)
> */
> if (new_dir != old_dir) {
> if (is_dir) {
> - error = inode_permission(rd->old_mnt_idmap, source,
> + error = inode_permission(rd->mnt_idmap, source,
> MAY_WRITE);
> if (error)
> return error;
> }
> if ((flags & RENAME_EXCHANGE) && new_is_dir) {
> - error = inode_permission(rd->new_mnt_idmap, target,
> + error = inode_permission(rd->mnt_idmap, target,
> MAY_WRITE);
> if (error)
> return error;
> @@ -5126,7 +5126,7 @@ int vfs_rename(struct renamedata *rd)
> if (error)
> goto out;
> }
> - error = old_dir->i_op->rename(rd->new_mnt_idmap, old_dir, old_dentry,
> + error = old_dir->i_op->rename(rd->mnt_idmap, old_dir, old_dentry,
> new_dir, new_dentry, flags);
> if (error)
> goto out;
> @@ -5269,10 +5269,9 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
>
> rd.old_parent = old_path.dentry;
> rd.old_dentry = old_dentry;
> - rd.old_mnt_idmap = mnt_idmap(old_path.mnt);
> + rd.mnt_idmap = mnt_idmap(old_path.mnt);
> rd.new_parent = new_path.dentry;
> rd.new_dentry = new_dentry;
> - rd.new_mnt_idmap = mnt_idmap(new_path.mnt);
> rd.delegated_inode = &delegated_inode;
> rd.flags = flags;
> error = vfs_rename(&rd);
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 7d522e426b2d..a21940cadede 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1940,10 +1940,9 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> goto out_dput_old;
> } else {
> struct renamedata rd = {
> - .old_mnt_idmap = &nop_mnt_idmap,
> + .mnt_idmap = &nop_mnt_idmap,
> .old_parent = fdentry,
> .old_dentry = odentry,
> - .new_mnt_idmap = &nop_mnt_idmap,
> .new_parent = tdentry,
> .new_dentry = ndentry,
> };
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index bb0d7ded8e76..4f84abaa0d68 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -361,10 +361,9 @@ static inline int ovl_do_rename(struct ovl_fs *ofs, struct dentry *olddir,
> {
> int err;
> struct renamedata rd = {
> - .old_mnt_idmap = ovl_upper_mnt_idmap(ofs),
> + .mnt_idmap = ovl_upper_mnt_idmap(ofs),
> .old_parent = olddir,
> .old_dentry = olddentry,
> - .new_mnt_idmap = ovl_upper_mnt_idmap(ofs),
> .new_parent = newdir,
> .new_dentry = newdentry,
> .flags = flags,
> diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> index 49e731dd0529..bfd62a21e75c 100644
> --- a/fs/smb/server/vfs.c
> +++ b/fs/smb/server/vfs.c
> @@ -770,10 +770,9 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
> goto out4;
> }
>
> - rd.old_mnt_idmap = mnt_idmap(old_path->mnt),
> + rd.mnt_idmap = mnt_idmap(old_path->mnt),
> rd.old_parent = old_parent,
> rd.old_dentry = old_child,
> - rd.new_mnt_idmap = mnt_idmap(new_path.mnt),
> rd.new_parent = new_path.dentry,
> rd.new_dentry = new_dentry,
> rd.flags = flags,
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 1948b2c828d3..d3e27da8a6aa 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2005,20 +2005,18 @@ int vfs_unlink(struct mnt_idmap *, struct inode *, struct dentry *,
>
> /**
> * struct renamedata - contains all information required for renaming
> - * @old_mnt_idmap: idmap of the old mount the inode was found from
> + * @mnt_idmap: idmap of the mount in which the rename is happening.
> * @old_parent: parent of source
> * @old_dentry: source
> - * @new_mnt_idmap: idmap of the new mount the inode was found from
> * @new_parent: parent of destination
> * @new_dentry: destination
> * @delegated_inode: returns an inode needing a delegation break
> * @flags: rename flags
> */
> struct renamedata {
> - struct mnt_idmap *old_mnt_idmap;
> + struct mnt_idmap *mnt_idmap;
> struct dentry *old_parent;
> struct dentry *old_dentry;
> - struct mnt_idmap *new_mnt_idmap;
> struct dentry *new_parent;
> struct dentry *new_dentry;
> struct inode **delegated_inode;
Nice cleanup.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
next prev parent reply other threads:[~2025-07-21 13:06 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-21 7:59 [PATCH 0/7 RFC] New APIs for name lookup and lock for directory operations NeilBrown
2025-07-21 7:59 ` [PATCH 1/7] VFS: unify old_mnt_idmap and new_mnt_idmap in renamedata NeilBrown
2025-07-21 13:06 ` Jeff Layton [this message]
2025-07-21 7:59 ` [PATCH 2/7] VFS: introduce done_dentry_lookup() NeilBrown
2025-07-21 13:39 ` Jeff Layton
2025-07-21 23:04 ` NeilBrown
2025-07-21 7:59 ` [PATCH 3/7] VFS: Change vfs_mkdir() to unlock on failure NeilBrown
2025-07-21 14:46 ` Jeff Layton
2025-07-21 23:08 ` NeilBrown
2025-07-21 8:00 ` [PATCH 4/7] VFS: introduce dentry_lookup() and friends NeilBrown
2025-07-21 10:20 ` Amir Goldstein
2025-07-21 23:27 ` NeilBrown
2025-07-23 15:17 ` Amir Goldstein
2025-07-21 8:00 ` [PATCH 5/7] VFS: add dentry_lookup_killable() NeilBrown
2025-07-21 8:00 ` [PATCH 6/7] VFS: add rename_lookup() NeilBrown
2025-07-21 8:00 ` [PATCH 7/7] VFS: introduce dentry_lock_in() NeilBrown
2025-07-21 11:32 ` [PATCH 0/7 RFC] New APIs for name lookup and lock for directory operations Amir Goldstein
2025-07-21 23:48 ` 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=c3cd7afa8a319d7e45266d30741acb582b0085b0.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=brauner@kernel.org \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=neil@brown.name \
--cc=torvalds@linux-foundation.org \
--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
all inboxes | Powered by JetHome®