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 >