public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Uros Bizjak' <ubizjak@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Subject: RE: [PATCH] atomics: Use atomic_try_cmpxchg_release in rcuref_put_slowpath()
Date: Wed, 10 May 2023 11:35:59 +0000	[thread overview]
Message-ID: <655cab1a213440f682eddc9cc1ad2d44@AcuMS.aculab.com> (raw)
In-Reply-To: <20230509150255.3691-1-ubizjak@gmail.com>

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)


  reply	other threads:[~2023-05-10 11:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-09 15:02 [PATCH] atomics: Use atomic_try_cmpxchg_release in rcuref_put_slowpath() Uros Bizjak
2023-05-10 11:35 ` David Laight [this message]
2023-10-10  8:27 ` [tip: locking/core] locking/atomics: Use atomic_try_cmpxchg_release() to micro-optimize rcuref_put_slowpath() tip-bot2 for Uros Bizjak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=655cab1a213440f682eddc9cc1ad2d44@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=ubizjak@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox