[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <86be5cf0-065e-d55d-fdb6-b9cf6655165e@linux-m68k.org>
Date: Tue, 23 Sep 2025 16:28:38 +1000 (AEST)
From: Finn Thain <fthain@...ux-m68k.org>
To: Arnd Bergmann <arnd@...db.de>
cc: Geert Uytterhoeven <geert@...ux-m68k.org>,
Peter Zijlstra <peterz@...radead.org>, Will Deacon <will@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Boqun Feng <boqun.feng@...il.com>, Jonathan Corbet <corbet@....net>,
Mark Rutland <mark.rutland@....com>, linux-kernel@...r.kernel.org,
Linux-Arch <linux-arch@...r.kernel.org>, linux-m68k@...r.kernel.org,
Lance Yang <lance.yang@...ux.dev>
Subject: Re: [RFC v2 2/3] atomic: Specify alignment for atomic_t and
atomic64_t
On Mon, 22 Sep 2025, Arnd Bergmann wrote:
> On Mon, Sep 22, 2025, at 10:16, Finn Thain wrote:
> >
> > Yes, I noticed that also, after I started building with defconfigs.
> > I pushed a new patch to my github repo.
> >
> > https://github.com/fthain/linux/tree/atomic_t
>
> I had a look at that repository, and I think the extra annotations you
> added on a lot of structures have the same mistake that I made when
> looking at annotating ABI structures for -malign-int earlier:
>
> Adding __aligned__((sizeof(long))) to a structure definition only
> changes the /outer/ alignment of the structure itself,
Right. I should have dropped those changes earlier.
@@ -8,7 +8,7 @@ struct vfsmount;
struct path {
struct vfsmount *mnt;
struct dentry *dentry;
-} __randomize_layout;
+} __aligned(sizeof(long)) __randomize_layout;
There's no need: struct path contains a struct dentry, which contains a
seqcount_spinlock_t, which contains a spinlock_t which contains an
atomic_t member, which is explicitly aligned.
Despite that, there's still some kmem cache or other allocator somewhere
that has produced some misaligned path and dentry structures. So we get
misaligned atomics somewhere in the VFS and TTY layers. I was unable to
find those allocations.
> so e.g.
>
> struct {
> short a;
> int b;
> } __attribute__((aligned(sizeof(long))));
>
> creates a structure with 4-byte alignment and 8-byte
> size when building with -mno-align-int, but the padding
> is added after 'b', which is still misaligned.
>
Sure but -mno-align-int isn't particularly relevant here as it's only for
m68k. Besides, I'd like to avoid the mess that would create.
The basic aim here is to naturally align both atomic_t and atomic64_t on
all architectures (my testing is confined to m68k) and to try to find out
what that would cost and what benefit it might bring.
The patches you're reviewing were a hack to try to silence the WARN from
CONFIG_DEBUG_ATOMIC, because that way I could try to assess the
cost/benefit.
Powered by blists - more mailing lists