Re: [PATCH 14/20] ovl: change ovl_workdir_cleanup() to take dir lock as needed.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 11, 2025 at 1:21 AM NeilBrown <neil@xxxxxxxxxx> wrote:
>
> Rather than calling ovl_workdir_cleanup() with the dir already locked,
> change it to take the dir lock only when needed.
>
> Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
> ---
>  fs/overlayfs/overlayfs.h |  2 +-
>  fs/overlayfs/readdir.c   | 30 +++++++++++++-----------------
>  fs/overlayfs/super.c     |  4 +---
>  3 files changed, 15 insertions(+), 21 deletions(-)
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index ec804d6bb2ef..ca74be44dddd 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -738,7 +738,7 @@ void ovl_cleanup_whiteouts(struct ovl_fs *ofs, struct dentry *upper,
>  void ovl_cache_free(struct list_head *list);
>  void ovl_dir_cache_free(struct inode *inode);
>  int ovl_check_d_type_supported(const struct path *realpath);
> -int ovl_workdir_cleanup(struct ovl_fs *ofs, struct inode *dir,
> +int ovl_workdir_cleanup(struct ovl_fs *ofs, struct dentry *parent,
>                         struct vfsmount *mnt, struct dentry *dentry, int level);
>  int ovl_indexdir_cleanup(struct ovl_fs *ofs);
>
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index b3d44bf56c78..6cc5f885e036 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -1096,7 +1096,6 @@ static int ovl_workdir_cleanup_recurse(struct ovl_fs *ofs, const struct path *pa
>                                        int level)
>  {
>         int err;
> -       struct inode *dir = path->dentry->d_inode;
>         LIST_HEAD(list);
>         struct ovl_cache_entry *p;
>         struct ovl_readdir_data rdd = {
> @@ -1139,11 +1138,9 @@ static int ovl_workdir_cleanup_recurse(struct ovl_fs *ofs, const struct path *pa
>                 dentry = ovl_lookup_upper_unlocked(ofs, p->name, path->dentry, p->len);
>                 if (IS_ERR(dentry))
>                         continue;
> -               if (dentry->d_inode) {
> -                       inode_lock_nested(dir, I_MUTEX_PARENT);
> -                       err = ovl_workdir_cleanup(ofs, dir, path->mnt, dentry, level);
> -                       inode_unlock(dir);
> -               }
> +               if (dentry->d_inode)
> +                       err = ovl_workdir_cleanup(ofs, path->dentry, path->mnt,
> +                                                 dentry, level);
>                 dput(dentry);
>                 if (err)
>                         break;
> @@ -1153,24 +1150,25 @@ static int ovl_workdir_cleanup_recurse(struct ovl_fs *ofs, const struct path *pa
>         return err;
>  }
>
> -int ovl_workdir_cleanup(struct ovl_fs *ofs, struct inode *dir,
> +int ovl_workdir_cleanup(struct ovl_fs *ofs, struct dentry *parent,
>                         struct vfsmount *mnt, struct dentry *dentry, int level)
>  {
>         int err;
>
> -       if (!d_is_dir(dentry) || level > 1) {
> -               return ovl_cleanup(ofs, dir, dentry);
> -       }
> +       if (!d_is_dir(dentry) || level > 1)
> +               return ovl_cleanup_unlocked(ofs, parent, dentry);
>
> -       err = ovl_do_rmdir(ofs, dir, dentry);
> +       err = parent_lock(parent, dentry);
> +       if (err)
> +               return err;
> +       err = ovl_do_rmdir(ofs, parent->d_inode, dentry);
> +       parent_unlock(parent);

At this point, the code looks correct,
but it replaces unsafe uses of inode_lock_nested() with correct use of
parent_lock().

Please fix patches 11-13

Thanks,
Amir.

>         if (err) {
>                 struct path path = { .mnt = mnt, .dentry = dentry };
>
> -               inode_unlock(dir);
>                 err = ovl_workdir_cleanup_recurse(ofs, &path, level + 1);
> -               inode_lock_nested(dir, I_MUTEX_PARENT);
>                 if (!err)
> -                       err = ovl_cleanup(ofs, dir, dentry);
> +                       err = ovl_cleanup_unlocked(ofs, parent, dentry);
>         }
>
>         return err;
> @@ -1210,9 +1208,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs)
>                 }
>                 /* Cleanup leftover from index create/cleanup attempt */
>                 if (index->d_name.name[0] == '#') {
> -                       inode_lock_nested(dir, I_MUTEX_PARENT);
> -                       err = ovl_workdir_cleanup(ofs, dir, path.mnt, index, 1);
> -                       inode_unlock(dir);
> +                       err = ovl_workdir_cleanup(ofs, indexdir, path.mnt, index, 1);
>                         if (err)
>                                 break;
>                         goto next;
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 239ae1946edf..23f43f8131dd 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -319,9 +319,7 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs,
>                                 goto out;
>
>                         retried = true;
> -                       inode_lock_nested(dir, I_MUTEX_PARENT);
> -                       err = ovl_workdir_cleanup(ofs, dir, mnt, work, 0);
> -                       inode_unlock(dir);
> +                       err = ovl_workdir_cleanup(ofs, ofs->workbasedir, mnt, work, 0);
>                         dput(work);
>                         if (err == -EINVAL) {
>                                 work = ERR_PTR(err);
> --
> 2.49.0
>





[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux