From: NeilBrown <neil@brown.name>
To: Miklos Szeredi <miklos@szeredi.hu>, Amir Goldstein <amir73il@gmail.com>
Cc: linux-unionfs@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: [PATCH 16/20] ovl: change ovl_cleanup_and_whiteout() to take rename lock as needed
Date: Fri, 11 Jul 2025 09:03:46 +1000 [thread overview]
Message-ID: <20250710232109.3014537-17-neil@brown.name> (raw)
In-Reply-To: <20250710232109.3014537-1-neil@brown.name>
Rather than locking the directory(s) before calling
ovl_cleanup_and_whiteout(), change it (and ovl_whiteout()) to do the
locking, so the locking can be fine grained as will be needed for
proposed locking changes.
Sometimes this is called to whiteout something in the index dir, in
which case only that dir must be locked. In one case it is called on
something in an upperdir, so two directories must be locked. We use
ovl_lock_rename_workdir() for this and remove the restriction that
upperdir cannot be indexdir - because now sometimes it is.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/overlayfs/dir.c | 20 +++++++++-----------
fs/overlayfs/readdir.c | 3 ---
fs/overlayfs/util.c | 7 -------
3 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 8580cd5c61e4..086719129be3 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,6 +84,7 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs)
struct dentry *workdir = ofs->workdir;
struct inode *wdir = workdir->d_inode;
+ inode_lock_nested(wdir, I_MUTEX_PARENT);
if (!ofs->whiteout) {
whiteout = ovl_lookup_temp(ofs, workdir);
if (IS_ERR(whiteout))
@@ -118,14 +118,13 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs)
whiteout = ofs->whiteout;
ofs->whiteout = NULL;
out:
+ inode_unlock(wdir);
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 +137,22 @@ 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, whiteout, dir, dentry);
+ if (!err) {
+ err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, dentry, flags);
+ 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;
}
@@ -782,10 +785,6 @@ static int ovl_remove_and_whiteout(struct dentry *dentry,
goto out_dput_upper;
}
- err = ovl_lock_rename_workdir(workdir, NULL, upperdir, upper);
- if (err)
- goto out_dput_upper;
-
err = ovl_cleanup_and_whiteout(ofs, upperdir, upper);
if (err)
goto out_d_drop;
@@ -793,7 +792,6 @@ static int ovl_remove_and_whiteout(struct dentry *dentry,
ovl_dir_modified(dentry->d_parent, true);
out_d_drop:
d_drop(dentry);
- unlock_rename(workdir, upperdir);
out_dput_upper:
dput(upper);
out_dput:
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 6cc5f885e036..4127d1f160b3 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -1179,7 +1179,6 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs)
int err;
struct dentry *indexdir = ofs->workdir;
struct dentry *index = NULL;
- struct inode *dir = indexdir->d_inode;
struct path path = { .mnt = ovl_upper_mnt(ofs), .dentry = indexdir };
LIST_HEAD(list);
struct ovl_cache_entry *p;
@@ -1231,9 +1230,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs)
* Whiteout orphan index to block future open by
* handle after overlay nlink dropped to zero.
*/
- inode_lock_nested(dir, I_MUTEX_PARENT);
err = ovl_cleanup_and_whiteout(ofs, indexdir, index);
- inode_unlock(dir);
} else {
/* Cleanup orphan index entries */
err = ovl_cleanup_unlocked(ofs, indexdir, index);
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 7369193b11ec..5218a477551b 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;
@@ -1113,10 +1112,8 @@ static void ovl_cleanup_index(struct dentry *dentry)
index = NULL;
} else if (ovl_index_all(dentry->d_sb)) {
/* Whiteout orphan index to block future open by handle */
- inode_lock_nested(dir, I_MUTEX_PARENT);
err = ovl_cleanup_and_whiteout(OVL_FS(dentry->d_sb),
indexdir, index);
- inode_unlock(dir);
} else {
/* Cleanup orphan index entries */
err = ovl_cleanup_unlocked(ofs, indexdir, index);
@@ -1224,10 +1221,6 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work,
{
struct dentry *trap;
- /* Workdir should not be the same as upperdir */
- if (workdir == upperdir)
- goto err;
-
/* Workdir should not be subdir of upperdir and vice versa */
trap = lock_rename(workdir, upperdir);
if (IS_ERR(trap))
--
2.49.0
next prev parent reply other threads:[~2025-07-10 23:21 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-10 23:03 [PATCH 00/20 v2] ovl: narrow regions protected by i_rw_sem NeilBrown
2025-07-10 23:03 ` [PATCH 01/20] ovl: simplify an error path in ovl_copy_up_workdir() NeilBrown
2025-07-11 8:25 ` Amir Goldstein
2025-07-11 10:30 ` Amir Goldstein
2025-07-14 0:13 ` NeilBrown
2025-07-14 5:42 ` parent_lock/unlock (Was: [PATCH 01/20] ovl: simplify an error path in ovl_copy_up_workdir()) Amir Goldstein
2025-07-16 3:55 ` NeilBrown
2025-07-10 23:03 ` [PATCH 02/20] ovl: change ovl_create_index() to take write and dir locks NeilBrown
2025-07-11 10:41 ` Amir Goldstein
2025-07-14 0:14 ` NeilBrown
2025-07-10 23:03 ` [PATCH 03/20] ovl: Call ovl_create_temp() without lock held NeilBrown
2025-07-11 11:10 ` Amir Goldstein
2025-07-10 23:03 ` [PATCH 04/20] ovl: narrow the locked region in ovl_copy_up_workdir() NeilBrown
2025-07-11 12:03 ` Amir Goldstein
2025-07-14 0:29 ` NeilBrown
2025-07-10 23:03 ` [PATCH 05/20] ovl: narrow locking in ovl_create_upper() NeilBrown
2025-07-11 12:09 ` Amir Goldstein
2025-07-10 23:03 ` [PATCH 06/20] ovl: narrow locking in ovl_clear_empty() NeilBrown
2025-07-11 12:27 ` Amir Goldstein
2025-07-10 23:03 ` [PATCH 07/20] ovl: narrow locking in ovl_create_over_whiteout() NeilBrown
2025-07-11 12:42 ` Amir Goldstein
2025-07-10 23:03 ` [PATCH 08/20] ovl: narrow locking in ovl_rename() NeilBrown
2025-07-11 13:03 ` Amir Goldstein
2025-07-14 1:00 ` NeilBrown
2025-07-14 5:12 ` Amir Goldstein
2025-07-10 23:03 ` [PATCH 09/20] ovl: narrow locking in ovl_cleanup_whiteouts() NeilBrown
2025-07-10 23:03 ` [PATCH 10/20] ovl: narrow locking in ovl_cleanup_index() NeilBrown
2025-07-11 13:12 ` Amir Goldstein
2025-07-14 1:03 ` NeilBrown
2025-07-10 23:03 ` [PATCH 11/20] ovl: narrow locking in ovl_workdir_create() NeilBrown
2025-07-11 13:32 ` Amir Goldstein
2025-07-14 1:08 ` NeilBrown
2025-07-10 23:03 ` [PATCH 12/20] ovl: narrow locking in ovl_indexdir_cleanup() NeilBrown
2025-07-11 13:33 ` Amir Goldstein
2025-07-10 23:03 ` [PATCH 13/20] ovl: narrow locking in ovl_workdir_cleanup_recurse() NeilBrown
2025-07-11 13:35 ` Amir Goldstein
2025-07-10 23:03 ` [PATCH 14/20] ovl: change ovl_workdir_cleanup() to take dir lock as needed NeilBrown
2025-07-11 13:28 ` Amir Goldstein
2025-07-10 23:03 ` [PATCH 15/20] ovl: narrow locking on ovl_remove_and_whiteout() NeilBrown
2025-07-11 13:42 ` Amir Goldstein
2025-07-14 1:35 ` NeilBrown
2025-07-10 23:03 ` NeilBrown [this message]
2025-07-11 13:50 ` [PATCH 16/20] ovl: change ovl_cleanup_and_whiteout() to take rename lock as needed Amir Goldstein
2025-07-10 23:03 ` [PATCH 17/20] ovl: narrow locking in ovl_whiteout() NeilBrown
2025-07-11 15:19 ` Amir Goldstein
2025-07-14 1:44 ` NeilBrown
2025-07-10 23:03 ` [PATCH 18/20] ovl: narrow locking in ovl_check_rename_whiteout() NeilBrown
2025-07-11 13:54 ` Amir Goldstein
2025-07-10 23:03 ` [PATCH 19/20] ovl: change ovl_create_real() to receive dentry parent NeilBrown
2025-07-10 23:03 ` [PATCH 20/20] ovl: rename ovl_cleanup_unlocked() to ovl_cleanup() NeilBrown
2025-07-11 9:57 ` Amir Goldstein
2025-07-11 16:41 ` [PATCH 00/20 v2] ovl: narrow regions protected by i_rw_sem Amir Goldstein
2025-07-14 5:57 ` Amir Goldstein
2025-07-16 0:13 ` NeilBrown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250710232109.3014537-17-neil@brown.name \
--to=neil@brown.name \
--cc=amir73il@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-unionfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).