Re: [PATCH] fs: dodge an atomic in putname if ref == 1

[Jan Kara]

On Tue 11-03-25 19:18:04, Mateusz Guzik wrote:
> While the structure is refcounted, the only consumer incrementing it is
> audit and even then the atomic operation is only needed when it
> interacts with io_uring.
> 
> If putname spots a count of 1, there is no legitimate way for anyone to
> bump it.
> 
> If audit is disabled, the count is guaranteed to be 1, which
> consistently elides the atomic for all path lookups. If audit is
> enabled, it still manages to elide the last decrement.
> 
> Note the patch does not do anything to prevent audit from suffering
> atomics. See [1] and [2] for a different approach.
> 
> Benchmarked on Sapphire Rapids issuing access() (ops/s):
> before: 5106246
> after:  5269678 (+3%)
> 
> Link 1:	https://lore.kernel.org/linux-fsdevel/20250307161155.760949-1-mjguzik@xxxxxxxxx/
> Link 2: https://lore.kernel.org/linux-fsdevel/20250307164216.GI2023217@ZenIV/
> Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx>

Yeah, I like this much more than the original. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> This is an alternative to the patch I linked above.
> 
> I think the improved commit message should also cover the feedback
> Christian previously shared concerning it.
> 
> This is a trivial win until the atomic issue gets resolved, I don't see
> any reason to NOT include it. At the same time I don't have that much
> interest arguing about it either.
> 
> That is to say, here is a different take, if you don't like it, I'm
> dropping the subject.
> 
> cheers
> 
>  fs/namei.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 06765d320e7e..add90981cfcd 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -275,14 +275,19 @@ EXPORT_SYMBOL(getname_kernel);
>  
>  void putname(struct filename *name)
>  {
> +	int refcnt;
> +
>  	if (IS_ERR_OR_NULL(name))
>  		return;
>  
> -	if (WARN_ON_ONCE(!atomic_read(&name->refcnt)))
> -		return;
> +	refcnt = atomic_read(&name->refcnt);
> +	if (refcnt != 1) {
> +		if (WARN_ON_ONCE(!refcnt))
> +			return;
>  
> -	if (!atomic_dec_and_test(&name->refcnt))
> -		return;
> +		if (!atomic_dec_and_test(&name->refcnt))
> +			return;
> +	}
>  
>  	if (name->name != name->iname) {
>  		__putname(name->name);
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR