Re: [peterz-queue:locking/core 10/10] kernel/futex/core.c:982:1: warning: spinlock 'atomic ? __u.__val : q->lock_ptr' is still held at the end of function

[Marco Elver <elver@google.com>]

+Cc Will

On Thu, Jan 22, 2026 at 10:16AM +0100, Peter Zijlstra wrote:
> On Thu, Jan 22, 2026 at 10:30:28AM +0800, kernel test robot wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git locking/core
> > head:   33d9e7fdf9c17417347d1f06ddebaf25f21ab62a
> > commit: 33d9e7fdf9c17417347d1f06ddebaf25f21ab62a [10/10] futex: Convert to compiler context analysis
> > config: arm64-randconfig-001-20260122 (https://download.01.org/0day-ci/archive/20260122/202601221040.TeM0ihff-lkp@intel.com/config)
> > compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 9b8addffa70cee5b2acc5454712d9cf78ce45710)
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260122/202601221040.TeM0ihff-lkp@intel.com/reproduce)
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202601221040.TeM0ihff-lkp@intel.com/
> > 
> > All warnings (new ones prefixed by >>):
> > 
> > >> kernel/futex/core.c:982:1: warning: spinlock 'atomic ? __u.__val : q->lock_ptr' is still held at the end of function [-Wthread-safety-analysis]
> >      982 | }
> >          | ^
> >    kernel/futex/core.c:976:2: note: spinlock acquired here
> >      976 |         spin_lock(lock_ptr);
> >          |         ^
> > >> kernel/futex/core.c:982:1: warning: expecting spinlock 'q->lock_ptr' to be held at the end of function [-Wthread-safety-analysis]
> >      982 | }
> >          | ^
> >    kernel/futex/core.c:966:6: note: spinlock acquired here
> >      966 | void futex_q_lockptr_lock(struct futex_q *q)
> >          |      ^
> >    2 warnings generated.
> 
> Urgh, this is the arm64 READ_ONCE() confusing the thing. It can't see
> through that mess and realize: lockptr == q->lock_ptr.
> 
> Marco, any suggestion how to best fix that?

I have a tentative solution:

diff --git a/arch/arm64/include/asm/rwonce.h b/arch/arm64/include/asm/rwonce.h
index 78beceec10cd..75265012ff2d 100644
--- a/arch/arm64/include/asm/rwonce.h
+++ b/arch/arm64/include/asm/rwonce.h
@@ -31,9 +31,8 @@
  */
 #define __READ_ONCE(x)							\
 ({									\
-	typeof(&(x)) __x = &(x);					\
-	int atomic = 1;							\
-	union { __unqual_scalar_typeof(*__x) __val; char __c[1]; } __u;	\
+	typeof(&(x)) __x = &(x), *__xp = &__x;				\
+	union { TYPEOF_UNQUAL(*__x) __val; char __c[1]; } __u;		\
 	switch (sizeof(x)) {						\
 	case 1:								\
 		asm volatile(__LOAD_RCPC(b, %w0, %1)			\
@@ -56,9 +55,10 @@
 			: "Q" (*__x) : "memory");			\
 		break;							\
 	default:							\
-		atomic = 0;						\
+		__u.__val = (*(volatile typeof(__x))__x);		\
 	}								\
-	atomic ? (typeof(*__x))__u.__val : (*(volatile typeof(__x))__x);\
+	*__xp = &__u.__val;						\
+	*__x;								\
 })
 
 #endif	/* !BUILD_VDSO */

This works because the compiler doesn't analyze alias reassignment
through pointer-to-alias within the same function. An alternative is to
do the alias reassignment via an __always_inline function that takes the
alias as a const pointer, and then cast the const away and do the
assignment. But I think the above is cleaner. Only compile-tested.

It also gets rid of the "atomic" flag, and switches to TYPEOF_UNQUAL
which fixes a latent bug where __unqual_scalar_typeof was applied to a
const non-integer variable that is less than 8 bytes, resulting in __val
being "const" itself (likely benign).

Before:

  File: vmlinux
  Size: 59628672  	Blocks: 116472     IO Block: 4096   regular file

After:

  File: vmlinux
  Size: 59563056  	Blocks: 116336     IO Block: 4096   regular file

So this claims we're saving space. But the couple functions I inspected
that use READ_ONCE were identical. I need to double-check the asm, see
where the differences are and if we're actually producing better code,
and write a commit message that makes sense.