[PATCH 0/2] Minor VFS-related cleanups

[PATCH 0/2] Minor VFS-related cleanups

[NeilBrown]

These are improvements that I think are generally useful which I
developed as part of my work to change directory locking.

Thanks,
NeilBrown

 [PATCH 1/2] VFS: change old_dir and new_dir in struct renamedata to
 [PATCH 2/2] fs/proc: take rcu_read_lock() in proc_sys_compare()

[PATCH 1/2] VFS: change old_dir and new_dir in struct renamedata to dentrys

[NeilBrown]

all users of 'struct renamedata' have the dentry for the old and new
directories, and often have no use for the inode except to store it in
the renamedata.

This patch changes struct renamedata to hold the dentry, rather than
the inode, for the old and new directories, and changes callers to
match.

This results in the removal of several local variables and several
dereferences of ->d_inode at the cost of adding ->d_inode dereferences
to vfs_rename().

Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/cachefiles/namei.c    |  4 ++--
 fs/ecryptfs/inode.c      |  4 ++--
 fs/namei.c               |  6 +++---
 fs/nfsd/vfs.c            |  7 ++-----
 fs/overlayfs/copy_up.c   |  6 +++---
 fs/overlayfs/dir.c       | 16 ++++++++--------
 fs/overlayfs/overlayfs.h |  6 +++---
 fs/overlayfs/readdir.c   |  2 +-
 fs/overlayfs/super.c     |  2 +-
 fs/overlayfs/util.c      |  2 +-
 fs/smb/server/vfs.c      |  4 ++--
 include/linux/fs.h       |  4 ++--
 12 files changed, 30 insertions(+), 33 deletions(-)

diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index aecfc5c37b49..053fc28b5423 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -388,10 +388,10 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
 	} else {
 		struct renamedata rd = {
 			.old_mnt_idmap	= &nop_mnt_idmap,
-			.old_dir	= d_inode(dir),
+			.old_dir	= dir,
 			.old_dentry	= rep,
 			.new_mnt_idmap	= &nop_mnt_idmap,
-			.new_dir	= d_inode(cache->graveyard),
+			.new_dir	= cache->graveyard,
 			.new_dentry	= grave,
 		};
 		trace_cachefiles_rename(object, d_inode(rep)->i_ino, why);
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 493d7f194956..c9fec8b7e000 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -635,10 +635,10 @@ ecryptfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 	}
 
 	rd.old_mnt_idmap	= &nop_mnt_idmap;
-	rd.old_dir		= d_inode(lower_old_dir_dentry);
+	rd.old_dir		= lower_old_dir_dentry;
 	rd.old_dentry		= lower_old_dentry;
 	rd.new_mnt_idmap	= &nop_mnt_idmap;
-	rd.new_dir		= d_inode(lower_new_dir_dentry);
+	rd.new_dir		= lower_new_dir_dentry;
 	rd.new_dentry		= lower_new_dentry;
 	rc = vfs_rename(&rd);
 	if (rc)
diff --git a/fs/namei.c b/fs/namei.c
index 019073162b8a..5b0be8bca50d 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -5007,7 +5007,7 @@ SYSCALL_DEFINE2(link, const char __user *, oldname, const char __user *, newname
 int vfs_rename(struct renamedata *rd)
 {
 	int error;
-	struct inode *old_dir = rd->old_dir, *new_dir = rd->new_dir;
+	struct inode *old_dir = d_inode(rd->old_dir), *new_dir = d_inode(rd->new_dir);
 	struct dentry *old_dentry = rd->old_dentry;
 	struct dentry *new_dentry = rd->new_dentry;
 	struct inode **delegated_inode = rd->delegated_inode;
@@ -5266,10 +5266,10 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
 	if (error)
 		goto exit5;
 
-	rd.old_dir	   = old_path.dentry->d_inode;
+	rd.old_dir	   = old_path.dentry;
 	rd.old_dentry	   = old_dentry;
 	rd.old_mnt_idmap   = mnt_idmap(old_path.mnt);
-	rd.new_dir	   = new_path.dentry->d_inode;
+	rd.new_dir	   = new_path.dentry;
 	rd.new_dentry	   = new_dentry;
 	rd.new_mnt_idmap   = mnt_idmap(new_path.mnt);
 	rd.delegated_inode = &delegated_inode;
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index cd689df2ca5d..3c87fbd22c57 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1864,7 +1864,6 @@ 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 inode	*fdir, *tdir;
 	int		type = S_IFDIR;
 	__be32		err;
 	int		host_err;
@@ -1880,10 +1879,8 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
 		goto out;
 
 	fdentry = ffhp->fh_dentry;
-	fdir = d_inode(fdentry);
 
 	tdentry = tfhp->fh_dentry;
-	tdir = d_inode(tdentry);
 
 	err = nfserr_perm;
 	if (!flen || isdotent(fname, flen) || !tlen || isdotent(tname, tlen))
@@ -1944,10 +1941,10 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
 	} else {
 		struct renamedata rd = {
 			.old_mnt_idmap	= &nop_mnt_idmap,
-			.old_dir	= fdir,
+			.old_dir	= fdentry,
 			.old_dentry	= odentry,
 			.new_mnt_idmap	= &nop_mnt_idmap,
-			.new_dir	= tdir,
+			.new_dir	= tdentry,
 			.new_dentry	= ndentry,
 		};
 		int retries;
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index d7310fcf3888..8a3c0d18ec2e 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -563,7 +563,7 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
 	if (IS_ERR(index)) {
 		err = PTR_ERR(index);
 	} else {
-		err = ovl_do_rename(ofs, dir, temp, dir, index, 0);
+		err = ovl_do_rename(ofs, indexdir, temp, indexdir, index, 0);
 		dput(index);
 	}
 out:
@@ -762,7 +762,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
 {
 	struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
 	struct inode *inode;
-	struct inode *udir = d_inode(c->destdir), *wdir = d_inode(c->workdir);
+	struct inode *wdir = d_inode(c->workdir);
 	struct path path = { .mnt = ovl_upper_mnt(ofs) };
 	struct dentry *temp, *upper, *trap;
 	struct ovl_cu_creds cc;
@@ -829,7 +829,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
 	if (IS_ERR(upper))
 		goto cleanup;
 
-	err = ovl_do_rename(ofs, wdir, temp, udir, upper, 0);
+	err = ovl_do_rename(ofs, c->workdir, temp, c->destdir, upper, 0);
 	dput(upper);
 	if (err)
 		goto cleanup;
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index fe493f3ed6b6..4fc221ea6480 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -107,7 +107,7 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs)
 }
 
 /* Caller must hold i_mutex on both workdir and dir */
-int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct inode *dir,
+int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir,
 			     struct dentry *dentry)
 {
 	struct inode *wdir = ofs->workdir->d_inode;
@@ -123,7 +123,7 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct inode *dir,
 	if (d_is_dir(dentry))
 		flags = RENAME_EXCHANGE;
 
-	err = ovl_do_rename(ofs, wdir, whiteout, dir, dentry, flags);
+	err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, dentry, flags);
 	if (err)
 		goto kill_whiteout;
 	if (flags)
@@ -384,7 +384,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
 	if (err)
 		goto out_cleanup;
 
-	err = ovl_do_rename(ofs, wdir, opaquedir, udir, upper, RENAME_EXCHANGE);
+	err = ovl_do_rename(ofs, workdir, opaquedir, upperdir, upper, RENAME_EXCHANGE);
 	if (err)
 		goto out_cleanup;
 
