Re: [PATCH] atomic: Specify natural alignment for atomic_t

[Lance Yang <lance.yang@linux.dev>]

Thanks for digging deeper!

On 2025/8/25 18:49, Finn Thain wrote:
> 
> [Belated Cc linux-m68k...]
> 
> On Mon, 25 Aug 2025, Lance Yang wrote:
> 
>>
>> On 2025/8/25 14:17, Finn Thain wrote:
>>>
>>> On Mon, 25 Aug 2025, Lance Yang wrote:
>>>
>>>>
>>>> What if we squash the runtime check fix into your patch?
>>>
>>> Did my patch not solve the problem?
>>
>> Hmm... it should solve the problem for natural alignment, which is a
>> critical fix.
>>
>> But it cannot solve the problem of forced misalignment from drivers
>> using #pragma pack(1). The runtime warning will still trigger in those
>> cases.
>>
>> I built a simple test module on a kernel with your patch applied:
>>
>> ```
>> #include <linux/module.h>
>> #include <linux/init.h>
>>
>> struct __attribute__((packed)) test_container {
>>      char padding[49];
>>      struct mutex io_lock;
>> };
>>
>> static int __init alignment_init(void)
>> {
>>      struct test_container cont;
>>      pr_info("io_lock address offset mod 4: %lu\n", (unsigned long)&cont.io_lock % 4);
>>      return 0;
>> }
>>
>> static void __exit alignment_exit(void)
>> {
>>      pr_info("Module unloaded\n");
>> }
>>
>> module_init(alignment_init);
>> module_exit(alignment_exit);
>> MODULE_LICENSE("GPL");
>> MODULE_AUTHOR("x");
>> MODULE_DESCRIPTION("x");
>> ```
>>
>> Result from dmesg:
>> [Mon Aug 25 15:44:50 2025] io_lock address offset mod 4: 1
>>
> 
> Thanks for sending code to illustrate your point. Unfortunately, I was not
> able to reproduce your results. Tested on x86, your test module shows no
> misalignment:
> 
> [131840.349042] io_lock address offset mod 4: 0
> 
> Tested on m68k I also get 0, given the patch at the top of this thread:
> 
> [    0.400000] io_lock address offset mod 4: 0
> 
>>
>> As we can see, a packed struct can still force the entire mutex object
>> to an unaligned address. With an address like this, the WARN_ON_ONCE can
>> still be triggered.

> 
> I don't think so. But there is something unexpected going on here -- the
> output from pahole appears to say io_lock is at offset 49, which seems to
> contradict the printk() output above.

Interesting! That contradiction is the key. It seems we're seeing different
compiler behaviors.

I'm on GCC 14.2.0 (Debian 14.2.0-19), and it appears to be strictly 
respecting
the packed attribute.

So let's print something more:

```
#include <linux/module.h>
#include <linux/init.h>

struct __attribute__((packed)) test_container {
     char padding[49];
     struct mutex io_lock;
};

static int __init alignment_init(void)
{
     struct test_container cont;
     pr_info("Container base address      : %px\n", &cont);
     pr_info("io_lock member address      : %px\n", &cont.io_lock);
     pr_info("io_lock address offset mod 4: %lu\n", (unsigned 
long)&cont.io_lock % 4);
     return 0;
}

static void __exit alignment_exit(void)
{
     pr_info("Module unloaded\n");
}

module_init(alignment_init);
module_exit(alignment_exit);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("x");
MODULE_DESCRIPTION("x");
```

Result from dmesg:

```
[Mon Aug 25 19:15:33 2025] Container base address      : ff1100063570f840
[Mon Aug 25 19:15:33 2025] io_lock member address      : ff1100063570f871
[Mon Aug 25 19:15:33 2025] io_lock address offset mod 4: 1
```

io_lock is exactly base + 49, resulting in misalignment.

Seems like your compiler is cleverly re-aligning the whole struct on the
stack, but we can't rely on that behavior, as it's not guaranteed across
all compilers or versions. wdyt?