public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
From: Eugen Hristev <eugen.hristev@collabora.com>
To: viro@zeniv.linux.org.uk, brauner@kernel.org, tytso@mit.edu,
	linux-ext4@vger.kernel.org
Cc: jack@suse.cz, adilger.kernel@dilger.ca,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	krisman@suse.de, kernel@collabora.com,
	shreeya.patel@collabora.com,
	Eugen Hristev <eugen.hristev@collabora.com>
Subject: [PATCH 1/2] fs/dcache: introduce d_alloc_parallel_check_existing
Date: Fri,  5 Jul 2024 09:26:20 +0300	[thread overview]
Message-ID: <20240705062621.630604-2-eugen.hristev@collabora.com> (raw)
In-Reply-To: <20240705062621.630604-1-eugen.hristev@collabora.com>

d_alloc_parallel currently looks for entries that match the name being
added and return them if found.
However if d_alloc_parallel is being called during the process of adding
a new entry (that becomes in_lookup), the same entry is found by
d_alloc_parallel in the in_lookup_hash and d_alloc_parallel will wait
forever for it to stop being in lookup.
To avoid this, it makes sense to check for an entry being currently
added (e.g. by d_add_ci from a lookup func, like xfs is doing) and if this
exact match is found, return the entry.
This way, to add a new entry , as xfs is doing, is the following flow:
_lookup_slow -> d_alloc_parallel -> entry is being created -> xfs_lookup ->
d_add_ci -> d_alloc_parallel_check_existing(entry created before) ->
d_splice_alias.
The initial entry stops being in_lookup after d_splice_alias finishes, and
it's returned to d_add_ci by d_alloc_parallel_check_existing.
Without d_alloc_parallel_check_existing, d_alloc_parallel would be called
instead and wait forever for the entry to stop being in lookup, as the
iteration through the in_lookup_hash matches the entry.
Currently XFS does not hang because it creates another entry in the second
call of d_alloc_parallel if the name differs by case as the hashing and
comparison functions used by XFS are not case insensitive.

Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
---
 fs/dcache.c            | 29 +++++++++++++++++++++++------
 include/linux/dcache.h |  4 ++++
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index a0a944fd3a1c..459a3d8b8bdb 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2049,8 +2049,9 @@ struct dentry *d_add_ci(struct dentry *dentry, struct inode *inode,
 		return found;
 	}
 	if (d_in_lookup(dentry)) {
-		found = d_alloc_parallel(dentry->d_parent, name,
-					dentry->d_wait);
+		found = d_alloc_parallel_check_existing(dentry,
+							dentry->d_parent, name,
+							dentry->d_wait);
 		if (IS_ERR(found) || !d_in_lookup(found)) {
 			iput(inode);
 			return found;
@@ -2452,9 +2453,10 @@ static void d_wait_lookup(struct dentry *dentry)
 	}
 }
 
-struct dentry *d_alloc_parallel(struct dentry *parent,
-				const struct qstr *name,
-				wait_queue_head_t *wq)
+struct dentry *d_alloc_parallel_check_existing(struct dentry *d_check,
+					       struct dentry *parent,
+					       const struct qstr *name,
+					       wait_queue_head_t *wq)
 {
 	unsigned int hash = name->hash;
 	struct hlist_bl_head *b = in_lookup_hash(parent, hash);
@@ -2523,6 +2525,14 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
 		}
 
 		rcu_read_unlock();
+
+		/* if the entry we found is the same as the original we
+		 * are checking against, then return it
+		 */
+		if (d_check == dentry) {
+			dput(new);
+			return dentry;
+		}
 		/*
 		 * somebody is likely to be still doing lookup for it;
 		 * wait for them to finish
@@ -2560,8 +2570,15 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
 	dput(dentry);
 	goto retry;
 }
-EXPORT_SYMBOL(d_alloc_parallel);
+EXPORT_SYMBOL(d_alloc_parallel_check_existing);
 
+struct dentry *d_alloc_parallel(struct dentry *parent,
+				const struct qstr *name,
+				wait_queue_head_t *wq)
+{
+	return d_alloc_parallel_check_existing(NULL, parent, name, wq);
+}
+EXPORT_SYMBOL(d_alloc_parallel);
 /*
  * - Unhash the dentry
  * - Retrieve and clear the waitqueue head in dentry
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index bf53e3894aae..6eb21a518cb0 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -232,6 +232,10 @@ extern struct dentry * d_alloc(struct dentry *, const struct qstr *);
 extern struct dentry * d_alloc_anon(struct super_block *);
 extern struct dentry * d_alloc_parallel(struct dentry *, const struct qstr *,
 					wait_queue_head_t *);
+extern struct dentry * d_alloc_parallel_check_existing(struct dentry *,
+						       struct dentry *,
+						       const struct qstr *,
+						       wait_queue_head_t *);
 extern struct dentry * d_splice_alias(struct inode *, struct dentry *);
 extern struct dentry * d_add_ci(struct dentry *, struct inode *, struct qstr *);
 extern bool d_same_name(const struct dentry *dentry, const struct dentry *parent,
-- 
2.34.1


  reply	other threads:[~2024-07-05  6:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-05  6:26 [PATCH 0/2] fs/dcache: fix cache inconsistency on case-insensitive lookups Eugen Hristev
2024-07-05  6:26 ` Eugen Hristev [this message]
2024-08-20 20:16   ` [PATCH 1/2] fs/dcache: introduce d_alloc_parallel_check_existing Gabriel Krisman Bertazi
2024-08-21  9:10     ` Eugen Hristev
2024-08-21 23:22       ` Gabriel Krisman Bertazi
2024-08-22  1:13         ` Al Viro
2024-08-22 17:25           ` Gabriel Krisman Bertazi
2024-07-05  6:26 ` [PATCH 2/2] ext4: in lookup call d_add_ci if there is a case mismatch Eugen Hristev
2024-08-15  6:02 ` [PATCH 0/2] fs/dcache: fix cache inconsistency on case-insensitive lookups Eugen Hristev

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=20240705062621.630604-2-eugen.hristev@collabora.com \
    --to=eugen.hristev@collabora.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=kernel@collabora.com \
    --cc=krisman@suse.de \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shreeya.patel@collabora.com \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    /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