@@ -491,14 +491,14 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
 		if (err)
 			goto out_cleanup;
 
-		err = ovl_do_rename(ofs, wdir, newdentry, udir, upper,
+		err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper,
 				    RENAME_EXCHANGE);
 		if (err)
 			goto out_cleanup;
 
 		ovl_cleanup(ofs, wdir, upper);
 	} else {
-		err = ovl_do_rename(ofs, wdir, newdentry, udir, upper, 0);
+		err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper, 0);
 		if (err)
 			goto out_cleanup;
 	}
@@ -774,7 +774,7 @@ static int ovl_remove_and_whiteout(struct dentry *dentry,
 		goto out_dput_upper;
 	}
 
-	err = ovl_cleanup_and_whiteout(ofs, d_inode(upperdir), upper);
+	err = ovl_cleanup_and_whiteout(ofs, upperdir, upper);
 	if (err)
 		goto out_d_drop;
 
@@ -1246,8 +1246,8 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	if (err)
 		goto out_dput;
 
-	err = ovl_do_rename(ofs, old_upperdir->d_inode, olddentry,
-			    new_upperdir->d_inode, newdentry, flags);
+	err = ovl_do_rename(ofs, old_upperdir, olddentry,
+			    new_upperdir, newdentry, flags);
 	if (err)
 		goto out_dput;
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 8baaba0a3fe5..65f9d51bed7c 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -353,8 +353,8 @@ 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(struct ovl_fs *ofs, struct inode *olddir,
-				struct dentry *olddentry, struct inode *newdir,
+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;
@@ -826,7 +826,7 @@ static inline void ovl_copyflags(struct inode *from, struct inode *to)
 
 /* dir.c */
 extern const struct inode_operations ovl_dir_inode_operations;
-int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct inode *dir,
+int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir,
 			     struct dentry *dentry);
 struct ovl_cattr {
 	dev_t rdev;
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 474c80d210d1..68cca52ae2ac 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -1235,7 +1235,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs)
 			 * Whiteout orphan index to block future open by
 			 * handle after overlay nlink dropped to zero.
 			 */
-			err = ovl_cleanup_and_whiteout(ofs, dir, index);
+			err = ovl_cleanup_and_whiteout(ofs, indexdir, index);
 		} else {
 			/* Cleanup orphan index entries */
 			err = ovl_cleanup(ofs, dir, index);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index e19940d649ca..cf99b276fdfb 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -580,7 +580,7 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
 
 	/* Name is inline and stable - using snapshot as a copy helper */
 	take_dentry_name_snapshot(&name, temp);
-	err = ovl_do_rename(ofs, dir, temp, dir, dest, RENAME_WHITEOUT);
+	err = ovl_do_rename(ofs, workdir, temp, workdir, dest, RENAME_WHITEOUT);
 	if (err) {
 		if (err == -EINVAL)
 			err = 0;
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index dcccb4b4a66c..2b4754c645ee 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1115,7 +1115,7 @@ static void ovl_cleanup_index(struct dentry *dentry)
 	} else if (ovl_index_all(dentry->d_sb)) {
 		/* Whiteout orphan index to block future open by handle */
 		err = ovl_cleanup_and_whiteout(OVL_FS(dentry->d_sb),
-					       dir, index);
+					       indexdir, index);
 	} else {
 		/* Cleanup orphan index entries */
 		err = ovl_cleanup(ofs, dir, index);
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index ba45e809555a..b8d913c61623 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -764,10 +764,10 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
 	}
 
 	rd.old_mnt_idmap	= mnt_idmap(old_path->mnt),
-	rd.old_dir		= d_inode(old_parent),
+	rd.old_dir		= old_parent,
 	rd.old_dentry		= old_child,
 	rd.new_mnt_idmap	= mnt_idmap(new_path.mnt),
