Shreeya Patel <shreeya.patel@xxxxxxxxxxxxx> writes:
> There is a soft hang caused by a deadlock in d_alloc_parallel which
> waits up on lookups to finish for the dentries in the parent directory's
> hash_table.
> In case when d_add_ci is called from the fs layer's lookup functions,
> the dentry being looked up is already in the hash table (created before
> the fs lookup function gets called). We should not be processing the
> same dentry that is being looked up, hence, in case of case-insensitive
> filesystems we are making it a case-exact match to prevent this from
> happening.
>
> Signed-off-by: Shreeya Patel <shreeya.patel@xxxxxxxxxxxxx>
> ---
> fs/dcache.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index cf871a81f4fd..2a28ab64a165 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -2565,6 +2565,15 @@ static void d_wait_lookup(struct dentry *dentry)
> }
> }
>
> +static inline bool d_same_exact_name(const struct dentry *dentry,
> + const struct dentry *parent,
> + const struct qstr *name)
> +{
> + if (dentry->d_name.len != name->len)
> + return false;
> + return dentry_cmp(dentry, name->name, name->len) == 0;
> +}
I don't like the idea of having a flavor of a dentry comparison function
that doesn't invoke d_compare. In particular because d_compare might be
used for all sorts of things, and this fix is really specific to the
case-insensitive case.
Would it be possible to fold this change into generic_ci_d_compare? If
we could flag the dentry as part of a parallel lookup under the relevant
condition, generic_ci_d_compare could simply return immediately in
such case.
> +
> struct dentry *d_alloc_parallel(struct dentry *parent,
> const struct qstr *name,
> wait_queue_head_t *wq)
> @@ -2575,6 +2584,7 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
> struct dentry *new = d_alloc(parent, name);
> struct dentry *dentry;
> unsigned seq, r_seq, d_seq;
> + int ci_dir = IS_CASEFOLDED(parent->d_inode);
>
> if (unlikely(!new))
> return ERR_PTR(-ENOMEM);
> @@ -2626,8 +2636,14 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
> continue;
> if (dentry->d_parent != parent)
> continue;
> - if (!d_same_name(dentry, parent, name))
> - continue;
> + if (ci_dir) {
> + if (!d_same_exact_name(dentry, parent, name))
> + continue;
> + } else {
As is, this is problematic because d_alloc_parallel is also part of the
lookup path (see lookup_open, lookup_slow). In those cases, you want to
do the CI comparison, to prevent racing two tasks creating a dentry
differing only by case.
> + if (!d_same_name(dentry, parent, name))
> + continue;
> + }
> +
> hlist_bl_unlock(b);
> /* now we can try to grab a reference */
> if (!lockref_get_not_dead(&dentry->d_lockref)) {
--
Gabriel Krisman Bertazi
