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 02/21] ovl: change ovl_create_index() to take dir locks
Date: Wed, 16 Jul 2025 10:44:13 +1000	[thread overview]
Message-ID: <20250716004725.1206467-3-neil@brown.name> (raw)
In-Reply-To: <20250716004725.1206467-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.

ovl_create_index() then takes the required lock only when needed.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/overlayfs/copy_up.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 79f41ef6ffa7..d485bd7edd8f 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,9 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
 	if (err)
 		return err;
 
+	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 +559,9 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
 	if (err)
 		goto out;
 
+	err = ovl_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 +569,10 @@ 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);
 	}
+	ovl_parent_unlock(indexdir);
 out:
 	if (err)
-		ovl_cleanup(ofs, dir, temp);
+		ovl_cleanup_unlocked(ofs, indexdir, temp);
 	dput(temp);
 free_name:
 	kfree(name.name);
@@ -798,6 +802,12 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
 	if (err)
 		goto cleanup_unlocked;
 
+	if (S_ISDIR(c->stat.mode) && c->indexed) {
+		err = ovl_create_index(c->dentry, c->origin_fh, temp);
+		if (err)
+			goto cleanup_unlocked;
+	}
+
 	/*
 	 * We cannot hold lock_rename() throughout this helper, because of
 	 * lock ordering with sb_writers, which shouldn't be held when calling
@@ -818,12 +828,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-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 ` NeilBrown [this message]
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 ` [PATCH v3 17/21] ovl: change ovl_cleanup_and_whiteout() to take rename lock as needed NeilBrown
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-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).