-	rd.new_dir		= new_path.dentry->d_inode,
+	rd.new_dir		= new_path.dentry,
 	rd.new_dentry		= new_dentry,
 	rd.flags		= flags,
 	rd.delegated_inode	= NULL,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 16f40a6f8264..9a83904c9d4a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2016,10 +2016,10 @@ int vfs_unlink(struct mnt_idmap *, struct inode *, struct dentry *,
  */
 struct renamedata {
 	struct mnt_idmap *old_mnt_idmap;
-	struct inode *old_dir;
+	struct dentry *old_dir;
 	struct dentry *old_dentry;
 	struct mnt_idmap *new_mnt_idmap;
-	struct inode *new_dir;
+	struct dentry *new_dir;
 	struct dentry *new_dentry;
 	struct inode **delegated_inode;
 	unsigned int flags;
-- 
2.49.0

[PATCH 2/2] fs/proc: take rcu_read_lock() in proc_sys_compare()

[NeilBrown]

proc_sys_compare() is the ->d_compare function for /proc/sys.
It uses rcu_dereference() which assumes the RCU read lock is held and
can complain if it isn't.

However there is no guarantee that this lock is held by d_same_name()
(the caller of ->d_compare).  In particularly d_alloc_parallel() calls
d_same_name() after rcu_read_unlock().

So this patch calls rcu_read_lock() before accessing the inode (which
seems to be the focus of RCU protection here), and drops it afterwards.

Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/proc/proc_sysctl.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index cc9d74a06ff0..a4cdc0a189ef 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -917,19 +917,23 @@ static int proc_sys_compare(const struct dentry *dentry,
 {
 	struct ctl_table_header *head;
 	struct inode *inode;
+	int ret;
 
 	/* Although proc doesn't have negative dentries, rcu-walk means
 	 * that inode here can be NULL */
 	/* AV: can it, indeed? */
+	rcu_read_lock();
 	inode = d_inode_rcu(dentry);
-	if (!inode)
-		return 1;
-	if (name->len != len)
-		return 1;
-	if (memcmp(name->name, str, len))
-		return 1;
-	head = rcu_dereference(PROC_I(inode)->sysctl);
-	return !head || !sysctl_is_seen(head);
+	if (!inode ||
+	    name->len != len ||
+	    memcmp(name->name, str, len)) {
+		ret = 1;
+	} else {
+		head = rcu_dereference(PROC_I(inode)->sysctl);
+		ret = !head || !sysctl_is_seen(head);
+	}
+	rcu_read_unlock();
+	return ret;
 }
 
 static const struct dentry_operations proc_sys_dentry_operations = {
-- 
2.49.0

Re: [PATCH 2/2] fs/proc: take rcu_read_lock() in proc_sys_compare()

[Al Viro]

On Thu, Jun 12, 2025 at 08:57:03AM +1000, NeilBrown wrote:

> However there is no guarantee that this lock is held by d_same_name()
> (the caller of ->d_compare).  In particularly d_alloc_parallel() calls
> d_same_name() after rcu_read_unlock().

d_alloc_parallel() calls d_same_name() with dentry being pinned;
if it's positive, nothing's going to happen to its inode,
rcu_read_lock() or not.  It can go from negative to positive,
but that's it.

Why is it needed?  We do care about possibly NULL inode (basically,
when RCU dcache lookup runs into a dentry getting evicted right
under it), but that's not relevant here.

Re: [PATCH 1/2] VFS: change old_dir and new_dir in struct renamedata to dentrys

[Al Viro]

On Thu, Jun 12, 2025 at 08:57:02AM +1000, NeilBrown wrote:
> all users of 'struct renamedata' have the dentry for the old and new
> directories, and often have no use for the inode except to store it in
> the renamedata.
> 
> This patch changes struct renamedata to hold the dentry, rather than
> the inode, for the old and new directories, and changes callers to
> match.
> 
> This results in the removal of several local variables and several
> dereferences of ->d_inode at the cost of adding ->d_inode dereferences
> to vfs_rename().

Umm...  No objections, as long as overlayfs part is correct; it seems
to be, but I hadn't checked every chunk there...

Re: [PATCH 2/2] fs/proc: take rcu_read_lock() in proc_sys_compare()

[NeilBrown]

On Thu, 12 Jun 2025, Al Viro wrote:
> On Thu, Jun 12, 2025 at 08:57:03AM +1000, NeilBrown wrote:
> 
> > However there is no guarantee that this lock is held by d_same_name()
> > (the caller of ->d_compare).  In particularly d_alloc_parallel() calls
> > d_same_name() after rcu_read_unlock().
> 
> d_alloc_parallel() calls d_same_name() with dentry being pinned;
> if it's positive, nothing's going to happen to its inode,
> rcu_read_lock() or not.  It can go from negative to positive,
> but that's it.
> 
> Why is it needed?  We do care about possibly NULL inode (basically,
> when RCU dcache lookup runs into a dentry getting evicted right
> under it), but that's not relevant here.
> 

Maybe it isn't needed.  Maybe I could fix the warning by removing the
rcu_dereference() (and the RCU_INIT_POINTER() in inode.c).  But then I
might have to pretend that I understand the code - and it makes no
sense.

If a second d_alloc_parallel() is called while there is already a
d_in_lookup() dentry, then ->d_compare will return 1 so a second
d_in_lookup() will be created and ->lookup will be called twice
(possibly concurrently) and both will be added to the dcache.  Probably
not harmful but not really wanted.

And I'm having trouble seeing how sysctl_is_seen() is useful.  If it
reports that the sysctl is not visible to this process, it'll just
create a new dentry/inode which is that same as any other that would be
created... 

NeilBrown

Re: [PATCH 1/2] VFS: change old_dir and new_dir in struct renamedata to dentrys

[Miklos Szeredi]

On Thu, 12 Jun 2025 at 01:38, Al Viro <viro@zeniv.linux.org.uk> wrote:

> Umm...  No objections, as long as overlayfs part is correct; it seems
> to be, but I hadn't checked every chunk there...

Overlayfs parts looks okay too.

A followup would be nice (e.g. make ovl_cleanup() take a dentry for
the directory as well, etc) so that there's no need to have local
variables for both the inode and dentry of the directory.

Thanks,
Miklos

Re: [PATCH 1/2] VFS: change old_dir and new_dir in struct renamedata to dentrys

[Amir Goldstein]

On Thu, Jun 12, 2025 at 12:59 AM NeilBrown <neil@brown.name> wrote:
>
> all users of 'struct renamedata' have the dentry for the old and new
> directories, and often have no use for the inode except to store it in
> the renamedata.
>
> This patch changes struct renamedata to hold the dentry, rather than
> the inode, for the old and new directories, and changes callers to
> match.
>
> This results in the removal of several local variables and several
> dereferences of ->d_inode at the cost of adding ->d_inode dereferences
> to vfs_rename().
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>  fs/cachefiles/namei.c    |  4 ++--
>  fs/ecryptfs/inode.c      |  4 ++--
>  fs/namei.c               |  6 +++---
>  fs/nfsd/vfs.c            |  7 ++-----
>  fs/overlayfs/copy_up.c   |  6 +++---
>  fs/overlayfs/dir.c       | 16 ++++++++--------
>  fs/overlayfs/overlayfs.h |  6 +++---
>  fs/overlayfs/readdir.c   |  2 +-
>  fs/overlayfs/super.c     |  2 +-
>  fs/overlayfs/util.c      |  2 +-
>  fs/smb/server/vfs.c      |  4 ++--
>  include/linux/fs.h       |  4 ++--
>  12 files changed, 30 insertions(+), 33 deletions(-)
>
> diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> index aecfc5c37b49..053fc28b5423 100644
> --- a/fs/cachefiles/namei.c
> +++ b/fs/cachefiles/namei.c
> @@ -388,10 +388,10 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
>         } else {
>                 struct renamedata rd = {
>                         .old_mnt_idmap  = &nop_mnt_idmap,
> -                       .old_dir        = d_inode(dir),
> +                       .old_dir        = dir,
>                         .old_dentry     = rep,
>                         .new_mnt_idmap  = &nop_mnt_idmap,
> -                       .new_dir        = d_inode(cache->graveyard),
> +                       .new_dir        = cache->graveyard,
>                         .new_dentry     = grave,
>                 };
>                 trace_cachefiles_rename(object, d_inode(rep)->i_ino, why);
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 493d7f194956..c9fec8b7e000 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -635,10 +635,10 @@ ecryptfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>         }
>
>         rd.old_mnt_idmap        = &nop_mnt_idmap;
> -       rd.old_dir              = d_inode(lower_old_dir_dentry);
> +       rd.old_dir              = lower_old_dir_dentry;
>         rd.old_dentry           = lower_old_dentry;
>         rd.new_mnt_idmap        = &nop_mnt_idmap;
> -       rd.new_dir              = d_inode(lower_new_dir_dentry);
> +       rd.new_dir              = lower_new_dir_dentry;
>         rd.new_dentry           = lower_new_dentry;
>         rc = vfs_rename(&rd);
>         if (rc)
> diff --git a/fs/namei.c b/fs/namei.c
> index 019073162b8a..5b0be8bca50d 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -5007,7 +5007,7 @@ SYSCALL_DEFINE2(link, const char __user *, oldname, const char __user *, newname
>  int vfs_rename(struct renamedata *rd)
>  {
>         int error;
> -       struct inode *old_dir = rd->old_dir, *new_dir = rd->new_dir;
> +       struct inode *old_dir = d_inode(rd->old_dir), *new_dir = d_inode(rd->new_dir);
>         struct dentry *old_dentry = rd->old_dentry;
>         struct dentry *new_dentry = rd->new_dentry;
>         struct inode **delegated_inode = rd->delegated_inode;
> @@ -5266,10 +5266,10 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
>         if (error)
>                 goto exit5;
>
> -       rd.old_dir         = old_path.dentry->d_inode;
> +       rd.old_dir         = old_path.dentry;
>         rd.old_dentry      = old_dentry;
>         rd.old_mnt_idmap   = mnt_idmap(old_path.mnt);
> -       rd.new_dir         = new_path.dentry->d_inode;
> +       rd.new_dir         = new_path.dentry;
>         rd.new_dentry      = new_dentry;
>         rd.new_mnt_idmap   = mnt_idmap(new_path.mnt);
>         rd.delegated_inode = &delegated_inode;
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index cd689df2ca5d..3c87fbd22c57 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1864,7 +1864,6 @@ 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 inode    *fdir, *tdir;
>         int             type = S_IFDIR;
>         __be32          err;
>         int             host_err;
> @@ -1880,10 +1879,8 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
>                 goto out;
>
>         fdentry = ffhp->fh_dentry;
> -       fdir = d_inode(fdentry);
>
>         tdentry = tfhp->fh_dentry;
> -       tdir = d_inode(tdentry);
>
>         err = nfserr_perm;
>         if (!flen || isdotent(fname, flen) || !tlen || isdotent(tname, tlen))
> @@ -1944,10 +1941,10 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
>         } else {
>                 struct renamedata rd = {
>                         .old_mnt_idmap  = &nop_mnt_idmap,
> -                       .old_dir        = fdir,
> +                       .old_dir        = fdentry,
>                         .old_dentry     = odentry,
>                         .new_mnt_idmap  = &nop_mnt_idmap,
> -                       .new_dir        = tdir,
> +                       .new_dir        = tdentry,
>                         .new_dentry     = ndentry,
>                 };
>                 int retries;
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index d7310fcf3888..8a3c0d18ec2e 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -563,7 +563,7 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
>         if (IS_ERR(index)) {
>                 err = PTR_ERR(index);
>         } else {
> -               err = ovl_do_rename(ofs, dir, temp, dir, index, 0);
> +               err = ovl_do_rename(ofs, indexdir, temp, indexdir, index, 0);
>                 dput(index);
>         }
>  out:
> @@ -762,7 +762,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
>  {
>         struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
>         struct inode *inode;
> -       struct inode *udir = d_inode(c->destdir), *wdir = d_inode(c->workdir);
> +       struct inode *wdir = d_inode(c->workdir);
>         struct path path = { .mnt = ovl_upper_mnt(ofs) };
>         struct dentry *temp, *upper, *trap;
>         struct ovl_cu_creds cc;
> @@ -829,7 +829,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
>         if (IS_ERR(upper))
>                 goto cleanup;
>
> -       err = ovl_do_rename(ofs, wdir, temp, udir, upper, 0);
> +       err = ovl_do_rename(ofs, c->workdir, temp, c->destdir, upper, 0);
>         dput(upper);
>         if (err)
>                 goto cleanup;
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index fe493f3ed6b6..4fc221ea6480 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -107,7 +107,7 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs)
>  }
>
>  /* Caller must hold i_mutex on both workdir and dir */
> -int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct inode *dir,
> +int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir,
>                              struct dentry *dentry)
>  {
>         struct inode *wdir = ofs->workdir->d_inode;
> @@ -123,7 +123,7 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct inode *dir,
>         if (d_is_dir(dentry))
>                 flags = RENAME_EXCHANGE;
>
> -       err = ovl_do_rename(ofs, wdir, whiteout, dir, dentry, flags);
> +       err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, dentry, flags);
>         if (err)
>                 goto kill_whiteout;
>         if (flags)
> @@ -384,7 +384,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
>         if (err)
>                 goto out_cleanup;
>
> -       err = ovl_do_rename(ofs, wdir, opaquedir, udir, upper, RENAME_EXCHANGE);
> +       err = ovl_do_rename(ofs, workdir, opaquedir, upperdir, upper, RENAME_EXCHANGE);
>         if (err)
>                 goto out_cleanup;
>
> @@ -491,14 +491,14 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
>                 if (err)
>                         goto out_cleanup;
>
> -               err = ovl_do_rename(ofs, wdir, newdentry, udir, upper,
> +               err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper,
>                                     RENAME_EXCHANGE);
>                 if (err)
>                         goto out_cleanup;
>
>                 ovl_cleanup(ofs, wdir, upper);
>         } else {
> -               err = ovl_do_rename(ofs, wdir, newdentry, udir, upper, 0);
> +               err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper, 0);
>                 if (err)
>                         goto out_cleanup;
>         }
> @@ -774,7 +774,7 @@ static int ovl_remove_and_whiteout(struct dentry *dentry,
>                 goto out_dput_upper;
>         }
>
> -       err = ovl_cleanup_and_whiteout(ofs, d_inode(upperdir), upper);
> +       err = ovl_cleanup_and_whiteout(ofs, upperdir, upper);
>         if (err)
>                 goto out_d_drop;
>
> @@ -1246,8 +1246,8 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>         if (err)
>                 goto out_dput;
>
> -       err = ovl_do_rename(ofs, old_upperdir->d_inode, olddentry,
> -                           new_upperdir->d_inode, newdentry, flags);
> +       err = ovl_do_rename(ofs, old_upperdir, olddentry,
> +                           new_upperdir, newdentry, flags);
>         if (err)
>                 goto out_dput;
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 8baaba0a3fe5..65f9d51bed7c 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -353,8 +353,8 @@ 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(struct ovl_fs *ofs, struct inode *olddir,
> -                               struct dentry *olddentry, struct inode *newdir,
> +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;
> @@ -826,7 +826,7 @@ static inline void ovl_copyflags(struct inode *from, struct inode *to)
>
>  /* dir.c */
>  extern const struct inode_operations ovl_dir_inode_operations;
> -int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct inode *dir,
> +int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir,
>                              struct dentry *dentry);
>  struct ovl_cattr {
>         dev_t rdev;
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index 474c80d210d1..68cca52ae2ac 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -1235,7 +1235,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs)
>                          * Whiteout orphan index to block future open by
>                          * handle after overlay nlink dropped to zero.
>                          */
> -                       err = ovl_cleanup_and_whiteout(ofs, dir, index);
> +                       err = ovl_cleanup_and_whiteout(ofs, indexdir, index);
>                 } else {
>                         /* Cleanup orphan index entries */
>                         err = ovl_cleanup(ofs, dir, index);
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index e19940d649ca..cf99b276fdfb 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -580,7 +580,7 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
>
>         /* Name is inline and stable - using snapshot as a copy helper */
>         take_dentry_name_snapshot(&name, temp);
> -       err = ovl_do_rename(ofs, dir, temp, dir, dest, RENAME_WHITEOUT);
> +       err = ovl_do_rename(ofs, workdir, temp, workdir, dest, RENAME_WHITEOUT);
>         if (err) {
>                 if (err == -EINVAL)
>                         err = 0;
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index dcccb4b4a66c..2b4754c645ee 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -1115,7 +1115,7 @@ static void ovl_cleanup_index(struct dentry *dentry)
>         } else if (ovl_index_all(dentry->d_sb)) {
>                 /* Whiteout orphan index to block future open by handle */
>                 err = ovl_cleanup_and_whiteout(OVL_FS(dentry->d_sb),
> -                                              dir, index);
> +                                              indexdir, index);
>         } else {
>                 /* Cleanup orphan index entries */
>                 err = ovl_cleanup(ofs, dir, index);
> diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> index ba45e809555a..b8d913c61623 100644
> --- a/fs/smb/server/vfs.c
> +++ b/fs/smb/server/vfs.c
> @@ -764,10 +764,10 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
>         }
>
>         rd.old_mnt_idmap        = mnt_idmap(old_path->mnt),
> -       rd.old_dir              = d_inode(old_parent),
> +       rd.old_dir              = old_parent,
>         rd.old_dentry           = old_child,
>         rd.new_mnt_idmap        = mnt_idmap(new_path.mnt),
> -       rd.new_dir              = new_path.dentry->d_inode,
> +       rd.new_dir              = new_path.dentry,
>         rd.new_dentry           = new_dentry,
>         rd.flags                = flags,
>         rd.delegated_inode      = NULL,
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 16f40a6f8264..9a83904c9d4a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2016,10 +2016,10 @@ int vfs_unlink(struct mnt_idmap *, struct inode *, struct dentry *,
>   */
>  struct renamedata {
>         struct mnt_idmap *old_mnt_idmap;
> -       struct inode *old_dir;
> +       struct dentry *old_dir;
>         struct dentry *old_dentry;
>         struct mnt_idmap *new_mnt_idmap;
> -       struct inode *new_dir;
> +       struct dentry *new_dir;
>         struct dentry *new_dentry;
>         struct inode **delegated_inode;
>         unsigned int flags;
> --

It bothers me a bit that we are keeping the field name while changing its type.

There is a wide convention in vfs methods and helpers of using
struct inode *dir
as the parent directory inode
and often (but not always) using
struct dentry *parent
as the parent dentry

How do you feel about making struct renamedata use:

struct dentry *old_parent;
struct dentry *new_parent;

I don't think it will add any churn beyond what this patch already does.

Thanks,
Amir.

Re: [PATCH 1/2] VFS: change old_dir and new_dir in struct renamedata to dentrys

[Chuck Lever]

On 6/11/25 6:57 PM, NeilBrown wrote:
> all users of 'struct renamedata' have the dentry for the old and new
> directories, and often have no use for the inode except to store it in
> the renamedata.
> 
> This patch changes struct renamedata to hold the dentry, rather than
> the inode, for the old and new directories, and changes callers to
> match.
> 
> This results in the removal of several local variables and several
> dereferences of ->d_inode at the cost of adding ->d_inode dereferences
> to vfs_rename().
> 
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>  fs/cachefiles/namei.c    |  4 ++--
>  fs/ecryptfs/inode.c      |  4 ++--
>  fs/namei.c               |  6 +++---
>  fs/nfsd/vfs.c            |  7 ++-----
>  fs/overlayfs/copy_up.c   |  6 +++---
>  fs/overlayfs/dir.c       | 16 ++++++++--------
>  fs/overlayfs/overlayfs.h |  6 +++---
>  fs/overlayfs/readdir.c   |  2 +-
>  fs/overlayfs/super.c     |  2 +-
>  fs/overlayfs/util.c      |  2 +-
>  fs/smb/server/vfs.c      |  4 ++--
>  include/linux/fs.h       |  4 ++--
>  12 files changed, 30 insertions(+), 33 deletions(-)
> 
> diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> index aecfc5c37b49..053fc28b5423 100644
> --- a/fs/cachefiles/namei.c
> +++ b/fs/cachefiles/namei.c
> @@ -388,10 +388,10 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
>  	} else {
>  		struct renamedata rd = {
>  			.old_mnt_idmap	= &nop_mnt_idmap,
> -			.old_dir	= d_inode(dir),
> +			.old_dir	= dir,
>  			.old_dentry	= rep,
>  			.new_mnt_idmap	= &nop_mnt_idmap,
> -			.new_dir	= d_inode(cache->graveyard),
> +			.new_dir	= cache->graveyard,
>  			.new_dentry	= grave,
>  		};
>  		trace_cachefiles_rename(object, d_inode(rep)->i_ino, why);
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 493d7f194956..c9fec8b7e000 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -635,10 +635,10 @@ ecryptfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>  	}
>  
>  	rd.old_mnt_idmap	= &nop_mnt_idmap;
> -	rd.old_dir		= d_inode(lower_old_dir_dentry);
> +	rd.old_dir		= lower_old_dir_dentry;
>  	rd.old_dentry		= lower_old_dentry;
>  	rd.new_mnt_idmap	= &nop_mnt_idmap;
> -	rd.new_dir		= d_inode(lower_new_dir_dentry);
> +	rd.new_dir		= lower_new_dir_dentry;
>  	rd.new_dentry		= lower_new_dentry;
>  	rc = vfs_rename(&rd);
>  	if (rc)
> diff --git a/fs/namei.c b/fs/namei.c
> index 019073162b8a..5b0be8bca50d 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -5007,7 +5007,7 @@ SYSCALL_DEFINE2(link, const char __user *, oldname, const char __user *, newname
>  int vfs_rename(struct renamedata *rd)
>  {
>  	int error;
> -	struct inode *old_dir = rd->old_dir, *new_dir = rd->new_dir;
> +	struct inode *old_dir = d_inode(rd->old_dir), *new_dir = d_inode(rd->new_dir);
>  	struct dentry *old_dentry = rd->old_dentry;
>  	struct dentry *new_dentry = rd->new_dentry;
>  	struct inode **delegated_inode = rd->delegated_inode;
> @@ -5266,10 +5266,10 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
>  	if (error)
>  		goto exit5;
>  
> -	rd.old_dir	   = old_path.dentry->d_inode;
> +	rd.old_dir	   = old_path.dentry;
>  	rd.old_dentry	   = old_dentry;
>  	rd.old_mnt_idmap   = mnt_idmap(old_path.mnt);
> -	rd.new_dir	   = new_path.dentry->d_inode;
> +	rd.new_dir	   = new_path.dentry;
>  	rd.new_dentry	   = new_dentry;
>  	rd.new_mnt_idmap   = mnt_idmap(new_path.mnt);
>  	rd.delegated_inode = &delegated_inode;
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index cd689df2ca5d..3c87fbd22c57 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1864,7 +1864,6 @@ 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 inode	*fdir, *tdir;
>  	int		type = S_IFDIR;
>  	__be32		err;
>  	int		host_err;
> @@ -1880,10 +1879,8 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
>  		goto out;
>  
>  	fdentry = ffhp->fh_dentry;
> -	fdir = d_inode(fdentry);
>  
>  	tdentry = tfhp->fh_dentry;
> -	tdir = d_inode(tdentry);
>  
>  	err = nfserr_perm;
>  	if (!flen || isdotent(fname, flen) || !tlen || isdotent(tname, tlen))
> @@ -1944,10 +1941,10 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
>  	} else {
>  		struct renamedata rd = {
>  			.old_mnt_idmap	= &nop_mnt_idmap,
> -			.old_dir	= fdir,
> +			.old_dir	= fdentry,
>  			.old_dentry	= odentry,
>  			.new_mnt_idmap	= &nop_mnt_idmap,
> -			.new_dir	= tdir,
> +			.new_dir	= tdentry,
>  			.new_dentry	= ndentry,
>  		};
>  		int retries;
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index d7310fcf3888..8a3c0d18ec2e 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -563,7 +563,7 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
>  	if (IS_ERR(index)) {
>  		err = PTR_ERR(index);
>  	} else {
> -		err = ovl_do_rename(ofs, dir, temp, dir, index, 0);
> +		err = ovl_do_rename(ofs, indexdir, temp, indexdir, index, 0);
>  		dput(index);
>  	}
>  out:
> @@ -762,7 +762,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
>  {
>  	struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
>  	struct inode *inode;
> -	struct inode *udir = d_inode(c->destdir), *wdir = d_inode(c->workdir);
> +	struct inode *wdir = d_inode(c->workdir);
>  	struct path path = { .mnt = ovl_upper_mnt(ofs) };
>  	struct dentry *temp, *upper, *trap;
>  	struct ovl_cu_creds cc;
> @@ -829,7 +829,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
>  	if (IS_ERR(upper))
>  		goto cleanup;
>  
> -	err = ovl_do_rename(ofs, wdir, temp, udir, upper, 0);
> +	err = ovl_do_rename(ofs, c->workdir, temp, c->destdir, upper, 0);
>  	dput(upper);
>  	if (err)
>  		goto cleanup;
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index fe493f3ed6b6..4fc221ea6480 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -107,7 +107,7 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs)
>  }
>  
>  /* Caller must hold i_mutex on both workdir and dir */
> -int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct inode *dir,
> +int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir,
>  			     struct dentry *dentry)
>  {
>  	struct inode *wdir = ofs->workdir->d_inode;
> @@ -123,7 +123,7 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct inode *dir,
>  	if (d_is_dir(dentry))
>  		flags = RENAME_EXCHANGE;
>  
> -	err = ovl_do_rename(ofs, wdir, whiteout, dir, dentry, flags);
> +	err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, dentry, flags);
>  	if (err)
>  		goto kill_whiteout;
>  	if (flags)
> @@ -384,7 +384,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
>  	if (err)
>  		goto out_cleanup;
>  
> -	err = ovl_do_rename(ofs, wdir, opaquedir, udir, upper, RENAME_EXCHANGE);
> +	err = ovl_do_rename(ofs, workdir, opaquedir, upperdir, upper, RENAME_EXCHANGE);
>  	if (err)
>  		goto out_cleanup;
>  
> @@ -491,14 +491,14 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
>  		if (err)
>  			goto out_cleanup;
>  
> -		err = ovl_do_rename(ofs, wdir, newdentry, udir, upper,
> +		err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper,
>  				    RENAME_EXCHANGE);
>  		if (err)
>  			goto out_cleanup;
>  
>  		ovl_cleanup(ofs, wdir, upper);
>  	} else {
> -		err = ovl_do_rename(ofs, wdir, newdentry, udir, upper, 0);
> +		err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper, 0);
>  		if (err)
>  			goto out_cleanup;
>  	}
> @@ -774,7 +774,7 @@ static int ovl_remove_and_whiteout(struct dentry *dentry,
>  		goto out_dput_upper;
>  	}
>  
> -	err = ovl_cleanup_and_whiteout(ofs, d_inode(upperdir), upper);
> +	err = ovl_cleanup_and_whiteout(ofs, upperdir, upper);
>  	if (err)
>  		goto out_d_drop;
>  
> @@ -1246,8 +1246,8 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>  	if (err)
>  		goto out_dput;
>  
> -	err = ovl_do_rename(ofs, old_upperdir->d_inode, olddentry,
> -			    new_upperdir->d_inode, newdentry, flags);
> +	err = ovl_do_rename(ofs, old_upperdir, olddentry,
> +			    new_upperdir, newdentry, flags);
>  	if (err)
>  		goto out_dput;
>  
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 8baaba0a3fe5..65f9d51bed7c 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -353,8 +353,8 @@ 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(struct ovl_fs *ofs, struct inode *olddir,
> -				struct dentry *olddentry, struct inode *newdir,
> +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;
> @@ -826,7 +826,7 @@ static inline void ovl_copyflags(struct inode *from, struct inode *to)
>  
>  /* dir.c */
>  extern const struct inode_operations ovl_dir_inode_operations;
> -int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct inode *dir,
> +int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir,
>  			     struct dentry *dentry);
>  struct ovl_cattr {
>  	dev_t rdev;
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index 474c80d210d1..68cca52ae2ac 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -1235,7 +1235,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs)
>  			 * Whiteout orphan index to block future open by
>  			 * handle after overlay nlink dropped to zero.
>  			 */
> -			err = ovl_cleanup_and_whiteout(ofs, dir, index);
> +			err = ovl_cleanup_and_whiteout(ofs, indexdir, index);
>  		} else {
>  			/* Cleanup orphan index entries */
>  			err = ovl_cleanup(ofs, dir, index);
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index e19940d649ca..cf99b276fdfb 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -580,7 +580,7 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
>  
>  	/* Name is inline and stable - using snapshot as a copy helper */
>  	take_dentry_name_snapshot(&name, temp);
> -	err = ovl_do_rename(ofs, dir, temp, dir, dest, RENAME_WHITEOUT);
> +	err = ovl_do_rename(ofs, workdir, temp, workdir, dest, RENAME_WHITEOUT);
>  	if (err) {
>  		if (err == -EINVAL)
>  			err = 0;
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index dcccb4b4a66c..2b4754c645ee 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -1115,7 +1115,7 @@ static void ovl_cleanup_index(struct dentry *dentry)
>  	} else if (ovl_index_all(dentry->d_sb)) {
>  		/* Whiteout orphan index to block future open by handle */
>  		err = ovl_cleanup_and_whiteout(OVL_FS(dentry->d_sb),
> -					       dir, index);
> +					       indexdir, index);
>  	} else {
>  		/* Cleanup orphan index entries */
>  		err = ovl_cleanup(ofs, dir, index);
> diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> index ba45e809555a..b8d913c61623 100644
> --- a/fs/smb/server/vfs.c
> +++ b/fs/smb/server/vfs.c
> @@ -764,10 +764,10 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
>  	}
>  
>  	rd.old_mnt_idmap	= mnt_idmap(old_path->mnt),
> -	rd.old_dir		= d_inode(old_parent),
> +	rd.old_dir		= old_parent,
>  	rd.old_dentry		= old_child,
>  	rd.new_mnt_idmap	= mnt_idmap(new_path.mnt),
> -	rd.new_dir		= new_path.dentry->d_inode,
> +	rd.new_dir		= new_path.dentry,
>  	rd.new_dentry		= new_dentry,
>  	rd.flags		= flags,
>  	rd.delegated_inode	= NULL,
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 16f40a6f8264..9a83904c9d4a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2016,10 +2016,10 @@ int vfs_unlink(struct mnt_idmap *, struct inode *, struct dentry *,
>   */
>  struct renamedata {
>  	struct mnt_idmap *old_mnt_idmap;
> -	struct inode *old_dir;
> +	struct dentry *old_dir;
>  	struct dentry *old_dentry;
>  	struct mnt_idmap *new_mnt_idmap;
> -	struct inode *new_dir;
> +	struct dentry *new_dir;
>  	struct dentry *new_dentry;
>  	struct inode **delegated_inode;
>  	unsigned int flags;

For the NFSD hunks:

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


-- 
Chuck Lever

Re: [PATCH 1/2] VFS: change old_dir and new_dir in struct renamedata to dentrys

[Namjae Jeon]

On Thu, Jun 12, 2025 at 7:59 AM NeilBrown <neil@brown.name> wrote:
>
> all users of 'struct renamedata' have the dentry for the old and new
> directories, and often have no use for the inode except to store it in
> the renamedata.
>
> This patch changes struct renamedata to hold the dentry, rather than
> the inode, for the old and new directories, and changes callers to
> match.
>
> This results in the removal of several local variables and several
> dereferences of ->d_inode at the cost of adding ->d_inode dereferences
> to vfs_rename().
>
> Signed-off-by: NeilBrown <neil@brown.name>
For ksmbd part,
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks!

Re: [PATCH 1/2] VFS: change old_dir and new_dir in struct renamedata to dentrys

[NeilBrown]

On Thu, 12 Jun 2025, Amir Goldstein wrote:
> On Thu, Jun 12, 2025 at 12:59 AM NeilBrown <neil@brown.name> wrote:
> >
> > all users of 'struct renamedata' have the dentry for the old and new
> > directories, and often have no use for the inode except to store it in
> > the renamedata.
> >
> > This patch changes struct renamedata to hold the dentry, rather than
> > the inode, for the old and new directories, and changes callers to
> > match.
> >
> > This results in the removal of several local variables and several
> > dereferences of ->d_inode at the cost of adding ->d_inode dereferences
> > to vfs_rename().
> >
> > Signed-off-by: NeilBrown <neil@brown.name>
> > ---
> >  fs/cachefiles/namei.c    |  4 ++--
> >  fs/ecryptfs/inode.c      |  4 ++--
> >  fs/namei.c               |  6 +++---
> >  fs/nfsd/vfs.c            |  7 ++-----
> >  fs/overlayfs/copy_up.c   |  6 +++---
> >  fs/overlayfs/dir.c       | 16 ++++++++--------
> >  fs/overlayfs/overlayfs.h |  6 +++---
> >  fs/overlayfs/readdir.c   |  2 +-
> >  fs/overlayfs/super.c     |  2 +-
> >  fs/overlayfs/util.c      |  2 +-
> >  fs/smb/server/vfs.c      |  4 ++--
> >  include/linux/fs.h       |  4 ++--
> >  12 files changed, 30 insertions(+), 33 deletions(-)
> >
> > diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> > index aecfc5c37b49..053fc28b5423 100644
> > --- a/fs/cachefiles/namei.c
> > +++ b/fs/cachefiles/namei.c
> > @@ -388,10 +388,10 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> >         } else {
> >                 struct renamedata rd = {
> >                         .old_mnt_idmap  = &nop_mnt_idmap,
> > -                       .old_dir        = d_inode(dir),
> > +                       .old_dir        = dir,
> >                         .old_dentry     = rep,
> >                         .new_mnt_idmap  = &nop_mnt_idmap,
> > -                       .new_dir        = d_inode(cache->graveyard),
> > +                       .new_dir        = cache->graveyard,
> >                         .new_dentry     = grave,
> >                 };
> >                 trace_cachefiles_rename(object, d_inode(rep)->i_ino, why);
> > diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> > index 493d7f194956..c9fec8b7e000 100644
> > --- a/fs/ecryptfs/inode.c
> > +++ b/fs/ecryptfs/inode.c
> > @@ -635,10 +635,10 @@ ecryptfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
> >         }
> >
> >         rd.old_mnt_idmap        = &nop_mnt_idmap;
> > -       rd.old_dir              = d_inode(lower_old_dir_dentry);
> > +       rd.old_dir              = lower_old_dir_dentry;
> >         rd.old_dentry           = lower_old_dentry;
> >         rd.new_mnt_idmap        = &nop_mnt_idmap;
> > -       rd.new_dir              = d_inode(lower_new_dir_dentry);
> > +       rd.new_dir              = lower_new_dir_dentry;
> >         rd.new_dentry           = lower_new_dentry;
> >         rc = vfs_rename(&rd);
> >         if (rc)
> > diff --git a/fs/namei.c b/fs/namei.c
> > index 019073162b8a..5b0be8bca50d 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -5007,7 +5007,7 @@ SYSCALL_DEFINE2(link, const char __user *, oldname, const char __user *, newname
> >  int vfs_rename(struct renamedata *rd)
> >  {
> >         int error;
> > -       struct inode *old_dir = rd->old_dir, *new_dir = rd->new_dir;
> > +       struct inode *old_dir = d_inode(rd->old_dir), *new_dir = d_inode(rd->new_dir);
> >         struct dentry *old_dentry = rd->old_dentry;
> >         struct dentry *new_dentry = rd->new_dentry;
> >         struct inode **delegated_inode = rd->delegated_inode;
> > @@ -5266,10 +5266,10 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
> >         if (error)
> >                 goto exit5;
> >
> > -       rd.old_dir         = old_path.dentry->d_inode;
> > +       rd.old_dir         = old_path.dentry;
> >         rd.old_dentry      = old_dentry;
> >         rd.old_mnt_idmap   = mnt_idmap(old_path.mnt);
> > -       rd.new_dir         = new_path.dentry->d_inode;
> > +       rd.new_dir         = new_path.dentry;
> >         rd.new_dentry      = new_dentry;
> >         rd.new_mnt_idmap   = mnt_idmap(new_path.mnt);
> >         rd.delegated_inode = &delegated_inode;
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index cd689df2ca5d..3c87fbd22c57 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -1864,7 +1864,6 @@ 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 inode    *fdir, *tdir;
> >         int             type = S_IFDIR;
> >         __be32          err;
> >         int             host_err;
> > @@ -1880,10 +1879,8 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> >                 goto out;
> >
> >         fdentry = ffhp->fh_dentry;
> > -       fdir = d_inode(fdentry);
> >
> >         tdentry = tfhp->fh_dentry;
> > -       tdir = d_inode(tdentry);
> >
> >         err = nfserr_perm;
> >         if (!flen || isdotent(fname, flen) || !tlen || isdotent(tname, tlen))
> > @@ -1944,10 +1941,10 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> >         } else {
> >                 struct renamedata rd = {
> >                         .old_mnt_idmap  = &nop_mnt_idmap,
> > -                       .old_dir        = fdir,
> > +                       .old_dir        = fdentry,
> >                         .old_dentry     = odentry,
> >                         .new_mnt_idmap  = &nop_mnt_idmap,
> > -                       .new_dir        = tdir,
> > +                       .new_dir        = tdentry,
> >                         .new_dentry     = ndentry,
> >                 };
> >                 int retries;
> > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> > index d7310fcf3888..8a3c0d18ec2e 100644
> > --- a/fs/overlayfs/copy_up.c
> > +++ b/fs/overlayfs/copy_up.c
> > @@ -563,7 +563,7 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
> >         if (IS_ERR(index)) {
> >                 err = PTR_ERR(index);
> >         } else {
> > -               err = ovl_do_rename(ofs, dir, temp, dir, index, 0);
> > +               err = ovl_do_rename(ofs, indexdir, temp, indexdir, index, 0);
> >                 dput(index);
> >         }
> >  out:
> > @@ -762,7 +762,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
> >  {
> >         struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
> >         struct inode *inode;
> > -       struct inode *udir = d_inode(c->destdir), *wdir = d_inode(c->workdir);
> > +       struct inode *wdir = d_inode(c->workdir);
> >         struct path path = { .mnt = ovl_upper_mnt(ofs) };
> >         struct dentry *temp, *upper, *trap;
> >         struct ovl_cu_creds cc;
> > @@ -829,7 +829,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
> >         if (IS_ERR(upper))
> >                 goto cleanup;
> >
> > -       err = ovl_do_rename(ofs, wdir, temp, udir, upper, 0);
> > +       err = ovl_do_rename(ofs, c->workdir, temp, c->destdir, upper, 0);
> >         dput(upper);
> >         if (err)
> >                 goto cleanup;
> > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> > index fe493f3ed6b6..4fc221ea6480 100644
> > --- a/fs/overlayfs/dir.c
> > +++ b/fs/overlayfs/dir.c
> > @@ -107,7 +107,7 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs)
> >  }
> >
> >  /* Caller must hold i_mutex on both workdir and dir */
> > -int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct inode *dir,
> > +int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir,
> >                              struct dentry *dentry)
> >  {
> >         struct inode *wdir = ofs->workdir->d_inode;
> > @@ -123,7 +123,7 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct inode *dir,
> >         if (d_is_dir(dentry))
> >                 flags = RENAME_EXCHANGE;
> >
> > -       err = ovl_do_rename(ofs, wdir, whiteout, dir, dentry, flags);
> > +       err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, dentry, flags);
> >         if (err)
> >                 goto kill_whiteout;
> >         if (flags)
> > @@ -384,7 +384,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
> >         if (err)
> >                 goto out_cleanup;
> >
> > -       err = ovl_do_rename(ofs, wdir, opaquedir, udir, upper, RENAME_EXCHANGE);
> > +       err = ovl_do_rename(ofs, workdir, opaquedir, upperdir, upper, RENAME_EXCHANGE);
> >         if (err)
> >                 goto out_cleanup;
> >
> > @@ -491,14 +491,14 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
> >                 if (err)
> >                         goto out_cleanup;
> >
> > -               err = ovl_do_rename(ofs, wdir, newdentry, udir, upper,
> > +               err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper,
> >                                     RENAME_EXCHANGE);
> >                 if (err)
> >                         goto out_cleanup;
> >
> >                 ovl_cleanup(ofs, wdir, upper);
> >         } else {
> > -               err = ovl_do_rename(ofs, wdir, newdentry, udir, upper, 0);
> > +               err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper, 0);
> >                 if (err)
> >                         goto out_cleanup;
> >         }
> > @@ -774,7 +774,7 @@ static int ovl_remove_and_whiteout(struct dentry *dentry,
> >                 goto out_dput_upper;
> >         }
> >
> > -       err = ovl_cleanup_and_whiteout(ofs, d_inode(upperdir), upper);
> > +       err = ovl_cleanup_and_whiteout(ofs, upperdir, upper);
> >         if (err)
> >                 goto out_d_drop;
> >
> > @@ -1246,8 +1246,8 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
> >         if (err)
> >                 goto out_dput;
> >
> > -       err = ovl_do_rename(ofs, old_upperdir->d_inode, olddentry,
> > -                           new_upperdir->d_inode, newdentry, flags);
> > +       err = ovl_do_rename(ofs, old_upperdir, olddentry,
> > +                           new_upperdir, newdentry, flags);
> >         if (err)
> >                 goto out_dput;
> >
> > diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> > index 8baaba0a3fe5..65f9d51bed7c 100644
> > --- a/fs/overlayfs/overlayfs.h
> > +++ b/fs/overlayfs/overlayfs.h
> > @@ -353,8 +353,8 @@ 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(struct ovl_fs *ofs, struct inode *olddir,
> > -                               struct dentry *olddentry, struct inode *newdir,
> > +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;
> > @@ -826,7 +826,7 @@ static inline void ovl_copyflags(struct inode *from, struct inode *to)
> >
> >  /* dir.c */
> >  extern const struct inode_operations ovl_dir_inode_operations;
> > -int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct inode *dir,
> > +int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir,
> >                              struct dentry *dentry);
> >  struct ovl_cattr {
> >         dev_t rdev;
> > diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> > index 474c80d210d1..68cca52ae2ac 100644
> > --- a/fs/overlayfs/readdir.c
> > +++ b/fs/overlayfs/readdir.c
> > @@ -1235,7 +1235,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs)
> >                          * Whiteout orphan index to block future open by
> >                          * handle after overlay nlink dropped to zero.
> >                          */
> > -                       err = ovl_cleanup_and_whiteout(ofs, dir, index);
> > +                       err = ovl_cleanup_and_whiteout(ofs, indexdir, index);
> >                 } else {
> >                         /* Cleanup orphan index entries */
> >                         err = ovl_cleanup(ofs, dir, index);
> > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > index e19940d649ca..cf99b276fdfb 100644
> > --- a/fs/overlayfs/super.c
> > +++ b/fs/overlayfs/super.c
> > @@ -580,7 +580,7 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
> >
> >         /* Name is inline and stable - using snapshot as a copy helper */
> >         take_dentry_name_snapshot(&name, temp);
> > -       err = ovl_do_rename(ofs, dir, temp, dir, dest, RENAME_WHITEOUT);
> > +       err = ovl_do_rename(ofs, workdir, temp, workdir, dest, RENAME_WHITEOUT);
> >         if (err) {
> >                 if (err == -EINVAL)
> >                         err = 0;
> > diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> > index dcccb4b4a66c..2b4754c645ee 100644
> > --- a/fs/overlayfs/util.c
> > +++ b/fs/overlayfs/util.c
> > @@ -1115,7 +1115,7 @@ static void ovl_cleanup_index(struct dentry *dentry)
> >         } else if (ovl_index_all(dentry->d_sb)) {
> >                 /* Whiteout orphan index to block future open by handle */
> >                 err = ovl_cleanup_and_whiteout(OVL_FS(dentry->d_sb),
> > -                                              dir, index);
> > +                                              indexdir, index);
> >         } else {
> >                 /* Cleanup orphan index entries */
> >                 err = ovl_cleanup(ofs, dir, index);
> > diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> > index ba45e809555a..b8d913c61623 100644
> > --- a/fs/smb/server/vfs.c
> > +++ b/fs/smb/server/vfs.c
> > @@ -764,10 +764,10 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
> >         }
> >
> >         rd.old_mnt_idmap        = mnt_idmap(old_path->mnt),
> > -       rd.old_dir              = d_inode(old_parent),
> > +       rd.old_dir              = old_parent,
> >         rd.old_dentry           = old_child,
> >         rd.new_mnt_idmap        = mnt_idmap(new_path.mnt),
> > -       rd.new_dir              = new_path.dentry->d_inode,
> > +       rd.new_dir              = new_path.dentry,
> >         rd.new_dentry           = new_dentry,
> >         rd.flags                = flags,
> >         rd.delegated_inode      = NULL,
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 16f40a6f8264..9a83904c9d4a 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2016,10 +2016,10 @@ int vfs_unlink(struct mnt_idmap *, struct inode *, struct dentry *,
> >   */
> >  struct renamedata {
> >         struct mnt_idmap *old_mnt_idmap;
> > -       struct inode *old_dir;
> > +       struct dentry *old_dir;
> >         struct dentry *old_dentry;
> >         struct mnt_idmap *new_mnt_idmap;
> > -       struct inode *new_dir;
> > +       struct dentry *new_dir;
> >         struct dentry *new_dentry;
> >         struct inode **delegated_inode;
> >         unsigned int flags;
> > --
> 
> It bothers me a bit that we are keeping the field name while changing its type.
> 
> There is a wide convention in vfs methods and helpers of using
> struct inode *dir
> as the parent directory inode
> and often (but not always) using
> struct dentry *parent
> as the parent dentry
> 
> How do you feel about making struct renamedata use:
> 
> struct dentry *old_parent;
> struct dentry *new_parent;
> 
> I don't think it will add any churn beyond what this patch already does.

I think that is an excellent idea - thanks.
Particularly as the kernel-doc documentation for struct renamedata
describe the fields as "parent of source" and "parent of destination".

I'll resubmit with that change and the reviewed-bys.

Thanks,
NeilBrown

Re: [PATCH 1/2] VFS: change old_dir and new_dir in struct renamedata to dentrys

[NeilBrown]

On Thu, 12 Jun 2025, Miklos Szeredi wrote:
> On Thu, 12 Jun 2025 at 01:38, Al Viro <viro@zeniv.linux.org.uk> wrote:
> 
> > Umm...  No objections, as long as overlayfs part is correct; it seems
> > to be, but I hadn't checked every chunk there...
> 
> Overlayfs parts looks okay too.
> 
> A followup would be nice (e.g. make ovl_cleanup() take a dentry for
> the directory as well, etc) so that there's no need to have local
> variables for both the inode and dentry of the directory.

I am planning some followups and will include that in them.
I'll also be sure to test with fs-tests after consulting README.overlay
as you suggest elsewhere - thanks.

NeilBrown