* Re: [PATCH 09/12] ovl: whiteout locking changes
@ 2025-06-28 4:47 kernel test robot
0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2025-06-28 4:47 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20250624230636.3233059-10-neil@brown.name>
References: <20250624230636.3233059-10-neil@brown.name>
TO: NeilBrown <neil@brown.name>
TO: Miklos Szeredi <miklos@szeredi.hu>
TO: Amir Goldstein <amir73il@gmail.com>
CC: linux-unionfs@vger.kernel.org
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Christian Brauner <brauner@kernel.org>
CC: Jan Kara <jack@suse.cz>
CC: linux-fsdevel@vger.kernel.org
Hi NeilBrown,
kernel test robot noticed the following build warnings:
[auto build test WARNING on brauner-vfs/vfs.all]
[also build test WARNING on next-20250627]
[cannot apply to mszeredi-vfs/overlayfs-next mszeredi-vfs/next linus/master v6.16-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/NeilBrown/ovl-use-is_subdir-for-testing-if-one-thing-is-a-subdir-of-another/20250625-070919
base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link: https://lore.kernel.org/r/20250624230636.3233059-10-neil%40brown.name
patch subject: [PATCH 09/12] ovl: whiteout locking changes
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
config: x86_64-randconfig-161-20250627 (https://download.01.org/0day-ci/archive/20250628/202506281246.X6htntN7-lkp@intel.com/config)
compiler: clang version 20.1.7 (https://github.com/llvm/llvm-project 6146a88f60492b520a36f8f8f3231e15f3cc6082)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202506281246.X6htntN7-lkp@intel.com/
New smatch warnings:
fs/overlayfs/dir.c:118 ovl_whiteout() error: uninitialized symbol 'err'.
Old smatch warnings:
fs/overlayfs/dir.c:58 ovl_cleanup_unlocked() error: uninitialized symbol 'err'.
fs/overlayfs/dir.c:437 ovl_clear_empty() warn: passing zero to 'ERR_PTR'
vim +/err +118 fs/overlayfs/dir.c
e9be9d5e76e348 Miklos Szeredi 2014-10-24 79
c21c839b8448dd Chengguang Xu 2020-04-24 80 static struct dentry *ovl_whiteout(struct ovl_fs *ofs)
e9be9d5e76e348 Miklos Szeredi 2014-10-24 81 {
e9be9d5e76e348 Miklos Szeredi 2014-10-24 82 int err;
e9be9d5e76e348 Miklos Szeredi 2014-10-24 83 struct dentry *whiteout;
c21c839b8448dd Chengguang Xu 2020-04-24 84 struct dentry *workdir = ofs->workdir;
e9be9d5e76e348 Miklos Szeredi 2014-10-24 85 struct inode *wdir = workdir->d_inode;
e9be9d5e76e348 Miklos Szeredi 2014-10-24 86
a33c6f65a2ab05 NeilBrown 2025-06-25 87 mutex_lock(&ofs->whiteout_lock);
c21c839b8448dd Chengguang Xu 2020-04-24 88 if (!ofs->whiteout) {
a33c6f65a2ab05 NeilBrown 2025-06-25 89 inode_lock(wdir);
576bb263450bbb Christian Brauner 2022-04-04 90 whiteout = ovl_lookup_temp(ofs, workdir);
a33c6f65a2ab05 NeilBrown 2025-06-25 91 if (!IS_ERR(whiteout)) {
576bb263450bbb Christian Brauner 2022-04-04 92 err = ovl_do_whiteout(ofs, wdir, whiteout);
e9be9d5e76e348 Miklos Szeredi 2014-10-24 93 if (err) {
e9be9d5e76e348 Miklos Szeredi 2014-10-24 94 dput(whiteout);
e9be9d5e76e348 Miklos Szeredi 2014-10-24 95 whiteout = ERR_PTR(err);
c21c839b8448dd Chengguang Xu 2020-04-24 96 }
a33c6f65a2ab05 NeilBrown 2025-06-25 97 }
a33c6f65a2ab05 NeilBrown 2025-06-25 98 inode_unlock(wdir);
a33c6f65a2ab05 NeilBrown 2025-06-25 99 if (IS_ERR(whiteout))
a33c6f65a2ab05 NeilBrown 2025-06-25 100 goto out;
c21c839b8448dd Chengguang Xu 2020-04-24 101 ofs->whiteout = whiteout;
e9be9d5e76e348 Miklos Szeredi 2014-10-24 102 }
e9be9d5e76e348 Miklos Szeredi 2014-10-24 103
e4599d4b1aeff0 Amir Goldstein 2023-06-17 104 if (!ofs->no_shared_whiteout) {
a33c6f65a2ab05 NeilBrown 2025-06-25 105 inode_lock(wdir);
576bb263450bbb Christian Brauner 2022-04-04 106 whiteout = ovl_lookup_temp(ofs, workdir);
a33c6f65a2ab05 NeilBrown 2025-06-25 107 if (!IS_ERR(whiteout)) {
576bb263450bbb Christian Brauner 2022-04-04 108 err = ovl_do_link(ofs, ofs->whiteout, wdir, whiteout);
a33c6f65a2ab05 NeilBrown 2025-06-25 109 if (err) {
a33c6f65a2ab05 NeilBrown 2025-06-25 110 dput(whiteout);
a33c6f65a2ab05 NeilBrown 2025-06-25 111 whiteout = ERR_PTR(err);
a33c6f65a2ab05 NeilBrown 2025-06-25 112 }
a33c6f65a2ab05 NeilBrown 2025-06-25 113 }
a33c6f65a2ab05 NeilBrown 2025-06-25 114 inode_unlock(wdir);
a33c6f65a2ab05 NeilBrown 2025-06-25 115 if (!IS_ERR(whiteout) || PTR_ERR(whiteout) != -EMLINK)
c21c839b8448dd Chengguang Xu 2020-04-24 116 goto out;
c21c839b8448dd Chengguang Xu 2020-04-24 117
c21c839b8448dd Chengguang Xu 2020-04-24 @118 pr_warn("Failed to link whiteout - disabling whiteout inode sharing(nlink=%u, err=%i)\n",
c21c839b8448dd Chengguang Xu 2020-04-24 119 ofs->whiteout->d_inode->i_nlink, err);
e4599d4b1aeff0 Amir Goldstein 2023-06-17 120 ofs->no_shared_whiteout = true;
c21c839b8448dd Chengguang Xu 2020-04-24 121 }
c21c839b8448dd Chengguang Xu 2020-04-24 122 whiteout = ofs->whiteout;
c21c839b8448dd Chengguang Xu 2020-04-24 123 ofs->whiteout = NULL;
c21c839b8448dd Chengguang Xu 2020-04-24 124 out:
a33c6f65a2ab05 NeilBrown 2025-06-25 125 mutex_unlock(&ofs->whiteout_lock);
e9be9d5e76e348 Miklos Szeredi 2014-10-24 126 return whiteout;
e9be9d5e76e348 Miklos Szeredi 2014-10-24 127 }
e9be9d5e76e348 Miklos Szeredi 2014-10-24 128
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 00/12] ovl: narrow regions protected by directory i_rw_sem @ 2025-06-24 22:54 NeilBrown 2025-06-24 22:55 ` [PATCH 09/12] ovl: whiteout locking changes NeilBrown 0 siblings, 1 reply; 4+ messages in thread From: NeilBrown @ 2025-06-24 22:54 UTC (permalink / raw) To: Miklos Szeredi, Amir Goldstein Cc: linux-unionfs, Alexander Viro, Christian Brauner, Jan Kara, linux-fsdevel This series of patches for overlayfs is primarily focussed on preparing for some proposed changes to directory locking. In the new scheme we wil lock individual dentries in a directory rather than the whole directory. ovl currently will sometimes lock a directory on the upper filesystem and do a few different things while holding the lock. This is incompatible with the new scheme. This series narrows the region of code protected by the directory lock, taking it multiple times when necessary. This theoretically open up the possibilty of other changes happening on the upper filesytem between the unlock and the lock. To some extent the patches guard against that by checking the dentries still have the expect parent after retaking the lock. In general, I think ovl would have trouble if upperfs were being changed independantly, and I don't think the changes here increase the problem in any important way. The first patch in this series doesn't exactly match the above, but it does relate to directory locking and I think it is a sensible simplificaiton. I have tested this with fstests, both generic and unionfs tests. I wouldn't be surprised if I missed something though, so please review carefully. After this series (with any needed changes) lands I will resubmit my change to vfs_rmdir() behaviour to have it drop the lock on error. ovl will be much better positioned to handle that change. It will come with the new "lookup_and_lock" API that I am proposing. Thanks, NeilBrown [PATCH 01/12] ovl: use is_subdir() for testing if one thing is a [PATCH 02/12] ovl: Call ovl_create_temp() and ovl_create_index() [PATCH 03/12] ovl: narrow the locked region in ovl_copy_up_workdir() [PATCH 04/12] ovl: narrow locking in ovl_create_upper() [PATCH 05/12] ovl: narrow locking in ovl_clear_empty() [PATCH 06/12] ovl: narrow locking in ovl_create_over_whiteout() [PATCH 07/12] ovl: narrow locking in ovl_rename() [PATCH 08/12] ovl: narrow locking in ovl_cleanup_whiteouts() [PATCH 09/12] ovl: whiteout locking changes [PATCH 10/12] ovl: narrow locking in ovl_check_rename_whiteout() [PATCH 11/12] ovl: change ovl_create_real() to receive dentry parent [PATCH 12/12] ovl: rename ovl_cleanup_unlocked() to ovl_cleanup() ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 09/12] ovl: whiteout locking changes 2025-06-24 22:54 [PATCH 00/12] ovl: narrow regions protected by directory i_rw_sem NeilBrown @ 2025-06-24 22:55 ` NeilBrown 2025-06-25 18:54 ` Amir Goldstein 0 siblings, 1 reply; 4+ messages in thread From: NeilBrown @ 2025-06-24 22:55 UTC (permalink / raw) To: Miklos Szeredi, Amir Goldstein Cc: linux-unionfs, Alexander Viro, Christian Brauner, Jan Kara, linux-fsdevel ovl_writeout() relies on the workdir i_rwsem to provide exclusive access to ofs->writeout which it manipulates. Rather than depending on this, add a new mutex, "writeout_lock" to explicitly provide the required locking. Then take the lock on workdir only when needed - to lookup the temp name and to do the whiteout or link. So ovl_writeout() and similarly ovl_cleanup_and_writeout() and ovl_workdir_cleanup() are now called without the lock held. This requires changes to ovl_remove_and_whiteout(), ovl_cleanup_index(), ovl_indexdir_cleanup(), and ovl_workdir_create(). Signed-off-by: NeilBrown <neil@brown.name> --- fs/overlayfs/dir.c | 71 +++++++++++++++++++++------------------- fs/overlayfs/overlayfs.h | 2 +- fs/overlayfs/ovl_entry.h | 1 + fs/overlayfs/params.c | 2 ++ fs/overlayfs/readdir.c | 31 ++++++++++-------- fs/overlayfs/super.c | 9 +++-- fs/overlayfs/util.c | 7 ++-- 7 files changed, 67 insertions(+), 56 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 5afe17cee305..78b0d956b0ac 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -77,7 +77,6 @@ struct dentry *ovl_lookup_temp(struct ovl_fs *ofs, struct dentry *workdir) return temp; } -/* caller holds i_mutex on workdir */ static struct dentry *ovl_whiteout(struct ovl_fs *ofs) { int err; @@ -85,47 +84,51 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs) struct dentry *workdir = ofs->workdir; struct inode *wdir = workdir->d_inode; + mutex_lock(&ofs->whiteout_lock); if (!ofs->whiteout) { + inode_lock(wdir); whiteout = ovl_lookup_temp(ofs, workdir); + if (!IS_ERR(whiteout)) { + err = ovl_do_whiteout(ofs, wdir, whiteout); + if (err) { + dput(whiteout); + whiteout = ERR_PTR(err); + } + } + inode_unlock(wdir); if (IS_ERR(whiteout)) goto out; - - err = ovl_do_whiteout(ofs, wdir, whiteout); - if (err) { - dput(whiteout); - whiteout = ERR_PTR(err); - goto out; - } ofs->whiteout = whiteout; } if (!ofs->no_shared_whiteout) { + inode_lock(wdir); whiteout = ovl_lookup_temp(ofs, workdir); - if (IS_ERR(whiteout)) - goto out; - - err = ovl_do_link(ofs, ofs->whiteout, wdir, whiteout); - if (!err) + if (!IS_ERR(whiteout)) { + err = ovl_do_link(ofs, ofs->whiteout, wdir, whiteout); + if (err) { + dput(whiteout); + whiteout = ERR_PTR(err); + } + } + inode_unlock(wdir); + if (!IS_ERR(whiteout) || PTR_ERR(whiteout) != -EMLINK) goto out; - if (err != -EMLINK) { - pr_warn("Failed to link whiteout - disabling whiteout inode sharing(nlink=%u, err=%i)\n", - ofs->whiteout->d_inode->i_nlink, err); - ofs->no_shared_whiteout = true; - } - dput(whiteout); + pr_warn("Failed to link whiteout - disabling whiteout inode sharing(nlink=%u, err=%i)\n", + ofs->whiteout->d_inode->i_nlink, err); + ofs->no_shared_whiteout = true; } whiteout = ofs->whiteout; ofs->whiteout = NULL; out: + mutex_unlock(&ofs->whiteout_lock); return whiteout; } -/* Caller must hold i_mutex on both workdir and dir */ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir, struct dentry *dentry) { - struct inode *wdir = ofs->workdir->d_inode; struct dentry *whiteout; int err; int flags = 0; @@ -138,18 +141,26 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir, if (d_is_dir(dentry)) flags = RENAME_EXCHANGE; - err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, dentry, flags); + err = ovl_lock_rename_workdir(ofs->workdir, dir); + if (!err) { + if (whiteout->d_parent == ofs->workdir) + err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, + dentry, flags); + else + err = -EINVAL; + unlock_rename(ofs->workdir, dir); + } if (err) goto kill_whiteout; if (flags) - ovl_cleanup(ofs, wdir, dentry); + ovl_cleanup_unlocked(ofs, ofs->workdir, dentry); out: dput(whiteout); return err; kill_whiteout: - ovl_cleanup(ofs, wdir, whiteout); + ovl_cleanup_unlocked(ofs, ofs->workdir, whiteout); goto out; } @@ -777,15 +788,11 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, goto out; } - err = ovl_lock_rename_workdir(workdir, upperdir); - if (err) - goto out_dput; - - upper = ovl_lookup_upper(ofs, dentry->d_name.name, upperdir, - dentry->d_name.len); + upper = ovl_lookup_upper_unlocked(ofs, dentry->d_name.name, upperdir, + dentry->d_name.len); err = PTR_ERR(upper); if (IS_ERR(upper)) - goto out_unlock; + goto out_dput; err = -ESTALE; if ((opaquedir && upper != opaquedir) || @@ -803,8 +810,6 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, d_drop(dentry); out_dput_upper: dput(upper); -out_unlock: - unlock_rename(workdir, upperdir); out_dput: dput(opaquedir); out: diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 508003e26e08..25378b81251e 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -732,7 +732,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/ovl_entry.h b/fs/overlayfs/ovl_entry.h index afb7762f873f..4c1bae935ced 100644 --- a/fs/overlayfs/ovl_entry.h +++ b/fs/overlayfs/ovl_entry.h @@ -88,6 +88,7 @@ struct ovl_fs { /* Shared whiteout cache */ struct dentry *whiteout; bool no_shared_whiteout; + struct mutex whiteout_lock; /* r/o snapshot of upperdir sb's only taken on volatile mounts */ errseq_t errseq; }; diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c index f42488c01957..cb1a17c066cd 100644 --- a/fs/overlayfs/params.c +++ b/fs/overlayfs/params.c @@ -797,6 +797,8 @@ int ovl_init_fs_context(struct fs_context *fc) fc->s_fs_info = ofs; fc->fs_private = ctx; fc->ops = &ovl_context_ops; + + mutex_init(&ofs->whiteout_lock); return 0; out_err: diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index 2a222b8185a3..fd98444dacef 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -1141,7 +1141,8 @@ static int ovl_workdir_cleanup_recurse(struct ovl_fs *ofs, const struct path *pa if (IS_ERR(dentry)) continue; if (dentry->d_inode) - err = ovl_workdir_cleanup(ofs, dir, path->mnt, dentry, level); + err = ovl_workdir_cleanup(ofs, path->dentry, path->mnt, + dentry, level); dput(dentry); if (err) break; @@ -1152,24 +1153,27 @@ 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); + return ovl_cleanup_unlocked(ofs, parent, dentry); } - err = ovl_do_rmdir(ofs, dir, dentry); + inode_lock_nested(parent->d_inode, I_MUTEX_PARENT); + if (dentry->d_parent == parent) + err = ovl_do_rmdir(ofs, parent->d_inode, dentry); + else + err = -EINVAL; + inode_unlock(parent->d_inode); 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; @@ -1180,7 +1184,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; @@ -1194,7 +1197,6 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) if (err) goto out; - inode_lock_nested(dir, I_MUTEX_PARENT); list_for_each_entry(p, &list, l_node) { if (p->name[0] == '.') { if (p->len == 1) @@ -1202,7 +1204,8 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) if (p->len == 2 && p->name[1] == '.') continue; } - index = ovl_lookup_upper(ofs, p->name, indexdir, p->len); + index = ovl_lookup_upper_unlocked(ofs, p->name, indexdir, + p->len); if (IS_ERR(index)) { err = PTR_ERR(index); index = NULL; @@ -1210,7 +1213,8 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) } /* Cleanup leftover from index create/cleanup attempt */ if (index->d_name.name[0] == '#') { - err = ovl_workdir_cleanup(ofs, dir, path.mnt, index, 1); + err = ovl_workdir_cleanup(ofs, indexdir, path.mnt, + index, 1); if (err) break; goto next; @@ -1220,7 +1224,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) goto next; } else if (err == -ESTALE) { /* Cleanup stale index entries */ - err = ovl_cleanup(ofs, dir, index); + err = ovl_cleanup_unlocked(ofs, indexdir, index); } else if (err != -ENOENT) { /* * Abort mount to avoid corrupting the index if @@ -1236,7 +1240,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) err = ovl_cleanup_and_whiteout(ofs, indexdir, index); } else { /* Cleanup orphan index entries */ - err = ovl_cleanup(ofs, dir, index); + err = ovl_cleanup_unlocked(ofs, indexdir, index); } if (err) @@ -1247,7 +1251,6 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) index = NULL; } dput(index); - inode_unlock(dir); out: ovl_cache_free(&list); if (err) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 576b5c3b537c..3583e359655f 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -299,8 +299,8 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, int err; bool retried = false; - inode_lock_nested(dir, I_MUTEX_PARENT); retry: + inode_lock_nested(dir, I_MUTEX_PARENT); work = ovl_lookup_upper(ofs, name, ofs->workbasedir, strlen(name)); if (!IS_ERR(work)) { @@ -311,6 +311,7 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, if (work->d_inode) { err = -EEXIST; + inode_unlock(dir); if (retried) goto out_dput; @@ -318,7 +319,8 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, goto out_unlock; retried = true; - err = ovl_workdir_cleanup(ofs, dir, mnt, work, 0); + err = ovl_workdir_cleanup(ofs, ofs->workbasedir, mnt, + work, 0); dput(work); if (err == -EINVAL) { work = ERR_PTR(err); @@ -328,6 +330,7 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, } work = ovl_do_mkdir(ofs, dir, work, attr.ia_mode); + inode_unlock(dir); err = PTR_ERR(work); if (IS_ERR(work)) goto out_err; @@ -365,11 +368,11 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, if (err) goto out_dput; } else { + inode_unlock(dir); err = PTR_ERR(work); goto out_err; } out_unlock: - inode_unlock(dir); return work; out_dput: diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 2b4754c645ee..565f7d8c0147 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -1071,7 +1071,6 @@ static void ovl_cleanup_index(struct dentry *dentry) { struct ovl_fs *ofs = OVL_FS(dentry->d_sb); struct dentry *indexdir = ovl_indexdir(dentry->d_sb); - struct inode *dir = indexdir->d_inode; struct dentry *lowerdentry = ovl_dentry_lower(dentry); struct dentry *upperdentry = ovl_dentry_upper(dentry); struct dentry *index = NULL; @@ -1107,8 +1106,7 @@ static void ovl_cleanup_index(struct dentry *dentry) goto out; } - inode_lock_nested(dir, I_MUTEX_PARENT); - index = ovl_lookup_upper(ofs, name.name, indexdir, name.len); + index = ovl_lookup_upper_unlocked(ofs, name.name, indexdir, name.len); err = PTR_ERR(index); if (IS_ERR(index)) { index = NULL; @@ -1118,10 +1116,9 @@ static void ovl_cleanup_index(struct dentry *dentry) indexdir, index); } else { /* Cleanup orphan index entries */ - err = ovl_cleanup(ofs, dir, index); + err = ovl_cleanup_unlocked(ofs, indexdir, index); } - inode_unlock(dir); if (err) goto fail; -- 2.49.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 09/12] ovl: whiteout locking changes 2025-06-24 22:55 ` [PATCH 09/12] ovl: whiteout locking changes NeilBrown @ 2025-06-25 18:54 ` Amir Goldstein 2025-07-02 2:21 ` NeilBrown 0 siblings, 1 reply; 4+ messages in thread From: Amir Goldstein @ 2025-06-25 18:54 UTC (permalink / raw) To: NeilBrown Cc: Miklos Szeredi, linux-unionfs, Alexander Viro, Christian Brauner, Jan Kara, linux-fsdevel On Wed, Jun 25, 2025 at 1:07 AM NeilBrown <neil@brown.name> wrote: > > ovl_writeout() relies on the workdir i_rwsem to provide exclusive access > to ofs->writeout which it manipulates. Rather than depending on this, typo writeout/whiteout all over this commit message > add a new mutex, "writeout_lock" to explicitly provide the required > locking. > > Then take the lock on workdir only when needed - to lookup the temp name > and to do the whiteout or link. So ovl_writeout() and similarly > ovl_cleanup_and_writeout() and ovl_workdir_cleanup() are now called > without the lock held. This requires changes to > ovl_remove_and_whiteout(), ovl_cleanup_index(), ovl_indexdir_cleanup(), > and ovl_workdir_create(). > > Signed-off-by: NeilBrown <neil@brown.name> > --- > fs/overlayfs/dir.c | 71 +++++++++++++++++++++------------------- > fs/overlayfs/overlayfs.h | 2 +- > fs/overlayfs/ovl_entry.h | 1 + > fs/overlayfs/params.c | 2 ++ > fs/overlayfs/readdir.c | 31 ++++++++++-------- > fs/overlayfs/super.c | 9 +++-- > fs/overlayfs/util.c | 7 ++-- > 7 files changed, 67 insertions(+), 56 deletions(-) > > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c > index 5afe17cee305..78b0d956b0ac 100644 > --- a/fs/overlayfs/dir.c > +++ b/fs/overlayfs/dir.c > @@ -77,7 +77,6 @@ struct dentry *ovl_lookup_temp(struct ovl_fs *ofs, struct dentry *workdir) > return temp; > } > > -/* caller holds i_mutex on workdir */ > static struct dentry *ovl_whiteout(struct ovl_fs *ofs) > { > int err; > @@ -85,47 +84,51 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs) > struct dentry *workdir = ofs->workdir; > struct inode *wdir = workdir->d_inode; > > + mutex_lock(&ofs->whiteout_lock); > if (!ofs->whiteout) { > + inode_lock(wdir); > whiteout = ovl_lookup_temp(ofs, workdir); > + if (!IS_ERR(whiteout)) { > + err = ovl_do_whiteout(ofs, wdir, whiteout); > + if (err) { > + dput(whiteout); > + whiteout = ERR_PTR(err); > + } > + } > + inode_unlock(wdir); > if (IS_ERR(whiteout)) > goto out; > - > - err = ovl_do_whiteout(ofs, wdir, whiteout); > - if (err) { > - dput(whiteout); > - whiteout = ERR_PTR(err); > - goto out; > - } > ofs->whiteout = whiteout; > } > > if (!ofs->no_shared_whiteout) { > + inode_lock(wdir); > whiteout = ovl_lookup_temp(ofs, workdir); > - if (IS_ERR(whiteout)) > - goto out; > - > - err = ovl_do_link(ofs, ofs->whiteout, wdir, whiteout); > - if (!err) > + if (!IS_ERR(whiteout)) { > + err = ovl_do_link(ofs, ofs->whiteout, wdir, whiteout); > + if (err) { > + dput(whiteout); > + whiteout = ERR_PTR(err); > + } > + } > + inode_unlock(wdir); > + if (!IS_ERR(whiteout) || PTR_ERR(whiteout) != -EMLINK) > goto out; > > - if (err != -EMLINK) { > - pr_warn("Failed to link whiteout - disabling whiteout inode sharing(nlink=%u, err=%i)\n", > - ofs->whiteout->d_inode->i_nlink, err); > - ofs->no_shared_whiteout = true; > - } > - dput(whiteout); Where did this dput go? > + pr_warn("Failed to link whiteout - disabling whiteout inode sharing(nlink=%u, err=%i)\n", > + ofs->whiteout->d_inode->i_nlink, err); > + ofs->no_shared_whiteout = true; > } > whiteout = ofs->whiteout; > ofs->whiteout = NULL; > out: > + mutex_unlock(&ofs->whiteout_lock); > return whiteout; > } > > -/* Caller must hold i_mutex on both workdir and dir */ > int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir, > struct dentry *dentry) > { > - struct inode *wdir = ofs->workdir->d_inode; > struct dentry *whiteout; > int err; > int flags = 0; > @@ -138,18 +141,26 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir, > if (d_is_dir(dentry)) > flags = RENAME_EXCHANGE; > > - err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, dentry, flags); > + err = ovl_lock_rename_workdir(ofs->workdir, dir); > + if (!err) { > + if (whiteout->d_parent == ofs->workdir) > + err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, > + dentry, flags); > + else > + err = -EINVAL; > + unlock_rename(ofs->workdir, dir); > + } > if (err) > goto kill_whiteout; > if (flags) > - ovl_cleanup(ofs, wdir, dentry); > + ovl_cleanup_unlocked(ofs, ofs->workdir, dentry); > > out: > dput(whiteout); > return err; > > kill_whiteout: > - ovl_cleanup(ofs, wdir, whiteout); > + ovl_cleanup_unlocked(ofs, ofs->workdir, whiteout); > goto out; > } > > @@ -777,15 +788,11 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, > goto out; > } > > - err = ovl_lock_rename_workdir(workdir, upperdir); > - if (err) > - goto out_dput; > - > - upper = ovl_lookup_upper(ofs, dentry->d_name.name, upperdir, > - dentry->d_name.len); > + upper = ovl_lookup_upper_unlocked(ofs, dentry->d_name.name, upperdir, > + dentry->d_name.len); > err = PTR_ERR(upper); > if (IS_ERR(upper)) > - goto out_unlock; > + goto out_dput; > > err = -ESTALE; > if ((opaquedir && upper != opaquedir) || > @@ -803,8 +810,6 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, > d_drop(dentry); > out_dput_upper: > dput(upper); > -out_unlock: > - unlock_rename(workdir, upperdir); > out_dput: > dput(opaquedir); > out: > diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h > index 508003e26e08..25378b81251e 100644 > --- a/fs/overlayfs/overlayfs.h > +++ b/fs/overlayfs/overlayfs.h > @@ -732,7 +732,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/ovl_entry.h b/fs/overlayfs/ovl_entry.h > index afb7762f873f..4c1bae935ced 100644 > --- a/fs/overlayfs/ovl_entry.h > +++ b/fs/overlayfs/ovl_entry.h > @@ -88,6 +88,7 @@ struct ovl_fs { > /* Shared whiteout cache */ > struct dentry *whiteout; > bool no_shared_whiteout; > + struct mutex whiteout_lock; > /* r/o snapshot of upperdir sb's only taken on volatile mounts */ > errseq_t errseq; > }; > diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c > index f42488c01957..cb1a17c066cd 100644 > --- a/fs/overlayfs/params.c > +++ b/fs/overlayfs/params.c > @@ -797,6 +797,8 @@ int ovl_init_fs_context(struct fs_context *fc) > fc->s_fs_info = ofs; > fc->fs_private = ctx; > fc->ops = &ovl_context_ops; > + > + mutex_init(&ofs->whiteout_lock); > return 0; > > out_err: > diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c > index 2a222b8185a3..fd98444dacef 100644 > --- a/fs/overlayfs/readdir.c > +++ b/fs/overlayfs/readdir.c > @@ -1141,7 +1141,8 @@ static int ovl_workdir_cleanup_recurse(struct ovl_fs *ofs, const struct path *pa > if (IS_ERR(dentry)) > continue; > if (dentry->d_inode) > - err = ovl_workdir_cleanup(ofs, dir, path->mnt, dentry, level); > + err = ovl_workdir_cleanup(ofs, path->dentry, path->mnt, > + dentry, level); > dput(dentry); > if (err) > break; > @@ -1152,24 +1153,27 @@ 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); > + return ovl_cleanup_unlocked(ofs, parent, dentry); > } > > - err = ovl_do_rmdir(ofs, dir, dentry); > + inode_lock_nested(parent->d_inode, I_MUTEX_PARENT); > + if (dentry->d_parent == parent) > + err = ovl_do_rmdir(ofs, parent->d_inode, dentry); > + else > + err = -EINVAL; > + inode_unlock(parent->d_inode); > 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; > @@ -1180,7 +1184,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; > @@ -1194,7 +1197,6 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) > if (err) > goto out; > > - inode_lock_nested(dir, I_MUTEX_PARENT); > list_for_each_entry(p, &list, l_node) { > if (p->name[0] == '.') { > if (p->len == 1) > @@ -1202,7 +1204,8 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) > if (p->len == 2 && p->name[1] == '.') > continue; > } > - index = ovl_lookup_upper(ofs, p->name, indexdir, p->len); > + index = ovl_lookup_upper_unlocked(ofs, p->name, indexdir, > + p->len); > if (IS_ERR(index)) { > err = PTR_ERR(index); > index = NULL; > @@ -1210,7 +1213,8 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) > } > /* Cleanup leftover from index create/cleanup attempt */ > if (index->d_name.name[0] == '#') { > - err = ovl_workdir_cleanup(ofs, dir, path.mnt, index, 1); > + err = ovl_workdir_cleanup(ofs, indexdir, path.mnt, > + index, 1); > if (err) > break; > goto next; > @@ -1220,7 +1224,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) > goto next; > } else if (err == -ESTALE) { > /* Cleanup stale index entries */ > - err = ovl_cleanup(ofs, dir, index); > + err = ovl_cleanup_unlocked(ofs, indexdir, index); > } else if (err != -ENOENT) { > /* > * Abort mount to avoid corrupting the index if > @@ -1236,7 +1240,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) > err = ovl_cleanup_and_whiteout(ofs, indexdir, index); > } else { > /* Cleanup orphan index entries */ > - err = ovl_cleanup(ofs, dir, index); > + err = ovl_cleanup_unlocked(ofs, indexdir, index); > } > > if (err) > @@ -1247,7 +1251,6 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) > index = NULL; > } > dput(index); > - inode_unlock(dir); > out: > ovl_cache_free(&list); > if (err) > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > index 576b5c3b537c..3583e359655f 100644 > --- a/fs/overlayfs/super.c > +++ b/fs/overlayfs/super.c > @@ -299,8 +299,8 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, > int err; > bool retried = false; > > - inode_lock_nested(dir, I_MUTEX_PARENT); > retry: > + inode_lock_nested(dir, I_MUTEX_PARENT); > work = ovl_lookup_upper(ofs, name, ofs->workbasedir, strlen(name)); > > if (!IS_ERR(work)) { > @@ -311,6 +311,7 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, > > if (work->d_inode) { > err = -EEXIST; > + inode_unlock(dir); > if (retried) > goto out_dput; > > @@ -318,7 +319,8 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, > goto out_unlock; > > retried = true; > - err = ovl_workdir_cleanup(ofs, dir, mnt, work, 0); > + err = ovl_workdir_cleanup(ofs, ofs->workbasedir, mnt, > + work, 0); > dput(work); > if (err == -EINVAL) { > work = ERR_PTR(err); > @@ -328,6 +330,7 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, > } > > work = ovl_do_mkdir(ofs, dir, work, attr.ia_mode); > + inode_unlock(dir); > err = PTR_ERR(work); > if (IS_ERR(work)) > goto out_err; > @@ -365,11 +368,11 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, > if (err) > goto out_dput; > } else { > + inode_unlock(dir); > err = PTR_ERR(work); > goto out_err; > } > out_unlock: This label name is now misleading > - inode_unlock(dir); > return work; > Thanks, Amir. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 09/12] ovl: whiteout locking changes 2025-06-25 18:54 ` Amir Goldstein @ 2025-07-02 2:21 ` NeilBrown 0 siblings, 0 replies; 4+ messages in thread From: NeilBrown @ 2025-07-02 2:21 UTC (permalink / raw) To: Amir Goldstein Cc: Miklos Szeredi, linux-unionfs, Alexander Viro, Christian Brauner, Jan Kara, linux-fsdevel On Thu, 26 Jun 2025, Amir Goldstein wrote: > On Wed, Jun 25, 2025 at 1:07 AM NeilBrown <neil@brown.name> wrote: > > > > ovl_writeout() relies on the workdir i_rwsem to provide exclusive access > > to ofs->writeout which it manipulates. Rather than depending on this, > > typo writeout/whiteout all over this commit message Fixed - thanks. > > > add a new mutex, "writeout_lock" to explicitly provide the required > > locking. > > > > Then take the lock on workdir only when needed - to lookup the temp name > > and to do the whiteout or link. So ovl_writeout() and similarly > > ovl_cleanup_and_writeout() and ovl_workdir_cleanup() are now called > > without the lock held. This requires changes to > > ovl_remove_and_whiteout(), ovl_cleanup_index(), ovl_indexdir_cleanup(), > > and ovl_workdir_create(). > > > > Signed-off-by: NeilBrown <neil@brown.name> > > --- > > fs/overlayfs/dir.c | 71 +++++++++++++++++++++------------------- > > fs/overlayfs/overlayfs.h | 2 +- > > fs/overlayfs/ovl_entry.h | 1 + > > fs/overlayfs/params.c | 2 ++ > > fs/overlayfs/readdir.c | 31 ++++++++++-------- > > fs/overlayfs/super.c | 9 +++-- > > fs/overlayfs/util.c | 7 ++-- > > 7 files changed, 67 insertions(+), 56 deletions(-) > > > > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c > > index 5afe17cee305..78b0d956b0ac 100644 > > --- a/fs/overlayfs/dir.c > > +++ b/fs/overlayfs/dir.c > > @@ -77,7 +77,6 @@ struct dentry *ovl_lookup_temp(struct ovl_fs *ofs, struct dentry *workdir) > > return temp; > > } > > > > -/* caller holds i_mutex on workdir */ > > static struct dentry *ovl_whiteout(struct ovl_fs *ofs) > > { > > int err; > > @@ -85,47 +84,51 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs) > > struct dentry *workdir = ofs->workdir; > > struct inode *wdir = workdir->d_inode; > > > > + mutex_lock(&ofs->whiteout_lock); > > if (!ofs->whiteout) { > > + inode_lock(wdir); > > whiteout = ovl_lookup_temp(ofs, workdir); > > + if (!IS_ERR(whiteout)) { > > + err = ovl_do_whiteout(ofs, wdir, whiteout); > > + if (err) { > > + dput(whiteout); > > + whiteout = ERR_PTR(err); > > + } > > + } > > + inode_unlock(wdir); > > if (IS_ERR(whiteout)) > > goto out; > > - > > - err = ovl_do_whiteout(ofs, wdir, whiteout); > > - if (err) { > > - dput(whiteout); > > - whiteout = ERR_PTR(err); > > - goto out; > > - } > > ofs->whiteout = whiteout; > > } > > > > if (!ofs->no_shared_whiteout) { > > + inode_lock(wdir); > > whiteout = ovl_lookup_temp(ofs, workdir); > > - if (IS_ERR(whiteout)) > > - goto out; > > - > > - err = ovl_do_link(ofs, ofs->whiteout, wdir, whiteout); > > - if (!err) > > + if (!IS_ERR(whiteout)) { > > + err = ovl_do_link(ofs, ofs->whiteout, wdir, whiteout); > > + if (err) { > > + dput(whiteout); > > + whiteout = ERR_PTR(err); > > + } > > + } > > + inode_unlock(wdir); > > + if (!IS_ERR(whiteout) || PTR_ERR(whiteout) != -EMLINK) > > goto out; > > > > - if (err != -EMLINK) { > > - pr_warn("Failed to link whiteout - disabling whiteout inode sharing(nlink=%u, err=%i)\n", > > - ofs->whiteout->d_inode->i_nlink, err); > > - ofs->no_shared_whiteout = true; > > - } > > - dput(whiteout); > > Where did this dput go? 13 lines up in the patch. It is called when ovl_do_link() fails. It is now closer to the ovl_do_link() call. > > > + pr_warn("Failed to link whiteout - disabling whiteout inode sharing(nlink=%u, err=%i)\n", > > + ofs->whiteout->d_inode->i_nlink, err); > > + ofs->no_shared_whiteout = true; > > } > > whiteout = ofs->whiteout; > > ofs->whiteout = NULL; > > out: > > + mutex_unlock(&ofs->whiteout_lock); > > return whiteout; > > } > > > > -/* Caller must hold i_mutex on both workdir and dir */ > > int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir, > > struct dentry *dentry) > > { > > - struct inode *wdir = ofs->workdir->d_inode; > > struct dentry *whiteout; > > int err; > > int flags = 0; > > @@ -138,18 +141,26 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir, > > if (d_is_dir(dentry)) > > flags = RENAME_EXCHANGE; > > > > - err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, dentry, flags); > > + err = ovl_lock_rename_workdir(ofs->workdir, dir); > > + if (!err) { > > + if (whiteout->d_parent == ofs->workdir) > > + err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, > > + dentry, flags); > > + else > > + err = -EINVAL; > > + unlock_rename(ofs->workdir, dir); > > + } > > if (err) > > goto kill_whiteout; > > if (flags) > > - ovl_cleanup(ofs, wdir, dentry); > > + ovl_cleanup_unlocked(ofs, ofs->workdir, dentry); > > > > out: > > dput(whiteout); > > return err; > > > > kill_whiteout: > > - ovl_cleanup(ofs, wdir, whiteout); > > + ovl_cleanup_unlocked(ofs, ofs->workdir, whiteout); > > goto out; > > } > > > > @@ -777,15 +788,11 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, > > goto out; > > } > > > > - err = ovl_lock_rename_workdir(workdir, upperdir); > > - if (err) > > - goto out_dput; > > - > > - upper = ovl_lookup_upper(ofs, dentry->d_name.name, upperdir, > > - dentry->d_name.len); > > + upper = ovl_lookup_upper_unlocked(ofs, dentry->d_name.name, upperdir, > > + dentry->d_name.len); > > err = PTR_ERR(upper); > > if (IS_ERR(upper)) > > - goto out_unlock; > > + goto out_dput; > > > > err = -ESTALE; > > if ((opaquedir && upper != opaquedir) || > > @@ -803,8 +810,6 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, > > d_drop(dentry); > > out_dput_upper: > > dput(upper); > > -out_unlock: > > - unlock_rename(workdir, upperdir); > > out_dput: > > dput(opaquedir); > > out: > > diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h > > index 508003e26e08..25378b81251e 100644 > > --- a/fs/overlayfs/overlayfs.h > > +++ b/fs/overlayfs/overlayfs.h > > @@ -732,7 +732,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/ovl_entry.h b/fs/overlayfs/ovl_entry.h > > index afb7762f873f..4c1bae935ced 100644 > > --- a/fs/overlayfs/ovl_entry.h > > +++ b/fs/overlayfs/ovl_entry.h > > @@ -88,6 +88,7 @@ struct ovl_fs { > > /* Shared whiteout cache */ > > struct dentry *whiteout; > > bool no_shared_whiteout; > > + struct mutex whiteout_lock; > > /* r/o snapshot of upperdir sb's only taken on volatile mounts */ > > errseq_t errseq; > > }; > > diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c > > index f42488c01957..cb1a17c066cd 100644 > > --- a/fs/overlayfs/params.c > > +++ b/fs/overlayfs/params.c > > @@ -797,6 +797,8 @@ int ovl_init_fs_context(struct fs_context *fc) > > fc->s_fs_info = ofs; > > fc->fs_private = ctx; > > fc->ops = &ovl_context_ops; > > + > > + mutex_init(&ofs->whiteout_lock); > > return 0; > > > > out_err: > > diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c > > index 2a222b8185a3..fd98444dacef 100644 > > --- a/fs/overlayfs/readdir.c > > +++ b/fs/overlayfs/readdir.c > > @@ -1141,7 +1141,8 @@ static int ovl_workdir_cleanup_recurse(struct ovl_fs *ofs, const struct path *pa > > if (IS_ERR(dentry)) > > continue; > > if (dentry->d_inode) > > - err = ovl_workdir_cleanup(ofs, dir, path->mnt, dentry, level); > > + err = ovl_workdir_cleanup(ofs, path->dentry, path->mnt, > > + dentry, level); > > dput(dentry); > > if (err) > > break; > > @@ -1152,24 +1153,27 @@ 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); > > + return ovl_cleanup_unlocked(ofs, parent, dentry); > > } > > > > - err = ovl_do_rmdir(ofs, dir, dentry); > > + inode_lock_nested(parent->d_inode, I_MUTEX_PARENT); > > + if (dentry->d_parent == parent) > > + err = ovl_do_rmdir(ofs, parent->d_inode, dentry); > > + else > > + err = -EINVAL; > > + inode_unlock(parent->d_inode); > > 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; > > @@ -1180,7 +1184,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; > > @@ -1194,7 +1197,6 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) > > if (err) > > goto out; > > > > - inode_lock_nested(dir, I_MUTEX_PARENT); > > list_for_each_entry(p, &list, l_node) { > > if (p->name[0] == '.') { > > if (p->len == 1) > > @@ -1202,7 +1204,8 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) > > if (p->len == 2 && p->name[1] == '.') > > continue; > > } > > - index = ovl_lookup_upper(ofs, p->name, indexdir, p->len); > > + index = ovl_lookup_upper_unlocked(ofs, p->name, indexdir, > > + p->len); > > if (IS_ERR(index)) { > > err = PTR_ERR(index); > > index = NULL; > > @@ -1210,7 +1213,8 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) > > } > > /* Cleanup leftover from index create/cleanup attempt */ > > if (index->d_name.name[0] == '#') { > > - err = ovl_workdir_cleanup(ofs, dir, path.mnt, index, 1); > > + err = ovl_workdir_cleanup(ofs, indexdir, path.mnt, > > + index, 1); > > if (err) > > break; > > goto next; > > @@ -1220,7 +1224,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) > > goto next; > > } else if (err == -ESTALE) { > > /* Cleanup stale index entries */ > > - err = ovl_cleanup(ofs, dir, index); > > + err = ovl_cleanup_unlocked(ofs, indexdir, index); > > } else if (err != -ENOENT) { > > /* > > * Abort mount to avoid corrupting the index if > > @@ -1236,7 +1240,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) > > err = ovl_cleanup_and_whiteout(ofs, indexdir, index); > > } else { > > /* Cleanup orphan index entries */ > > - err = ovl_cleanup(ofs, dir, index); > > + err = ovl_cleanup_unlocked(ofs, indexdir, index); > > } > > > > if (err) > > @@ -1247,7 +1251,6 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) > > index = NULL; > > } > > dput(index); > > - inode_unlock(dir); > > out: > > ovl_cache_free(&list); > > if (err) > > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > > index 576b5c3b537c..3583e359655f 100644 > > --- a/fs/overlayfs/super.c > > +++ b/fs/overlayfs/super.c > > @@ -299,8 +299,8 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, > > int err; > > bool retried = false; > > > > - inode_lock_nested(dir, I_MUTEX_PARENT); > > retry: > > + inode_lock_nested(dir, I_MUTEX_PARENT); > > work = ovl_lookup_upper(ofs, name, ofs->workbasedir, strlen(name)); > > > > if (!IS_ERR(work)) { > > @@ -311,6 +311,7 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, > > > > if (work->d_inode) { > > err = -EEXIST; > > + inode_unlock(dir); > > if (retried) > > goto out_dput; > > > > @@ -318,7 +319,8 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, > > goto out_unlock; > > > > retried = true; > > - err = ovl_workdir_cleanup(ofs, dir, mnt, work, 0); > > + err = ovl_workdir_cleanup(ofs, ofs->workbasedir, mnt, > > + work, 0); > > dput(work); > > if (err == -EINVAL) { > > work = ERR_PTR(err); > > @@ -328,6 +330,7 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, > > } > > > > work = ovl_do_mkdir(ofs, dir, work, attr.ia_mode); > > + inode_unlock(dir); > > err = PTR_ERR(work); > > if (IS_ERR(work)) > > goto out_err; > > @@ -365,11 +368,11 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, > > if (err) > > goto out_dput; > > } else { > > + inode_unlock(dir); > > err = PTR_ERR(work); > > goto out_err; > > } > > out_unlock: > > This label name is now misleading Yep - I'll fix it. Thanks, NeilBrown > > > - inode_unlock(dir); > > return work; > > > > Thanks, > Amir. > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-07-02 2:21 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-28 4:47 [PATCH 09/12] ovl: whiteout locking changes kernel test robot -- strict thread matches above, loose matches on Subject: below -- 2025-06-24 22:54 [PATCH 00/12] ovl: narrow regions protected by directory i_rw_sem NeilBrown 2025-06-24 22:55 ` [PATCH 09/12] ovl: whiteout locking changes NeilBrown 2025-06-25 18:54 ` Amir Goldstein 2025-07-02 2:21 ` NeilBrown
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.