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
next prev parent 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