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

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Jan 22, 2026 at 07:31:08PM +0100, Marco Elver wrote:
> +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.

Oh clever! I suppose it does help if you know how the compiler works :-)

I was thinking that perhaps it makes sense to invest in a 'directive'
where we can explicitly hand alias information to the compiler.

Consider:

  https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/tree/kernel/locking/rtmutex.c?h=locking/core&id=a31a1351e9ec9df7a72ea4877a8e53a9c02e8641#n1259

		rtm = container_of(lock, struct rt_mutex, rtmutex);
		__assume_ctx_lock(&rtm->rtmutex.wait_lock);
		res = __ww_mutex_add_waiter(waiter, rtm, ww_ctx, wake_q);

Here there is a similar issue, where container_of() wrecks things. It
cannot tell that rtm->rtmutex == lock and hence its idea of
lock->wait_lock gets 'confused'.

I hack around it with that __assume_ctx_lock() and I suppose that works,
but perhaps it makes sense to be able to tell the compiler that yes,
these things are the actual same thing. Perhaps it will even improve
code-gen, like in your example.