Re: [PATCH 1/2] fs/dcache: introduce d_alloc_parallel_check_existing

[Gabriel Krisman Bertazi <krisman@suse.de>]

Al Viro <viro@zeniv.linux.org.uk> writes:

> On Wed, Aug 21, 2024 at 07:22:39PM -0400, Gabriel Krisman Bertazi wrote:
>
>> Would it be acceptable to just change the dentry->d_name here in a new
>> flavor of d_add_ci used only by these filesystems? We are inside the
>> creation path, so the dentry has never been hashed.  Concurrent lookups
>> will be stuck in d_wait_lookup() until we are done and will never become
>> invalid after the change because the lookup was already done
>> case-insensitively, so they all match the same dentry, per-definition,
>> and we know there is no other matching dentries in the directory.  We'd
>> only need to be careful not to expose partial names to concurrent
>> parallel lookups.
>
> *Ow*
>
> ->d_name stability rules are already convoluted as hell; that would make
> them even more painful.
>
> What locking are you going to use there?

Since we are in the ->d_lookup() during the rename, and we use the
dcache-insensitively for the filesystems that will do the rename, we
know there is nothing in the dcache and the dentry is still in the
parallel lookup table.  So we are not racing with a creation of the same
name in the same directory.  A parallel lookup will either find that
dentry (old or new name, doesn't matter) or not find anything, in case
it sees a partial ->d_name.  Therefore, the only possible problem is a
false negative/positive in parent->d_in_lookup_hash.

Can we extend the rename_lock seqlock protection that already exists in
d_alloc_parallel to include the d_in_lookup_hash walk?  d_add_ci then
acquires the rename_lock before writing ->d_name and d_alloc_parallel
will see it changed after iterating over d_in_lookup_hash, in case it
didn't find anything, and retry the entire sequence.

Case-inexact lookups are not supposed to be frequent. Most lookups
should be done in a case-exact way, so the extra acquisition of
rename_lock shouldn't create more contention on the rename_lock for the
regular path or for non-case-insensitive filesystems.  The overhead in
d_alloc_parallel is another read_seqretry() that is done only in the
case where the dentry is not found anywhere and should be created.

-- 
Gabriel Krisman Bertazi