linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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 v3 17/21] ovl: change ovl_cleanup_and_whiteout() to take rename lock as needed
Date: Wed, 16 Jul 2025 10:44:28 +1000	[thread overview]
Message-ID: <20250716004725.1206467-18-neil@brown.name> (raw)
In-Reply-To: <20250716004725.1206467-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.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/overlayfs/dir.c     | 20 +++++++++-----------
 fs/overlayfs/readdir.c |  6 +-----
 fs/overlayfs/util.c    | 12 ++----------
 3 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 340f8679b6e7..6a70faeee6fa 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;
 }
 
@@ -783,16 +786,11 @@ 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)
 		ovl_dir_modified(dentry->d_parent, true);
 
 	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 e2d0c314df6c..ecbf39a49d57 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -1230,11 +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.
 			 */
-			err = ovl_parent_lock(indexdir, index);
-			if (!err) {
-				err = ovl_cleanup_and_whiteout(ofs, indexdir, index);
-				ovl_parent_unlock(indexdir);
-			}
+			err = ovl_cleanup_and_whiteout(ofs, indexdir, index);
 		} 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 b06136bbe170..3df0f3efe592 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1112,12 +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 */
-		err = ovl_parent_lock(indexdir, index);
-		if (!err) {
-			err = ovl_cleanup_and_whiteout(OVL_FS(dentry->d_sb),
-						       indexdir, index);
-			ovl_parent_unlock(indexdir);
-		}
+		err = ovl_cleanup_and_whiteout(OVL_FS(dentry->d_sb),
+					       indexdir, index);
 	} else {
 		/* Cleanup orphan index entries */
 		err = ovl_cleanup_unlocked(ofs, indexdir, index);
@@ -1225,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


  parent reply	other threads:[~2025-07-16  0:47 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-16  0:44 [PATCH v3 00/21] ovl: narrow regions protected by i_rw_sem NeilBrown
2025-07-16  0:44 ` [PATCH v3 01/21] ovl: simplify an error path in ovl_copy_up_workdir() NeilBrown
2025-07-16  7:12   ` Amir Goldstein
2025-08-04  8:57   ` Miklos Szeredi
2025-07-16  0:44 ` [PATCH v3 02/21] ovl: change ovl_create_index() to take dir locks NeilBrown
2025-07-16  0:44 ` [PATCH v3 03/21] ovl: Call ovl_create_temp() without lock held NeilBrown
2025-07-16  0:44 ` [PATCH v3 04/21] ovl: narrow the locked region in ovl_copy_up_workdir() NeilBrown
2025-07-16  7:13   ` Amir Goldstein
2025-07-16  0:44 ` [PATCH v3 05/21] ovl: narrow locking in ovl_create_upper() NeilBrown
2025-07-16  0:44 ` [PATCH v3 06/21] ovl: narrow locking in ovl_clear_empty() NeilBrown
2025-07-16  0:44 ` [PATCH v3 07/21] ovl: narrow locking in ovl_create_over_whiteout() NeilBrown
2025-07-16  0:44 ` [PATCH v3 08/21] ovl: simplify gotos in ovl_rename() NeilBrown
2025-07-16  7:13   ` Amir Goldstein
2025-07-16  0:44 ` [PATCH v3 09/21] ovl: narrow locking " NeilBrown
2025-07-16  7:13   ` Amir Goldstein
2025-07-16  0:44 ` [PATCH v3 10/21] ovl: narrow locking in ovl_cleanup_whiteouts() NeilBrown
2025-07-16  0:44 ` [PATCH v3 11/21] ovl: narrow locking in ovl_cleanup_index() NeilBrown
2025-07-16  7:14   ` Amir Goldstein
2025-07-16  0:44 ` [PATCH v3 12/21] ovl: narrow locking in ovl_workdir_create() NeilBrown
2025-07-16  7:15   ` Amir Goldstein
2025-07-16  0:44 ` [PATCH v3 13/21] ovl: narrow locking in ovl_indexdir_cleanup() NeilBrown
2025-07-16  7:15   ` Amir Goldstein
2025-07-16  0:44 ` [PATCH v3 14/21] ovl: narrow locking in ovl_workdir_cleanup_recurse() NeilBrown
2025-07-16  7:16   ` Amir Goldstein
2025-07-16  0:44 ` [PATCH v3 15/21] ovl: change ovl_workdir_cleanup() to take dir lock as needed NeilBrown
2025-07-16  7:16   ` Amir Goldstein
2025-07-16  0:44 ` [PATCH v3 16/21] ovl: narrow locking on ovl_remove_and_whiteout() NeilBrown
2025-07-16  7:16   ` Amir Goldstein
2025-07-16  0:44 ` NeilBrown [this message]
2025-07-16  0:44 ` [PATCH v3 18/21] ovl: narrow locking in ovl_whiteout() NeilBrown
2025-07-16  7:15   ` Amir Goldstein
2025-07-16  0:44 ` [PATCH v3 19/21] ovl: narrow locking in ovl_check_rename_whiteout() NeilBrown
2025-07-16  0:44 ` [PATCH v3 20/21] ovl: change ovl_create_real() to receive dentry parent NeilBrown
2025-07-16  0:44 ` [PATCH v3 21/21] ovl: rename ovl_cleanup_unlocked() to ovl_cleanup() NeilBrown
2025-07-16  7:14   ` Amir Goldstein
2025-07-16  7:10 ` [PATCH v3 00/21] ovl: narrow regions protected by i_rw_sem Amir Goldstein
2025-07-16  7:19   ` NeilBrown
2025-07-16  7:35     ` Amir Goldstein
2025-07-16  8:05       ` NeilBrown
2025-07-16  9:09         ` Amir Goldstein
2025-07-18  9:11 ` Christian Brauner

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=20250716004725.1206467-18-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).