[PATCH v4 09/17] vfs: add struct createdata for passing arguments to vfs_create()

Jeff Layton posted 17 patches 3 months ago
There is a newer version of this series
[PATCH v4 09/17] vfs: add struct createdata for passing arguments to vfs_create()
Posted by Jeff Layton 3 months ago
vfs_create() has grown an uncomfortably long argument list, and a
following patch will add another. Convert it to take a new struct
createdata pointer and fix up the callers to pass one in.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ecryptfs/inode.c      | 11 ++++++++---
 fs/namei.c               | 33 ++++++++++++++++++++-------------
 fs/nfsd/nfs3proc.c       |  9 ++++++++-
 fs/nfsd/vfs.c            | 19 ++++++++++++-------
 fs/open.c                |  9 ++++++---
 fs/overlayfs/overlayfs.h |  7 ++++++-
 fs/smb/server/vfs.c      |  9 +++++++--
 include/linux/fs.h       | 13 +++++++++++--
 8 files changed, 78 insertions(+), 32 deletions(-)

diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 88631291b32535f623a3fbe4ea9b6ed48a306ca0..51accd166dbf515eb5221b6a39b204622a6b0f7c 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -187,9 +187,14 @@ ecryptfs_do_create(struct inode *directory_inode,
 	struct inode *inode;
 
 	rc = lock_parent(ecryptfs_dentry, &lower_dentry, &lower_dir);
-	if (!rc)
-		rc = vfs_create(&nop_mnt_idmap, lower_dir,
-				lower_dentry, mode, true);
+	if (!rc) {
+		struct createdata args = { .idmap = &nop_mnt_idmap,
+					   .dir = lower_dir,
+					   .dentry = lower_dentry,
+					   .mode = mode,
+					   .excl = true };
+		rc = vfs_create(&args);
+	}
 	if (rc) {
 		printk(KERN_ERR "%s: Failure to create dentry in lower fs; "
 		       "rc = [%d]\n", __func__, rc);
diff --git a/fs/namei.c b/fs/namei.c
index f439429bdfa271ccc64c937771ef4175597feb53..fdf4e78cd041de8c564b7d1d89a46ba2aaf79d53 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3460,23 +3460,22 @@ static inline umode_t vfs_prepare_mode(struct mnt_idmap *idmap,
 
 /**
  * vfs_create - create new file
- * @idmap:	idmap of the mount the inode was found from
- * @dir:	inode of the parent directory
- * @dentry:	dentry of the child file
- * @mode:	mode of the child file
- * @want_excl:	whether the file must not yet exist
+ * @args:	struct createdata describing create to be done
  *
  * Create a new file.
  *
  * If the inode has been found through an idmapped mount the idmap of
- * the vfsmount must be passed through @idmap. This function will then take
- * care to map the inode according to @idmap before checking permissions.
+ * the vfsmount must be passed through @args->idmap. This function will then take
+ * care to map the inode according to @args->idmap before checking permissions.
  * On non-idmapped mounts or if permission checking is to be performed on the
  * raw inode simply pass @nop_mnt_idmap.
  */
-int vfs_create(struct mnt_idmap *idmap, struct inode *dir,
-	       struct dentry *dentry, umode_t mode, bool want_excl)
+int vfs_create(struct createdata *args)
 {
+	struct mnt_idmap *idmap = args->idmap;
+	struct inode *dir = args->dir;
+	struct dentry *dentry = args->dentry;
+	umode_t mode = args->mode;
 	int error;
 
 	error = may_create(idmap, dir, dentry);
@@ -3490,7 +3489,7 @@ int vfs_create(struct mnt_idmap *idmap, struct inode *dir,
 	error = security_inode_create(dir, dentry, mode);
 	if (error)
 		return error;
-	error = dir->i_op->create(idmap, dir, dentry, mode, want_excl);
+	error = dir->i_op->create(idmap, dir, dentry, mode, args->excl);
 	if (!error)
 		fsnotify_create(dir, dentry);
 	return error;
@@ -4382,12 +4381,20 @@ static int do_mknodat(int dfd, struct filename *name, umode_t mode,
 
 	idmap = mnt_idmap(path.mnt);
 	switch (mode & S_IFMT) {
-		case 0: case S_IFREG:
-			error = vfs_create(idmap, path.dentry->d_inode,
-					   dentry, mode, true);
+		case 0:
+		case S_IFREG:
+		{
+			struct createdata args = { .idmap = idmap,
+						   .dir = path.dentry->d_inode,
+						   .dentry = dentry,
+						   .mode = mode,
+						   .excl = true };
+
+			error = vfs_create(&args);
 			if (!error)
 				security_path_post_mknod(idmap, dentry);
 			break;
+		}
 		case S_IFCHR: case S_IFBLK:
 			error = vfs_mknod(idmap, path.dentry->d_inode,
 					  dentry, mode, new_decode_dev(dev));
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index b6d03e1ef5f7a5e8dd111b0d56c061f1e91abff7..dcd7de465e7e33d1c66ee0272c4f220d55e85928 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -258,6 +258,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	struct nfsd_attrs attrs = {
 		.na_iattr	= iap,
 	};
+	struct createdata cargs = { };
 	__u32 v_mtime, v_atime;
 	struct inode *inode;
 	__be32 status;
@@ -344,7 +345,13 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	status = fh_fill_pre_attrs(fhp);
 	if (status != nfs_ok)
 		goto out;
-	host_err = vfs_create(&nop_mnt_idmap, inode, child, iap->ia_mode, true);
+
+	cargs.idmap = &nop_mnt_idmap;
+	cargs.dir = inode;
+	cargs.dentry = child;
+	cargs.mode = iap->ia_mode;
+	cargs.excl = true;
+	host_err = vfs_create(&cargs);
 	if (host_err < 0) {
 		status = nfserrno(host_err);
 		goto out;
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index c400ea94ff2e837fd59719bf2c4b79ef1d064743..e4ed1952f02c0a66c64528e59453cc9b2352c18f 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1527,11 +1527,12 @@ nfsd_create_locked(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		   struct nfsd_attrs *attrs,
 		   int type, dev_t rdev, struct svc_fh *resfhp)
 {
-	struct dentry	*dentry, *dchild;
-	struct inode	*dirp;
-	struct iattr	*iap = attrs->na_iattr;
-	__be32		err;
-	int		host_err = 0;
+	struct dentry		*dentry, *dchild;
+	struct inode		*dirp;
+	struct iattr		*iap = attrs->na_iattr;
+	__be32			err;
+	int			host_err = 0;
+	struct createdata	cargs = { };
 
 	dentry = fhp->fh_dentry;
 	dirp = d_inode(dentry);
@@ -1552,8 +1553,12 @@ nfsd_create_locked(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	err = 0;
 	switch (type) {
 	case S_IFREG:
-		host_err = vfs_create(&nop_mnt_idmap, dirp, dchild,
-				      iap->ia_mode, true);
+		cargs.idmap = &nop_mnt_idmap;
+		cargs.dir = dirp;
+		cargs.dentry = dchild;
+		cargs.mode = iap->ia_mode;
+		cargs.excl = true;
+		host_err = vfs_create(&cargs);
 		if (!host_err)
 			nfsd_check_ignore_resizing(iap);
 		break;
diff --git a/fs/open.c b/fs/open.c
index fdaa6f08f6f4cac5c2fefd3eafa5e430e51f3979..006cc2aeb1fbbb3db48b32db798108da120f75c2 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1164,6 +1164,11 @@ struct file *dentry_open_nonotify(const struct path *path, int flags,
 struct file *dentry_create(const struct path *path, int flags, umode_t mode,
 			   const struct cred *cred)
 {
+	struct createdata cargs = { .idmap = mnt_idmap(path->mnt),
+				    .dir = d_inode(path->dentry->d_parent),
+				    .dentry = path->dentry,
+				    .mode = mode,
+				    .excl = true };
 	struct file *f;
 	int error;
 
@@ -1171,9 +1176,7 @@ struct file *dentry_create(const struct path *path, int flags, umode_t mode,
 	if (IS_ERR(f))
 		return f;
 
-	error = vfs_create(mnt_idmap(path->mnt),
-			   d_inode(path->dentry->d_parent),
-			   path->dentry, mode, true);
+	error = vfs_create(&cargs);
 	if (!error)
 		error = vfs_open(path, f);
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index d215d7349489686b66bb66e939b27046f7d836f6..5fa939ac842ed04df8f0088233f4cba4ac703c05 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -235,7 +235,12 @@ static inline int ovl_do_create(struct ovl_fs *ofs,
 				struct inode *dir, struct dentry *dentry,
 				umode_t mode)
 {
-	int err = vfs_create(ovl_upper_mnt_idmap(ofs), dir, dentry, mode, true);
+	struct createdata cargs = { .idmap = ovl_upper_mnt_idmap(ofs),
+				    .dir = dir,
+				    .dentry = dentry,
+				    .mode = mode,
+				    .excl = true };
+	int err = vfs_create(&cargs);
 
 	pr_debug("create(%pd2, 0%o) = %i\n", dentry, mode, err);
 	return err;
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index c5f0f3170d586cb2dc4d416b80948c642797fb82..fbc3c34e14b870f1750b945349335afb62d89d0d 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -173,6 +173,7 @@ void ksmbd_vfs_query_maximal_access(struct mnt_idmap *idmap,
  */
 int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode)
 {
+	struct createdata cargs = { };
 	struct path path;
 	struct dentry *dentry;
 	int err;
@@ -188,8 +189,12 @@ int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode)
 	}
 
 	mode |= S_IFREG;
-	err = vfs_create(mnt_idmap(path.mnt), d_inode(path.dentry),
-			 dentry, mode, true);
+	cargs.idmap = mnt_idmap(path.mnt);
+	cargs.dir = d_inode(path.dentry);
+	cargs.dentry = dentry;
+	cargs.mode = mode;
+	cargs.excl = true;
+	err = vfs_create(&cargs);
 	if (!err) {
 		ksmbd_vfs_inherit_owner(work, d_inode(path.dentry),
 					d_inode(dentry));
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 12873214e1c7811735ea5d2dee3d57e2a5604d8f..b61873767b37591aecadd147623d7dfc866bef82 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2111,8 +2111,17 @@ bool inode_owner_or_capable(struct mnt_idmap *idmap,
 /*
  * VFS helper functions..
  */
-int vfs_create(struct mnt_idmap *, struct inode *,
-	       struct dentry *, umode_t, bool);
+
+struct createdata {
+	struct mnt_idmap *idmap;	// idmap of the mount the inode was found from
+	struct inode *dir;		// inode of parent directory
+	struct dentry *dentry;		// dentry of the child file
+	umode_t mode;			// mode of the child file
+	bool excl;			// whether the file must not yet exist
+};
+
+int vfs_create(struct createdata *);
+
 struct dentry *vfs_mkdir(struct mnt_idmap *, struct inode *,
 			 struct dentry *, umode_t, struct delegated_inode *);
 int vfs_mknod(struct mnt_idmap *, struct inode *, struct dentry *,

-- 
2.51.1
Re: [PATCH v4 09/17] vfs: add struct createdata for passing arguments to vfs_create()
Posted by NeilBrown 3 months ago
On Mon, 03 Nov 2025, Jeff Layton wrote:
> vfs_create() has grown an uncomfortably long argument list, and a
> following patch will add another. Convert it to take a new struct
> createdata pointer and fix up the callers to pass one in.
> 

I know Christian asked for this and he is a Maintainer so.....

but I would like say that I don't think this is a win.  The argument
list isn't *that* long, and all the args are quite different so there is
little room for confusion.

I would be in favour of dropping the "dir" arg because it is always
   d_inode(dentry->d_parent)
which is stable.

I would rather pass the vfsmnt rather than the idmap, then we could pass
"struct path", for both that and dentry, but I know Christian disagrees.

So if anyone really thinks the arg list is too long, I think there are
better solutions.  But I don't even think the length is a problem.

NeilBrown


> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ecryptfs/inode.c      | 11 ++++++++---
>  fs/namei.c               | 33 ++++++++++++++++++++-------------
>  fs/nfsd/nfs3proc.c       |  9 ++++++++-
>  fs/nfsd/vfs.c            | 19 ++++++++++++-------
>  fs/open.c                |  9 ++++++---
>  fs/overlayfs/overlayfs.h |  7 ++++++-
>  fs/smb/server/vfs.c      |  9 +++++++--
>  include/linux/fs.h       | 13 +++++++++++--
>  8 files changed, 78 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 88631291b32535f623a3fbe4ea9b6ed48a306ca0..51accd166dbf515eb5221b6a39b204622a6b0f7c 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -187,9 +187,14 @@ ecryptfs_do_create(struct inode *directory_inode,
>  	struct inode *inode;
>  
>  	rc = lock_parent(ecryptfs_dentry, &lower_dentry, &lower_dir);
> -	if (!rc)
> -		rc = vfs_create(&nop_mnt_idmap, lower_dir,
> -				lower_dentry, mode, true);
> +	if (!rc) {
> +		struct createdata args = { .idmap = &nop_mnt_idmap,
> +					   .dir = lower_dir,
> +					   .dentry = lower_dentry,
> +					   .mode = mode,
> +					   .excl = true };
> +		rc = vfs_create(&args);
> +	}
>  	if (rc) {
>  		printk(KERN_ERR "%s: Failure to create dentry in lower fs; "
>  		       "rc = [%d]\n", __func__, rc);
> diff --git a/fs/namei.c b/fs/namei.c
> index f439429bdfa271ccc64c937771ef4175597feb53..fdf4e78cd041de8c564b7d1d89a46ba2aaf79d53 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3460,23 +3460,22 @@ static inline umode_t vfs_prepare_mode(struct mnt_idmap *idmap,
>  
>  /**
>   * vfs_create - create new file
> - * @idmap:	idmap of the mount the inode was found from
> - * @dir:	inode of the parent directory
> - * @dentry:	dentry of the child file
> - * @mode:	mode of the child file
> - * @want_excl:	whether the file must not yet exist
> + * @args:	struct createdata describing create to be done
>   *
>   * Create a new file.
>   *
>   * If the inode has been found through an idmapped mount the idmap of
> - * the vfsmount must be passed through @idmap. This function will then take
> - * care to map the inode according to @idmap before checking permissions.
> + * the vfsmount must be passed through @args->idmap. This function will then take
> + * care to map the inode according to @args->idmap before checking permissions.
>   * On non-idmapped mounts or if permission checking is to be performed on the
>   * raw inode simply pass @nop_mnt_idmap.
>   */
> -int vfs_create(struct mnt_idmap *idmap, struct inode *dir,
> -	       struct dentry *dentry, umode_t mode, bool want_excl)
> +int vfs_create(struct createdata *args)
>  {
> +	struct mnt_idmap *idmap = args->idmap;
> +	struct inode *dir = args->dir;
> +	struct dentry *dentry = args->dentry;
> +	umode_t mode = args->mode;
>  	int error;
>  
>  	error = may_create(idmap, dir, dentry);
> @@ -3490,7 +3489,7 @@ int vfs_create(struct mnt_idmap *idmap, struct inode *dir,
>  	error = security_inode_create(dir, dentry, mode);
>  	if (error)
>  		return error;
> -	error = dir->i_op->create(idmap, dir, dentry, mode, want_excl);
> +	error = dir->i_op->create(idmap, dir, dentry, mode, args->excl);
>  	if (!error)
>  		fsnotify_create(dir, dentry);
>  	return error;
> @@ -4382,12 +4381,20 @@ static int do_mknodat(int dfd, struct filename *name, umode_t mode,
>  
>  	idmap = mnt_idmap(path.mnt);
>  	switch (mode & S_IFMT) {
> -		case 0: case S_IFREG:
> -			error = vfs_create(idmap, path.dentry->d_inode,
> -					   dentry, mode, true);
> +		case 0:
> +		case S_IFREG:
> +		{
> +			struct createdata args = { .idmap = idmap,
> +						   .dir = path.dentry->d_inode,
> +						   .dentry = dentry,
> +						   .mode = mode,
> +						   .excl = true };
> +
> +			error = vfs_create(&args);
>  			if (!error)
>  				security_path_post_mknod(idmap, dentry);
>  			break;
> +		}
>  		case S_IFCHR: case S_IFBLK:
>  			error = vfs_mknod(idmap, path.dentry->d_inode,
>  					  dentry, mode, new_decode_dev(dev));
> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
> index b6d03e1ef5f7a5e8dd111b0d56c061f1e91abff7..dcd7de465e7e33d1c66ee0272c4f220d55e85928 100644
> --- a/fs/nfsd/nfs3proc.c
> +++ b/fs/nfsd/nfs3proc.c
> @@ -258,6 +258,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	struct nfsd_attrs attrs = {
>  		.na_iattr	= iap,
>  	};
> +	struct createdata cargs = { };
>  	__u32 v_mtime, v_atime;
>  	struct inode *inode;
>  	__be32 status;
> @@ -344,7 +345,13 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	status = fh_fill_pre_attrs(fhp);
>  	if (status != nfs_ok)
>  		goto out;
> -	host_err = vfs_create(&nop_mnt_idmap, inode, child, iap->ia_mode, true);
> +
> +	cargs.idmap = &nop_mnt_idmap;
> +	cargs.dir = inode;
> +	cargs.dentry = child;
> +	cargs.mode = iap->ia_mode;
> +	cargs.excl = true;
> +	host_err = vfs_create(&cargs);
>  	if (host_err < 0) {
>  		status = nfserrno(host_err);
>  		goto out;
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index c400ea94ff2e837fd59719bf2c4b79ef1d064743..e4ed1952f02c0a66c64528e59453cc9b2352c18f 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1527,11 +1527,12 @@ nfsd_create_locked(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		   struct nfsd_attrs *attrs,
>  		   int type, dev_t rdev, struct svc_fh *resfhp)
>  {
> -	struct dentry	*dentry, *dchild;
> -	struct inode	*dirp;
> -	struct iattr	*iap = attrs->na_iattr;
> -	__be32		err;
> -	int		host_err = 0;
> +	struct dentry		*dentry, *dchild;
> +	struct inode		*dirp;
> +	struct iattr		*iap = attrs->na_iattr;
> +	__be32			err;
> +	int			host_err = 0;
> +	struct createdata	cargs = { };
>  
>  	dentry = fhp->fh_dentry;
>  	dirp = d_inode(dentry);
> @@ -1552,8 +1553,12 @@ nfsd_create_locked(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	err = 0;
>  	switch (type) {
>  	case S_IFREG:
> -		host_err = vfs_create(&nop_mnt_idmap, dirp, dchild,
> -				      iap->ia_mode, true);
> +		cargs.idmap = &nop_mnt_idmap;
> +		cargs.dir = dirp;
> +		cargs.dentry = dchild;
> +		cargs.mode = iap->ia_mode;
> +		cargs.excl = true;
> +		host_err = vfs_create(&cargs);
>  		if (!host_err)
>  			nfsd_check_ignore_resizing(iap);
>  		break;
> diff --git a/fs/open.c b/fs/open.c
> index fdaa6f08f6f4cac5c2fefd3eafa5e430e51f3979..006cc2aeb1fbbb3db48b32db798108da120f75c2 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -1164,6 +1164,11 @@ struct file *dentry_open_nonotify(const struct path *path, int flags,
>  struct file *dentry_create(const struct path *path, int flags, umode_t mode,
>  			   const struct cred *cred)
>  {
> +	struct createdata cargs = { .idmap = mnt_idmap(path->mnt),
> +				    .dir = d_inode(path->dentry->d_parent),
> +				    .dentry = path->dentry,
> +				    .mode = mode,
> +				    .excl = true };
>  	struct file *f;
>  	int error;
>  
> @@ -1171,9 +1176,7 @@ struct file *dentry_create(const struct path *path, int flags, umode_t mode,
>  	if (IS_ERR(f))
>  		return f;
>  
> -	error = vfs_create(mnt_idmap(path->mnt),
> -			   d_inode(path->dentry->d_parent),
> -			   path->dentry, mode, true);
> +	error = vfs_create(&cargs);
>  	if (!error)
>  		error = vfs_open(path, f);
>  
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index d215d7349489686b66bb66e939b27046f7d836f6..5fa939ac842ed04df8f0088233f4cba4ac703c05 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -235,7 +235,12 @@ static inline int ovl_do_create(struct ovl_fs *ofs,
>  				struct inode *dir, struct dentry *dentry,
>  				umode_t mode)
>  {
> -	int err = vfs_create(ovl_upper_mnt_idmap(ofs), dir, dentry, mode, true);
> +	struct createdata cargs = { .idmap = ovl_upper_mnt_idmap(ofs),
> +				    .dir = dir,
> +				    .dentry = dentry,
> +				    .mode = mode,
> +				    .excl = true };
> +	int err = vfs_create(&cargs);
>  
>  	pr_debug("create(%pd2, 0%o) = %i\n", dentry, mode, err);
>  	return err;
> diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> index c5f0f3170d586cb2dc4d416b80948c642797fb82..fbc3c34e14b870f1750b945349335afb62d89d0d 100644
> --- a/fs/smb/server/vfs.c
> +++ b/fs/smb/server/vfs.c
> @@ -173,6 +173,7 @@ void ksmbd_vfs_query_maximal_access(struct mnt_idmap *idmap,
>   */
>  int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode)
>  {
> +	struct createdata cargs = { };
>  	struct path path;
>  	struct dentry *dentry;
>  	int err;
> @@ -188,8 +189,12 @@ int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode)
>  	}
>  
>  	mode |= S_IFREG;
> -	err = vfs_create(mnt_idmap(path.mnt), d_inode(path.dentry),
> -			 dentry, mode, true);
> +	cargs.idmap = mnt_idmap(path.mnt);
> +	cargs.dir = d_inode(path.dentry);
> +	cargs.dentry = dentry;
> +	cargs.mode = mode;
> +	cargs.excl = true;
> +	err = vfs_create(&cargs);
>  	if (!err) {
>  		ksmbd_vfs_inherit_owner(work, d_inode(path.dentry),
>  					d_inode(dentry));
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 12873214e1c7811735ea5d2dee3d57e2a5604d8f..b61873767b37591aecadd147623d7dfc866bef82 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2111,8 +2111,17 @@ bool inode_owner_or_capable(struct mnt_idmap *idmap,
>  /*
>   * VFS helper functions..
>   */
> -int vfs_create(struct mnt_idmap *, struct inode *,
> -	       struct dentry *, umode_t, bool);
> +
> +struct createdata {
> +	struct mnt_idmap *idmap;	// idmap of the mount the inode was found from
> +	struct inode *dir;		// inode of parent directory
> +	struct dentry *dentry;		// dentry of the child file
> +	umode_t mode;			// mode of the child file
> +	bool excl;			// whether the file must not yet exist
> +};
> +
> +int vfs_create(struct createdata *);
> +
>  struct dentry *vfs_mkdir(struct mnt_idmap *, struct inode *,
>  			 struct dentry *, umode_t, struct delegated_inode *);
>  int vfs_mknod(struct mnt_idmap *, struct inode *, struct dentry *,
> 
> -- 
> 2.51.1
> 
> 
Re: [PATCH v4 09/17] vfs: add struct createdata for passing arguments to vfs_create()
Posted by NeilBrown 3 months ago
On Tue, 04 Nov 2025, NeilBrown wrote:
> On Mon, 03 Nov 2025, Jeff Layton wrote:
> > vfs_create() has grown an uncomfortably long argument list, and a
> > following patch will add another. Convert it to take a new struct
> > createdata pointer and fix up the callers to pass one in.
> > 
> 
> I know Christian asked for this and he is a Maintainer so.....
> 
> but I would like say that I don't think this is a win.  The argument
> list isn't *that* long, and all the args are quite different so there is
> little room for confusion.
> 
> I would be in favour of dropping the "dir" arg because it is always
>    d_inode(dentry->d_parent)
> which is stable.
> 
> I would rather pass the vfsmnt rather than the idmap, then we could pass
> "struct path", for both that and dentry, but I know Christian disagrees.
> 
> So if anyone really thinks the arg list is too long, I think there are
> better solutions.  But I don't even think the length is a problem.

Also *every* caller of vfs_create() passes ".excl = true".  So maybe we
don't need that arg at all.

I think that the last time false might have been passed to vfs_create()
was before

Commit ce8644fcadc5 ("lookup_open(): expand the call of vfs_create()")

NeilBrown