All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Marco Elver <elver@google.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Will Deacon <will@kernel.org>, Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Boqun Feng <boqun.feng@gmail.com>,
	Waiman Long <longman@redhat.com>,
	Bart Van Assche <bvanassche@acm.org>,
	llvm@lists.linux.dev, Catalin Marinas <catalin.marinas@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] arm64: Optimize __READ_ONCE() with CONFIG_LTO=y
Date: Mon, 26 Jan 2026 22:55:46 +0000	[thread overview]
Message-ID: <20260126225546.30d96049@pumpkin> (raw)
In-Reply-To: <aXfGcIpeXQNRUI-A@elver.google.com>

On Mon, 26 Jan 2026 20:54:24 +0100
Marco Elver <elver@google.com> wrote:

> On Mon, Jan 26, 2026 at 08:56AM +0100, Arnd Bergmann wrote:
> > On Mon, Jan 26, 2026, at 01:25, Marco Elver wrote:  
> > > diff --git a/arch/arm64/include/asm/rwonce.h b/arch/arm64/include/asm/rwonce.h
> > > index fc0fb42b0b64..9963948f4b44 100644
> > > --- a/arch/arm64/include/asm/rwonce.h
> > > +++ b/arch/arm64/include/asm/rwonce.h
> > > @@ -32,8 +32,7 @@
> > >  #define __READ_ONCE(x)							\
> > >  ({									\
> > >  	typeof(&(x)) __x = &(x);					\
> > > -	int atomic = 1;							\
> > > -	union { __unqual_scalar_typeof(*__x) __val; char __c[1]; } __u;	\
> > > +	union { TYPEOF_UNQUAL(*__x) __val; char __c[1]; } __u;		\
> > >  	switch (sizeof(x)) {						\
> > >  	case 1:								\
> > >  		asm volatile(__LOAD_RCPC(b, %w0, %1)			\  
> > 
> > How does this work with CC_HAS_TYPEOF_UNQUAL=false?
> > 
> > As far as I can tell, TYPEOF_UNQUAL() falls back to __typeof__
> > on gcc-13, clang-18 and earlier, and not strip out qualifiers.  
> 
> I think we only need to worry about Clang for LTO builds. But yeah, our
> minimum supported Clang is 15, so between 15-18 it'd be broken.
> 
> > With fd69b2f7d5f4 ("compiler: Use __typeof_unqual__() for
> > __unqual_scalar_typeof()"), I would expect __unqual_scalar_typeof()
> > to do the right thing already.  
> 
> It'd still be broken for Clang 15-18, so it won't help much. We need
> this to work for more than "scalar", so even though it'll work for Clang
> 19+ given the redefinition to __typeof_unqual__, we should deprecate the
> _Generic-based __unqual_scalar_typeof() sooner than later.
> 
> I was able to make this work for older compilers:
> 
> diff --git a/arch/arm64/include/asm/rwonce.h b/arch/arm64/include/asm/rwonce.h
> index 85b1dd7b0274..d6c808cc01be 100644
> --- a/arch/arm64/include/asm/rwonce.h
> +++ b/arch/arm64/include/asm/rwonce.h
> @@ -19,6 +19,18 @@
>  		"ldapr"	#sfx "\t" #regs,				\
>  	ARM64_HAS_LDAPR)
>  
> +#ifdef USE_TYPEOF_UNQUAL
> +#define __read_once_typeof(x) TYPEOF_UNQUAL(x)
> +#else
> +/*
> + * Fallback for older compilers to infer an unqualified type, using the fact
> + * that __auto_type is supposed to drop qualifiers. Unlike typeof_unqual(), the
> + * type must be complete (defines an unevaluated local variable). This must
> + * already be guaranteed because sizeof(x) is used in the __READ_ONCE macro.
> + */
> +#define __read_once_typeof(x) typeof(({ __auto_type ____t = (x); ____t; }))

Did you try old versions of gcc?
gcc before 11.0 don't drop qualifiers.

Even const int y=0; __auto_type x = +y; generates a 'const' x (same for - and ~).
You need to do '0+y' to lose the const.
None of them help here because you don't want the integer promotion.

Can you use:
	union { u8 u8v, u16, u16v, u32 u32v, u64 u64v } u;
Then get the asm to write to the correct sized u.unn, then finish with:
	*(typeof(*x))&u;
or does that spill to stack for 'ptr to volatile'?

It doesn't solve the problem for other sized items, but I'm not sure
how many there really are.
If a handful changing them to READ_BIG_STRUCT_ONCE() shouldn't be a
real problem.

	David

> +#endif
> +
>  /*
>   * When building with LTO, there is an increased risk of the compiler
>   * converting an address dependency headed by a READ_ONCE() invocation
> @@ -32,8 +44,8 @@
>  #define __READ_ONCE(x)							\
>  ({									\
>  	auto __x = &(x);						\
> -	auto __ret = (TYPEOF_UNQUAL(*__x) *)__x, *__retp = &__ret;	\
> -	union { TYPEOF_UNQUAL(*__x) __val; char __c[1]; } __u;		\
> +	auto __ret = (__read_once_typeof(*__x) *)__x, *__retp = &__ret;	\
> +	union { __read_once_typeof(*__x) __val; char __c[1]; } __u;	\
>  	*__retp = &__u.__val;						\
>  	switch (sizeof(x)) {						\
>  	case 1:								\
> 
> 
> Thoughts?
> 



  parent reply	other threads:[~2026-01-26 22:56 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-26  0:25 [PATCH 0/3] arm64: Fixes for __READ_ONCE() with CONFIG_LTO=y Marco Elver
2026-01-26  0:25 ` [PATCH 1/3] arm64: Fix non-atomic " Marco Elver
2026-01-26  0:25 ` [PATCH 2/3] arm64: Optimize " Marco Elver
2026-01-26  7:56   ` Arnd Bergmann
2026-01-26 19:54     ` Marco Elver
2026-01-26 22:24       ` Arnd Bergmann
2026-01-27 12:01         ` Marco Elver
2026-01-27 14:30           ` David Laight
2026-01-27 15:04             ` Marco Elver
2026-01-27 18:54               ` David Laight
2026-01-26 22:55       ` David Laight [this message]
2026-01-26 11:16   ` David Laight
2026-01-26 23:15     ` Marco Elver
2026-01-27 10:13       ` David Laight
2026-01-26  0:25 ` [PATCH 3/3] arm64, compiler-context-analysis: Permit alias analysis through " Marco Elver

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=20260126225546.30d96049@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=arnd@arndb.de \
    --cc=boqun.feng@gmail.com \
    --cc=bvanassche@acm.org \
    --cc=catalin.marinas@arm.com \
    --cc=elver@google.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=longman@redhat.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=will@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.