[PATCH 1/7] VFS: unify old_mnt_idmap and new_mnt_idmap in renamedata

NeilBrown posted 7 patches 2 months, 2 weeks ago
[PATCH 1/7] VFS: unify old_mnt_idmap and new_mnt_idmap in renamedata
Posted by NeilBrown 2 months, 2 weeks ago
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;
-- 
2.49.0
Re: [PATCH 1/7] VFS: unify old_mnt_idmap and new_mnt_idmap in renamedata
Posted by Jeff Layton 2 months, 2 weeks ago
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>