Re: [RFC v2 3/3] atomic: Add alignment check to instrumented atomic operations

[Peter Zijlstra <peterz@infradead.org>]

On Sun, Sep 14, 2025 at 10:45:29AM +1000, Finn Thain wrote:
> From: Peter Zijlstra <peterz@infradead.org>
> 
> Add a Kconfig option for debug builds which logs a warning when an
> instrumented atomic operation takes place at some location that isn't
> a long word boundary. Some platforms don't trap for this.
> 
> Link: https://lore.kernel.org/lkml/20250901093600.GF4067720@noisy.programming.kicks-ass.net/
> ---
> This patch differs slightly from Peter's code which checked for natural
> alignment.
> ---
>  include/linux/instrumented.h |  4 ++++
>  lib/Kconfig.debug            | 10 ++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/include/linux/instrumented.h b/include/linux/instrumented.h
> index 711a1f0d1a73..55f5685971a1 100644
> --- a/include/linux/instrumented.h
> +++ b/include/linux/instrumented.h
> @@ -7,6 +7,7 @@
>  #ifndef _LINUX_INSTRUMENTED_H
>  #define _LINUX_INSTRUMENTED_H
>  
> +#include <linux/bug.h>
>  #include <linux/compiler.h>
>  #include <linux/kasan-checks.h>
>  #include <linux/kcsan-checks.h>
> @@ -67,6 +68,7 @@ static __always_inline void instrument_atomic_read(const volatile void *v, size_
>  {
>  	kasan_check_read(v, size);
>  	kcsan_check_atomic_read(v, size);
> +	WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ATOMIC) && ((unsigned long)v & (sizeof(long) - 1)));
>  }
>  
>  /**
> @@ -81,6 +83,7 @@ static __always_inline void instrument_atomic_write(const volatile void *v, size
>  {
>  	kasan_check_write(v, size);
>  	kcsan_check_atomic_write(v, size);
> +	WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ATOMIC) && ((unsigned long)v & (sizeof(long) - 1)));
>  }
>  
>  /**
> @@ -95,6 +98,7 @@ static __always_inline void instrument_atomic_read_write(const volatile void *v,
>  {
>  	kasan_check_write(v, size);
>  	kcsan_check_atomic_read_write(v, size);
> +	WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ATOMIC) && ((unsigned long)v & (sizeof(long) - 1)));
>  }

Right, so why aren't we trusting the size argument? And instead
mandating a possibly larger alignment?

Note how things like test_and_set_bit() will use sizeof(long), while
atomic_set() will use sizeof(*v), which, on LP64 architectures are very
much not the same.

The same with atomic_*() vs atomic_long_*() / atomic64_*(), they will
have different alignment requirements.

And then there is cmpxchg(), that can be u8 u16, u32 and u64 depending
on the user. And then there is cmpxchg128().

I really don't see how using long here is correct.