linux-alpha.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Dave Hansen <dave.hansen@intel.com>
Cc: Uros Bizjak <ubizjak@gmail.com>,
	linux-alpha@vger.kernel.org, loongarch@lists.linux.dev,
	linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	x86@kernel.org, linux-arch@vger.kernel.org,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	Richard Henderson <richard.henderson@linaro.org>,
	Ivan Kokshaysky <ink@jurassic.park.msu.ru>,
	Matt Turner <mattst88@gmail.com>,
	Huacai Chen <chenhuacai@kernel.org>,
	WANG Xuerui <kernel@xen0n.name>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Nicholas Piggin <npiggin@gmail.com>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.co>
Subject: Re: [PATCH v2 0/5] locking: Introduce local{,64}_try_cmpxchg
Date: Tue, 11 Apr 2023 12:35:50 +0100	[thread overview]
Message-ID: <ZDVGFhMwOtpxJtnQ@FVFF77S0Q05N> (raw)
In-Reply-To: <7360ffd2-a5aa-1373-8309-93e71ff36cbb@intel.com>

On Wed, Apr 05, 2023 at 09:37:04AM -0700, Dave Hansen wrote:
> On 4/5/23 07:17, Uros Bizjak wrote:
> > Add generic and target specific support for local{,64}_try_cmpxchg
> > and wire up support for all targets that use local_t infrastructure.
> 
> I feel like I'm missing some context.
> 
> What are the actual end user visible effects of this series?  Is there a
> measurable decrease in perf overhead?  Why go to all this trouble for
> perf?  Who else will use local_try_cmpxchg()?

Overall, the theory is that it can generate slightly better code (e.g. by
reusing the flags on x86). In practice, that might be in the noise, but as
demonstrated in prior postings the code generation is no worse than before.

From my perspective, the more important part is that this aligns local_t with
the other atomic*_t APIs, which all have ${atomictype}_try_cmpxchg(), and for
consistency/legibility/maintainability it's nice to be able to use the same
code patterns, e.g.

	${inttype} new, old = ${atomictype}_read(ptr);
	do {
		...
		new = do_something_with(old);
	} while (${atomictype}_try_cmpxvhg(ptr, &oldval, newval);

> I'm all for improving things, and perf is an important user.  But, if
> the goal here is improving performance, it would be nice to see at least
> a stab at quantifying the performance delta.

IIUC, Steve's original request for local_try_cmpxchg() was a combination of a
theoretical performance benefit and a more general preference to use
try_cmpxchg() for consistency / better structure of the source code:

  https://lore.kernel.org/lkml/20230301131831.6c8d4ff5@gandalf.local.home/

I agree it'd be nice to have performance figures, but I think those would only
need to demonstrate a lack of a regression rather than a performance
improvement, and I think it's fairly clear from eyeballing the generated
instructions that a regression isn't likely.

Thanks,
Mark.

  parent reply	other threads:[~2023-04-11 11:35 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-05 14:17 [PATCH v2 0/5] locking: Introduce local{,64}_try_cmpxchg Uros Bizjak
2023-04-05 14:17 ` [PATCH v2 1/5] locking/atomic: Add generic try_cmpxchg{,64}_local support Uros Bizjak
2023-04-11 11:10   ` Mark Rutland
2023-04-05 14:17 ` [PATCH v2 2/5] locking/generic: Wire up local{,64}_try_cmpxchg Uros Bizjak
2023-04-11 11:13   ` Mark Rutland
2023-04-05 14:17 ` [PATCH v2 3/5] locking/arch: Wire up local_try_cmpxchg Uros Bizjak
2023-04-12 11:32   ` Peter Zijlstra
2023-04-12 13:37     ` Uros Bizjak
2023-04-12 13:40       ` Peter Zijlstra
2023-05-17  7:41   ` Charlemagne Lasse
2023-04-05 14:17 ` [PATCH v2 4/5] locking/x86: Define arch_try_cmpxchg_local Uros Bizjak
2023-04-05 14:17 ` [PATCH v2 5/5] events: Illustrate the transition to local{,64}_try_cmpxchg Uros Bizjak
2023-04-05 16:37 ` [PATCH v2 0/5] locking: Introduce local{,64}_try_cmpxchg Dave Hansen
2023-04-05 18:53   ` Uros Bizjak
2023-04-06  8:25   ` David Laight
2023-04-06  8:38     ` Uros Bizjak
2023-04-06  9:01       ` David Laight
2023-04-11 11:35   ` Mark Rutland [this message]
2023-04-11 13:43     ` Dave Hansen
2023-04-11 21:34       ` David Laight

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=ZDVGFhMwOtpxJtnQ@FVFF77S0Q05N \
    --to=mark.rutland@arm.com \
    --cc=bp@alien8.de \
    --cc=chenhuacai@kernel.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=dave.hansen@intel.com \
    --cc=dave.hansen@linux.intel.co \
    --cc=ink@jurassic.park.msu.ru \
    --cc=kernel@xen0n.name \
    --cc=linux-alpha@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=loongarch@lists.linux.dev \
    --cc=mattst88@gmail.com \
    --cc=mingo@redhat.com \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=richard.henderson@linaro.org \
    --cc=tglx@linutronix.de \
    --cc=tsbogend@alpha.franken.de \
    --cc=ubizjak@gmail.com \
    --cc=x86@kernel.org \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).