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

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 <[email protected]>
> ---
>  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
> 
>