Re: [PATCH 18/20] ovl: narrow locking in ovl_check_rename_whiteout()

[Amir Goldstein <amir73il@gmail.com>]

On Fri, Jul 11, 2025 at 1:21 AM NeilBrown <neil@brown.name> wrote:
>
> ovl_check_rename_whiteout() now only holds the directory lock when
> needed, and takes it again if necessary.
>
> This makes way for future changes where locks are taken on individual
> dentries rather than the whole directory.
>
> Signed-off-by: NeilBrown <neil@brown.name>

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

> ---
>  fs/overlayfs/super.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 23f43f8131dd..78f4fcfb9ff6 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -559,7 +559,6 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
>  static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
>  {
>         struct dentry *workdir = ofs->workdir;
> -       struct inode *dir = d_inode(workdir);
>         struct dentry *temp;
>         struct dentry *dest;
>         struct dentry *whiteout;
> @@ -580,19 +579,22 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
>         err = PTR_ERR(dest);
>         if (IS_ERR(dest)) {
>                 dput(temp);
> -               goto out_unlock;
> +               parent_unlock(workdir);
> +               return err;
>         }
>
>         /* Name is inline and stable - using snapshot as a copy helper */
>         take_dentry_name_snapshot(&name, temp);
>         err = ovl_do_rename(ofs, workdir, temp, workdir, dest, RENAME_WHITEOUT);
> +       parent_unlock(workdir);
>         if (err) {
>                 if (err == -EINVAL)
>                         err = 0;
>                 goto cleanup_temp;
>         }
>
> -       whiteout = ovl_lookup_upper(ofs, name.name.name, workdir, name.name.len);
> +       whiteout = ovl_lookup_upper_unlocked(ofs, name.name.name,
> +                                            workdir, name.name.len);
>         err = PTR_ERR(whiteout);
>         if (IS_ERR(whiteout))
>                 goto cleanup_temp;
> @@ -601,18 +603,15 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
>
>         /* Best effort cleanup of whiteout and temp file */
>         if (err)
> -               ovl_cleanup(ofs, dir, whiteout);
> +               ovl_cleanup_unlocked(ofs, workdir, whiteout);
>         dput(whiteout);
>
>  cleanup_temp:
> -       ovl_cleanup(ofs, dir, temp);
> +       ovl_cleanup_unlocked(ofs, workdir, temp);
>         release_dentry_name_snapshot(&name);
>         dput(temp);
>         dput(dest);
>
> -out_unlock:
> -       parent_unlock(workdir);
> -
>         return err;
>  }
>
> --
> 2.49.0
>