linux-unionfs.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 02/20] ovl: change ovl_create_index() to take write and dir locks
Date: Fri, 11 Jul 2025 09:03:32 +1000	[thread overview]
Message-ID: <20250710232109.3014537-3-neil@brown.name> (raw)
In-Reply-To: <20250710232109.3014537-1-neil@brown.name>

ovl_copy_up_workdir() currently take a rename lock on two directories,
then use the lock to both create a file in one directory, perform a
rename, and possibly unlink the file for cleanup.  This is incompatible
with proposed changes which will lock just the dentry of objects being
acted on.

This patch moves the call to ovl_create_index() earlier in
ovl_copy_up_workdir() to before the lock is taken, and also before write
access to the filesystem is gained (this last is not strictly necessary
but seems cleaner).

ovl_create_index() then take the requires locks and drops them before
returning.

Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/overlayfs/copy_up.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 5d21b8d94a0a..25be0b80a40b 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -517,8 +517,6 @@ static int ovl_set_upper_fh(struct ovl_fs *ofs, struct dentry *upper,
 
 /*
  * Create and install index entry.
- *
- * Caller must hold i_mutex on indexdir.
  */
 static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
 			    struct dentry *upper)
@@ -550,7 +548,10 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
 	if (err)
 		return err;
 
+	ovl_start_write(dentry);
+	inode_lock(dir);
 	temp = ovl_create_temp(ofs, indexdir, OVL_CATTR(S_IFDIR | 0));
+	inode_unlock(dir);
 	err = PTR_ERR(temp);
 	if (IS_ERR(temp))
 		goto free_name;
@@ -559,6 +560,9 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
 	if (err)
 		goto out;
 
+	err = parent_lock(indexdir, temp);
+	if (err)
+		goto out;
 	index = ovl_lookup_upper(ofs, name.name, indexdir, name.len);
 	if (IS_ERR(index)) {
 		err = PTR_ERR(index);
@@ -566,9 +570,11 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
 		err = ovl_do_rename(ofs, indexdir, temp, indexdir, index, 0);
 		dput(index);
 	}
+	parent_unlock(indexdir);
 out:
 	if (err)
-		ovl_cleanup(ofs, dir, temp);
+		ovl_cleanup_unlocked(ofs, indexdir, temp);
+	ovl_end_write(dentry);
 	dput(temp);
 free_name:
 	kfree(name.name);
@@ -797,6 +803,12 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
 	if (err)
 		goto cleanup_need_write;
 
+	if (S_ISDIR(c->stat.mode) && c->indexed) {
+		err = ovl_create_index(c->dentry, c->origin_fh, temp);
+		if (err)
+			goto cleanup_need_write;
+	}
+
 	/*
 	 * We cannot hold lock_rename() throughout this helper, because of
 	 * lock ordering with sb_writers, which shouldn't be held when calling
@@ -818,12 +830,6 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
 	if (err)
 		goto cleanup;
 
-	if (S_ISDIR(c->stat.mode) && c->indexed) {
-		err = ovl_create_index(c->dentry, c->origin_fh, temp);
-		if (err)
-			goto cleanup;
-	}
-
 	upper = ovl_lookup_upper(ofs, c->destname.name, c->destdir,
 				 c->destname.len);
 	err = PTR_ERR(upper);
-- 
2.49.0


  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 ` NeilBrown [this message]
2025-07-11 10:41   ` [PATCH 02/20] ovl: change ovl_create_index() to take write and dir locks 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 ` [PATCH 16/20] ovl: change ovl_cleanup_and_whiteout() to take rename lock as needed NeilBrown
2025-07-11 13:50   ` 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-3-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).