Re: [PATCH 1/2] fs: dcache: Handle case-exact lookup in d_alloc_parallel

On 03/10/21 7:08 pm, Al Viro wrote:

On Wed, Sep 29, 2021 at 04:23:38PM +0530, Shreeya Patel wrote:

quoted

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.

NAK.  What you are doing would lead to parallel calls of ->lookup() in the
same directory for names that would compare as equal.  Which violates
all kinds of assumptions in the analysis of dentry tree locking.

d_add_ci() is used to force the "exact" spelling of the name on lookup -
that's the whole point of that thing.  What are you trying to achieve,
and what's the point of mixing that with non-trivial ->d_compare()?

Sending again as plain text...

Hi Al Viro,

This patch was added to resolve some of the issues faced in patch 02/02 
of the series.

Originally, the 'native', per-directory case-insensitive implementation
merged in ext4/f2fs stores the case of the first lookup on the dcache,
regardless of the disk exact file name case. This gets reflected in symlink
returned by /proc/self/cwd.

To solve this we are calling d_add_ci from the fs lookup function to 
store the
disk exact name in the dcache even if an inexact-match string is used on 
the FIRST lookup.
But this caused a soft hang since there was a deadlock in d_wait_lookup 
called from d_alloc_parallel.

The reason for the hang is that d_same_name uses d_compare which does a
case-insensitive match and is able to find the dentry name in the 
secondary hash table
leading it to d_wait_lookup which would wait for the lookup to finish on 
that dentry
causing a deadlock.

To avoid the hang, we are doing a case-sensitive match using dentry_cmp 
here.


Thanks

If it's "force to exact spelling on lookup, avoid calling ->lookup() on
aliases", d_add_ci() is simply not a good match.