Re: [RFC v2 2/3] atomic: Specify alignment for atomic_t and atomic64_t

[Arnd Bergmann]

On Wed, Oct 1, 2025, at 03:03, Finn Thain wrote:
> On Tue, 30 Sep 2025, Arnd Bergmann wrote:

>> What is the alignment of stack variables on m68k? E.g. if you have a 
>> function with two local variables, would that still be able to trigger 
>> the check?
>> 
>> int f(atomic64_t *a)
>> {
>>      u16 pad;
>>      u64 old;
>>      
>>      g(&pad);
>>      atomic64_try_cmpxchg(a, &old, 0);
>> }
>> 
>
> I assume so:
>
> int foo(void) {
>     short s;
>     long long ll;
>     return alignof(ll);
> }
>
> # Compilation provided by Compiler Explorer at https://godbolt.org/
> foo():
>         link.w %fp,#0
>         moveq #2,%d0
>         unlk %fp
>         rts

This just returns the guaranteed alignment of the 'long long'
type based on -malign-int/-mno-align-int. Checking again I
find that gcc's m68k-linux target aligns stack allocations
to 4 bytes, though the m68k-unknown target apparently keeps
the 2 byte alignment:

https://gcc.gnu.org/legacy-ml/gcc-patches/2007-09/msg01572.html

https://godbolt.org/z/48fGMj56W

Surprisingly the godbolt.org link also shows a significant
overhead when building the same code with -malign-int
in the second tab. This is unrelated to the issue here,
but I wonder if that is something to report to the gcc
bug tracker if we ever get to building the kernel with
-malign-int. Similarly, I noticed that clang does not
support the -malign-int flag on m68k at all.

>> Since there is nothing telling the compiler that the 'old' argument to 
>> atomic*_try_cmpcxchg() needs to be naturally aligned, maybe that check 
>> should be changed to only test for the ABI-guaranteed alignment? I think 
>> that would still be needed on x86-32.
>>  
>
> I don't know why we would check the alignment of the 'old' quantity. It's 
> going to be loaded into a register before being used, right?

I was wondering about that as well, but checking for alignof(*old)
probably can't hurt. The only architectures that actually have
a custom arch_try_cmpxchg*() are s390 and x86 and those don't
care about alignmnent of 'old', but it's possible that another
architecture that can't handle unaligned load/store would add
an inline asm implementation in the future and break if an
alignment fixup happens in the middle of an ll/sc loop.

      Arnd