Re: [PATCH v2] dcache: touch up predicts in __d_lookup_rcu()

[Jan Kara]

On Thu 27-11-25 14:15:26, Mateusz Guzik wrote:
> Rationale is that if the parent dentry is the same and the length is the
> same, then you have to be unlucky for the name to not match.
> 
> At the same time the dentry was literally just found on the hash, so you
> have to be even more unlucky to determine it is unhashed.
> 
> While here add commentary while d_unhashed() is necessary. It was
> already removed once and brought back in:
> 2e321806b681b192 ("Revert "vfs: remove unnecessary d_unhashed() check from __d_lookup_rcu"")
> 
> Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
> 
> - move and precit on d_unhashed as well
> - add commentary on it
> 
> this obsoletes https://lore.kernel.org/linux-fsdevel/20251127122412.4131818-1-mjguzik@xxxxxxxxx/T/#u
> 
>  fs/dcache.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 23d1752c29e6..dc2fff4811d1 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -2342,11 +2342,20 @@ struct dentry *__d_lookup_rcu(const struct dentry *parent,
>  		seq = raw_seqcount_begin(&dentry->d_seq);
>  		if (dentry->d_parent != parent)
>  			continue;
> -		if (d_unhashed(dentry))
> -			continue;
>  		if (dentry->d_name.hash_len != hashlen)
>  			continue;
> -		if (dentry_cmp(dentry, str, hashlen_len(hashlen)) != 0)
> +		if (unlikely(dentry_cmp(dentry, str, hashlen_len(hashlen)) != 0))
> +			continue;
> +		/*
> +		 * Check for the dentry being unhashed.
> +		 *
> +		 * As tempting as it is, we *can't* skip it because of a race window
> +		 * between us finding the dentry before it gets unhashed and loading
> +		 * the sequence counter after unhashing is finished.
> +		 *
> +		 * We can at least predict on it.
> +		 */
> +		if (unlikely(d_unhashed(dentry)))
>  			continue;
>  		*seqp = seq;
>  		return dentry;
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR