* [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-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
* 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.