Re: [PATCH 3/8] lockref: use bool for false/true returns

[Mateusz Guzik]

On Wed, Jan 15, 2025 at 10:46:39AM +0100, Christoph Hellwig wrote:
> Replace int used as bool with the actual bool type for return values that
> can only be true or false.
> 
[snip]

> -int lockref_get_not_zero(struct lockref *lockref)
> +bool lockref_get_not_zero(struct lockref *lockref)
>  {
> -	int retval;
> +	bool retval = false;
>  
>  	CMPXCHG_LOOP(
>  		new.count++;
>  		if (old.count <= 0)
> -			return 0;
> +			return false;
>  	,
> -		return 1;
> +		return true;
>  	);
>  
>  	spin_lock(&lockref->lock);
> -	retval = 0;
>  	if (lockref->count > 0) {
>  		lockref->count++;
> -		retval = 1;
> +		retval = true;
>  	}
>  	spin_unlock(&lockref->lock);
>  	return retval;

While this looks perfectly sane, it worsens codegen around the atomic
on x86-64 at least with gcc 13.3.0. It bisected to this commit and
confirmed top of next-20250318 with this reverted undoes it.

The expected state looks like this:
       f0 48 0f b1 13          lock cmpxchg %rdx,(%rbx)
       75 0e                   jne    ffffffff81b33626 <lockref_get_not_dead+0x46>

However, with the above patch I see:
       f0 48 0f b1 13          lock cmpxchg %rdx,(%rbx)
       40 0f 94 c5             sete   %bpl
       40 84 ed                test   %bpl,%bpl
       74 09                   je     ffffffff81b33636 <lockref_get_not_dead+0x46>

This is not the end of the world, but also really does not need to be
there.

Given that the patch is merely a cosmetic change, I would suggest I gets
dropped.