Re: [PATCH 03/18] VFS: replace nameidata and open_flag args to lookup_open()

> Op 01-06-2026 08:37 CEST schreef NeilBrown <[email protected]>:
> 
>  
> From: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
> 
> lookup_open is currently given "struct nameiodata" and "struct
> open_flag" pointer args.  These structures are internal to VFS.  Replace
> these with the individual fields that lookup_open() actually needs.
> This will allow it be exported so it can be used to replace
> dentry_create().
> 
> As lookup_open() can change both open_flag and mode, we keep the local
> variable and create an arg with a different name which is assigned to
> the local variable.
> 
> Signed-off-by: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
> ---
>  fs/namei.c | 38 +++++++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index b00ff3f2faf7..18a43c24d7f1 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -4402,12 +4402,15 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
>   *
>   * An error code is returned on failure.
>   */
> -static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> -				  const struct open_flags *op)
> +static struct dentry *lookup_open(const struct path *path, struct file *file,
> +				  const struct qstr *last,
> +				  unsigned int lookup_flags,
> +				  struct filename *name,
> +				  int open_flag_arg, umode_t mode_arg)
>  {
>  	struct delegated_inode delegated_inode = { };
>  	struct mnt_idmap *idmap;
> -	struct dentry *dir = nd->path.dentry;
> +	struct dentry *dir = path->dentry;
>  	struct inode *dir_inode = dir->d_inode;
>  	int open_flag;
>  	struct dentry *dentry;
> @@ -4416,13 +4419,13 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  	bool got_write;
>  
>  retry:
> -	open_flag = op->open_flag;
> +	open_flag = open_flag_arg;
>  	got_write = false;
> -	mode = op->mode;
> +	mode = mode_arg;
>  	create_error = 0;
>  
>  	if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) {
> -		got_write = !mnt_want_write(nd->path.mnt);
> +		got_write = !mnt_want_write(path->mnt);
>  		/*
>  		 * do _not_ fail yet - we might not need that or fail with
>  		 * a different error; let lookup_open() decide; we'll be
> @@ -4440,17 +4443,17 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  	}
>  
>  	file->f_mode &= ~FMODE_CREATED;
> -	dentry = d_lookup(dir, &nd->last);
> +	dentry = d_lookup(dir, last);
>  	for (;;) {
>  		if (!dentry) {
> -			dentry = d_alloc_parallel(dir, &nd->last);
> +			dentry = d_alloc_parallel(dir, last);
>  			if (IS_ERR(dentry))
>  				goto out;
>  		}
>  		if (d_in_lookup(dentry))
>  			break;
>  
> -		error = d_revalidate(dir_inode, &nd->last, dentry, nd->flags);
> +		error = d_revalidate(dir_inode, last, dentry, lookup_flags);
>  		if (likely(error > 0))
>  			break;
>  		if (error)
> @@ -4465,7 +4468,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  	}
>  
>  	if (open_flag & O_CREAT)
> -		audit_inode(nd->name, dir, AUDIT_INODE_PARENT);
> +		audit_inode(name, dir, AUDIT_INODE_PARENT);
>  
>  	/*
>  	 * Checking write permission is tricky, bacuse we don't know if we are
> @@ -4478,13 +4481,13 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  	 */
>  	if (unlikely(!got_write))
>  		open_flag &= ~O_TRUNC;
> -	idmap = mnt_idmap(nd->path.mnt);
> +	idmap = mnt_idmap(path->mnt);
>  	if (open_flag & O_CREAT) {
>  		if (open_flag & O_EXCL)
>  			open_flag &= ~O_TRUNC;
>  		mode = vfs_prepare_mode(idmap, dir_inode, mode, mode, mode);
>  		if (likely(got_write))
> -			create_error = may_o_create(idmap, &nd->path,
> +			create_error = may_o_create(idmap, path,
>  						    dentry, mode);
>  		else
>  			create_error = -EROFS;
> @@ -4492,9 +4495,9 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  	if (create_error)
>  		open_flag &= ~O_CREAT;
>  	if (dir_inode->i_op->atomic_open) {
> -		if (nd->flags & LOOKUP_DIRECTORY)
> +		if (lookup_flags & LOOKUP_DIRECTORY)
>  			open_flag |= O_DIRECTORY;
> -		dentry = atomic_open(&nd->path, dentry, file, open_flag, mode);
> +		dentry = atomic_open(path, dentry, file, open_flag, mode);
>  		if (unlikely(create_error) && dentry == ERR_PTR(-ENOENT))
>  			dentry = ERR_PTR(create_error);
>  		goto out;
> @@ -4502,7 +4505,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  
>  	if (d_in_lookup(dentry)) {
>  		struct dentry *res = dir_inode->i_op->lookup(dir_inode, dentry,
> -							     nd->flags);
> +							     lookup_flags);
>  		d_lookup_done(dentry);
>  		if (unlikely(res)) {
>  			if (IS_ERR(res)) {
> @@ -4550,7 +4553,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  		inode_unlock_shared(dir_inode);
>  
>  	if (got_write)
> -		mnt_drop_write(nd->path.mnt);
> +		mnt_drop_write(path->mnt);
>  
>  	if (is_delegated(&delegated_inode)) {
>  		/* Must have come through out_dput */
> @@ -4637,7 +4640,8 @@ static const char *open_last_lookups(struct nameidata *nd,
>  		}
>  	}
>  
> -	dentry = lookup_open(nd, file, op);
> +	dentry = lookup_open(&nd->path, file, &nd->last,
> +			     nd->flags, nd->name, op->open_flag, op->mode);
>  	if (IS_ERR(dentry))
>  		return ERR_CAST(dentry);
>  
> -- 
> 2.50.0.107.gf914562f5916.dirty

Sure. I think the churn and pushing nd state through many arguments
would normally be too much to justify the improvement of scope reduction,
but for the goal of getting rid of dentry_create() this is fine.

Reviewed-by: Jori Koolstra <jkoolstra-qWit8jRvyhVmR6Xm/[email protected]>