[PATCH] atomics: Use atomic_try_cmpxchg_release in rcuref_put_slowpath()

[PATCH] atomics: Use atomic_try_cmpxchg_release in rcuref_put_slowpath()

[Uros Bizjak]

Use atomic_try_cmpxchg instead of atomic_cmpxchg (*ptr, old, new) == old
in rcuref_put_slowpath(). 86 CMPXCHG instruction returns success in
ZF flag, so this change saves a compare after cmpxchg.  Additionaly,
the compiler reorders some code blocks to follow likely/unlikely
annotations in the atomic_try_cmpxchg macro, improving the code from

  9a:	f0 0f b1 0b          	lock cmpxchg %ecx,(%rbx)
  9e:	83 f8 ff             	cmp    $0xffffffff,%eax
  a1:	74 04                	je     a7 <rcuref_put_slowpath+0x27>
  a3:	31 c0                	xor    %eax,%eax

to

  9a:	f0 0f b1 0b          	lock cmpxchg %ecx,(%rbx)
  9e:	75 4c                	jne    ec <rcuref_put_slowpath+0x6c>
  a0:	b0 01                	mov    $0x1,%al

No functional change intended.

Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
---
 lib/rcuref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rcuref.c b/lib/rcuref.c
index 5ec00a4a64d1..97f300eca927 100644
--- a/lib/rcuref.c
+++ b/lib/rcuref.c
@@ -248,7 +248,7 @@ bool rcuref_put_slowpath(rcuref_t *ref)
 		 * require a retry. If this fails the caller is not
 		 * allowed to deconstruct the object.
 		 */
-		if (atomic_cmpxchg_release(&ref->refcnt, RCUREF_NOREF, RCUREF_DEAD) != RCUREF_NOREF)
+		if (!atomic_try_cmpxchg_release(&ref->refcnt, &cnt, RCUREF_DEAD))
 			return false;
 
 		/*
-- 
2.40.1

RE: [PATCH] atomics: Use atomic_try_cmpxchg_release in rcuref_put_slowpath()

[David Laight]

From: Uros Bizjak
> Sent: 09 May 2023 16:03
> 
> Use atomic_try_cmpxchg instead of atomic_cmpxchg (*ptr, old, new) == old
> in rcuref_put_slowpath(). 86 CMPXCHG instruction returns success in
> ZF flag, so this change saves a compare after cmpxchg.  Additionaly,
> the compiler reorders some code blocks to follow likely/unlikely
> annotations in the atomic_try_cmpxchg macro, improving the code from
> 
>   9a:	f0 0f b1 0b          	lock cmpxchg %ecx,(%rbx)
>   9e:	83 f8 ff             	cmp    $0xffffffff,%eax
>   a1:	74 04                	je     a7 <rcuref_put_slowpath+0x27>
>   a3:	31 c0                	xor    %eax,%eax
> 
> to
> 
>   9a:	f0 0f b1 0b          	lock cmpxchg %ecx,(%rbx)
>   9e:	75 4c                	jne    ec <rcuref_put_slowpath+0x6c>
>   a0:	b0 01                	mov    $0x1,%al
> 
> No functional change intended.

While I'm not against the change I bet you can't detect
any actual difference. IIRC:
- The 'cmp+je' get merged into a single u-op.
- The 'lock cmpxchg' will take long enough that the instruction
  decoder won't be a bottleneck.
- Whether the je/jne is predicted taken is pretty much random.
  So you'll speculatively execute somewhere (could be anywhere)
  while the locked cycle completes.
So the only change is three less bytes of object code.
That will change the cache line alignment of later code.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

[tip: locking/core] locking/atomics: Use atomic_try_cmpxchg_release() to micro-optimize rcuref_put_slowpath()

[tip-bot2 for Uros Bizjak]

The following commit has been merged into the locking/core branch of tip:

Commit-ID:     4fbf8b136ded943f8661cf48270482ad1f5ce7bd
Gitweb:        https://git.kernel.org/tip/4fbf8b136ded943f8661cf48270482ad1f5ce7bd
Author:        Uros Bizjak <ubizjak@gmail.com>
AuthorDate:    Tue, 09 May 2023 17:02:55 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 10 Oct 2023 10:14:27 +02:00

locking/atomics: Use atomic_try_cmpxchg_release() to micro-optimize rcuref_put_slowpath()

Use atomic_try_cmpxchg() instead of atomic_cmpxchg(*ptr, old, new) == old
in rcuref_put_slowpath(). On x86 the CMPXCHG instruction returns success in the
ZF flag, so this change saves a compare after CMPXCHG.  Additionaly,
the compiler reorders some code blocks to follow likely/unlikely
annotations in the atomic_try_cmpxchg() macro, improving the code from:

  9a:	f0 0f b1 0b          	lock cmpxchg %ecx,(%rbx)
  9e:	83 f8 ff             	cmp    $0xffffffff,%eax
  a1:	74 04                	je     a7 <rcuref_put_slowpath+0x27>
  a3:	31 c0                	xor    %eax,%eax

to:

  9a:	f0 0f b1 0b          	lock cmpxchg %ecx,(%rbx)
  9e:	75 4c                	jne    ec <rcuref_put_slowpath+0x6c>
  a0:	b0 01                	mov    $0x1,%al

No functional change intended.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Paul E. McKenney <paulmck@kernel.org>
Link: https://lore.kernel.org/r/20230509150255.3691-1-ubizjak@gmail.com
---
 lib/rcuref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rcuref.c b/lib/rcuref.c
index 5ec00a4..97f300e 100644
--- a/lib/rcuref.c
+++ b/lib/rcuref.c
@@ -248,7 +248,7 @@ bool rcuref_put_slowpath(rcuref_t *ref)
 		 * require a retry. If this fails the caller is not
 		 * allowed to deconstruct the object.
 		 */
-		if (atomic_cmpxchg_release(&ref->refcnt, RCUREF_NOREF, RCUREF_DEAD) != RCUREF_NOREF)
+		if (!atomic_try_cmpxchg_release(&ref->refcnt, &cnt, RCUREF_DEAD))
 			return false;
 
 		/*