Re: [PATCH 02/18] VFS: move delegated_inode retry loop into lookup_open()

> Op 01-06-2026 08:37 CEST schreef NeilBrown <[email protected]>:
> 
>  
> From: NeilBrown <[email protected]>
> 
> By moving this retry into lookup_open() we no longer need to pass around
> the delegated_inode pointer.
> 
> Various variable assignments need to be moved out of the declaration
> block so that they can still happen after the goto.
> 
> Signed-off-by: NeilBrown <[email protected]>
> ---
>  fs/namei.c | 42 +++++++++++++++++++++++-------------------
>  1 file changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 998fde251fcf..b00ff3f2faf7 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -4403,17 +4403,23 @@ 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,
> -				  struct delegated_inode *delegated_inode)
> +				  const struct open_flags *op)
>  {
> +	struct delegated_inode delegated_inode = { };
>  	struct mnt_idmap *idmap;
>  	struct dentry *dir = nd->path.dentry;
>  	struct inode *dir_inode = dir->d_inode;
> -	int open_flag = op->open_flag;
> +	int open_flag;
>  	struct dentry *dentry;
> -	int error, create_error = 0;
> -	umode_t mode = op->mode;
> -	bool got_write = false;
> +	int error, create_error;
> +	umode_t mode;
> +	bool got_write;
> +
> +retry:
> +	open_flag = op->open_flag;
> +	got_write = false;
> +	mode = op->mode;
> +	create_error = 0;
>  
>  	if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) {
>  		got_write = !mnt_want_write(nd->path.mnt);
> @@ -4511,7 +4517,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  	/* Negative dentry, just create the file */
>  	if (!dentry->d_inode && (open_flag & O_CREAT)) {
>  		/* but break the directory lease first! */
> -		error = try_break_deleg(dir_inode, LEASE_BREAK_DIR_CREATE, delegated_inode);
> +		error = try_break_deleg(dir_inode, LEASE_BREAK_DIR_CREATE, &delegated_inode);
>  		if (error)
>  			goto out_dput;
>  
> @@ -4546,6 +4552,14 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  	if (got_write)
>  		mnt_drop_write(nd->path.mnt);
>  
> +	if (is_delegated(&delegated_inode)) {
> +		/* Must have come through out_dput */
> +		error = break_deleg_wait(&delegated_inode);
> +
> +		if (!error)
> +			goto retry;
> +	}
> +
>  	return dentry;
>  
>  out_dput:
> @@ -4593,7 +4607,6 @@ static struct dentry *lookup_fast_for_open(struct nameidata *nd, int open_flag)
>  static const char *open_last_lookups(struct nameidata *nd,
>  		   struct file *file, const struct open_flags *op)
>  {
> -	struct delegated_inode delegated_inode = { };
>  	int open_flag = op->open_flag;
>  	struct dentry *dentry;
>  	const char *res;
> @@ -4623,19 +4636,10 @@ static const char *open_last_lookups(struct nameidata *nd,
>  				return ERR_PTR(-ECHILD);
>  		}
>  	}
> -retry:
> -	dentry = lookup_open(nd, file, op, &delegated_inode);
> -
> -	if (IS_ERR(dentry)) {
> -		if (is_delegated(&delegated_inode)) {
> -			int error = break_deleg_wait(&delegated_inode);
>  
> -			if (!error)
> -				goto retry;
> -			return ERR_PTR(error);
> -		}
> +	dentry = lookup_open(nd, file, op);
> +	if (IS_ERR(dentry))
>  		return ERR_CAST(dentry);
> -	}
>  
>  	if (file->f_mode & (FMODE_OPENED | FMODE_CREATED)) {
>  		dput(nd->path.dentry);
> -- 
> 2.50.0.107.gf914562f5916.dirty

It does make sense to me that we deal with the retry-loop in the function where
we call try_break_deleg(). That part is then dealt with on return to
open_last_lookups(). So I think, at least from a cognitive load perspective, this
change makes sense.

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