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 <ubiz...@gmail.com>
Cc: Jani Nikula <jani.nik...@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
Cc: Tvrtko Ursulin <tursu...@ursulin.net>
Cc: David Airlie <airl...@gmail.com>
Cc: Simona Vetter <sim...@ffwll.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
