linux-kernel - Re: [PATCH] drm/i915/active: Use try_cmpxchg64() in __active

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ceb80fde-dae5-478e-840c-9b949396d904@ursulin.net>
Date: Thu, 14 Aug 2025 07:44:40 +0100
From: Tvrtko Ursulin <tursulin@...ulin.net>
To: Uros Bizjak <ubizjak@...il.com>, intel-gfx@...ts.freedesktop.org,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Cc: Jani Nikula <jani.nikula@...ux.intel.com>,
 Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
 Rodrigo Vivi <rodrigo.vivi@...el.com>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>
Subject: Re: [PATCH] drm/i915/active: Use try_cmpxchg64() in __active_lookup()


Hi,

On 25/07/2025 08:26, Uros Bizjak wrote:
> Replace this pattern in __active_lookup():
> 
>      cmpxchg64(*ptr, old, new) == old
> 
> ... with the simpler and faster:
> 
>      try_cmpxchg64(*ptr, &old, new)
> 
> The x86 CMPXCHG instruction returns success in the ZF flag,
> so this change saves a compare after the CMPXCHG.
> 
> The patch also improves the explanation of what the code really
> does. cmpxchg64() will *succeed* for the winner of the race and
> try_cmpxchg64() nicely documents this fact.
> 
> No functional change intended.
> 
> Signed-off-by: Uros Bizjak <ubizjak@...il.com>
> Cc: Jani Nikula <jani.nikula@...ux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@...el.com>
> Cc: Tvrtko Ursulin <tursulin@...ulin.net>
> Cc: David Airlie <airlied@...il.com>
> Cc: Simona Vetter <simona@...ll.ch>
> ---
>   drivers/gpu/drm/i915/i915_active.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> index 0dbc4e289300..6b0c1162505a 100644
> --- a/drivers/gpu/drm/i915/i915_active.c
> +++ b/drivers/gpu/drm/i915/i915_active.c
> @@ -257,10 +257,9 @@ static struct active_node *__active_lookup(struct i915_active *ref, u64 idx)
>   		 * claimed the cache and we know that is does not match our
>   		 * idx. If, and only if, the timeline is currently zero is it
>   		 * worth competing to claim it atomically for ourselves (for
> -		 * only the winner of that race will cmpxchg return the old
> -		 * value of 0).
> +		 * only the winner of that race will cmpxchg succeed).
>   		 */
> -		if (!cached && !cmpxchg64(&it->timeline, 0, idx))
> +		if (!cached && try_cmpxchg64(&it->timeline, &cached, idx))
>   			return it;
>   	}
>   

Patch looks fine, thank you!

I've sent it for a CI pass (see 
https://patchwork.freedesktop.org/series/152185/) before merging.

Regards,

Tvrtko