On Wed, Jul 16, 2025 at 2:47 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.
>
> Also change ovl_workdir_cleanup() to take a dentry for the parent rather
> than an inode.
>
> Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/overlayfs/overlayfs.h | 2 +-
> fs/overlayfs/readdir.c | 36 +++++++++++++-----------------------
> fs/overlayfs/super.c | 6 +-----
> 3 files changed, 15 insertions(+), 29 deletions(-)
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index cff5bb625e9d..f6023442a45c 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 b0f9e5a00c1a..e2d0c314df6c 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,14 +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) {
> - err = ovl_parent_lock(path->dentry, dentry);
> - if (!err) {
> - err = ovl_workdir_cleanup(ofs, dir, path->mnt,
> - dentry, level);
> - ovl_parent_unlock(path->dentry);
> - }
> - }
> + if (dentry->d_inode)
> + err = ovl_workdir_cleanup(ofs, path->dentry, path->mnt,
> + dentry, level);
> dput(dentry);
> if (err)
> break;
> @@ -1156,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 = ovl_parent_lock(parent, dentry);
> + if (err)
> + return err;
> + err = ovl_do_rmdir(ofs, parent->d_inode, dentry);
> + ovl_parent_unlock(parent);
> 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;
> @@ -1184,7 +1179,6 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs)
> int err;
> struct dentry *indexdir = ofs->workdir;
> struct dentry *index = NULL;
> - struct inode *dir = indexdir->d_inode;
> struct path path = { .mnt = ovl_upper_mnt(ofs), .dentry = indexdir };
> LIST_HEAD(list);
> struct ovl_cache_entry *p;
> @@ -1213,11 +1207,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs)
> }
> /* Cleanup leftover from index create/cleanup attempt */
> if (index->d_name.name[0] == '#') {
> - err = ovl_parent_lock(indexdir, index);
> - if (!err) {
> - err = ovl_workdir_cleanup(ofs, dir, path.mnt, index, 1);
> - ovl_parent_unlock(indexdir);
> - }
> + 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 cb2551a155d8..4c3736bf2db4 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -319,11 +319,7 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs,
> return work;
>
> retried = true;
> - err = ovl_parent_lock(ofs->workbasedir, work);
> - if (!err) {
> - err = ovl_workdir_cleanup(ofs, dir, mnt, work, 0);
> - ovl_parent_unlock(ofs->workbasedir);
> - }
> + err = ovl_workdir_cleanup(ofs, ofs->workbasedir, mnt, work, 0);
> dput(work);
> if (err == -EINVAL)
> return ERR_PTR(err);
> --
> 2.49.0
>
