Re: [PATCH tip 1/2] signal, perf: Fix siginfo_t by avoiding u64 on 32-bit architectures

From: Marco Elver
Date: Thu Apr 22 2021 - 15:22:50 EST


On Thu, 22 Apr 2021 at 12:17, Marco Elver <elver@xxxxxxxxxx> wrote:
> On Thu, 22 Apr 2021 at 11:48, David Laight <David.Laight@xxxxxxxxxx> wrote:
> >
> > From: Marco Elver
> > > Sent: 22 April 2021 07:45
> > >
> > > On some architectures, like Arm, the alignment of a structure is that of
> > > its largest member.
> >
> > That is true everywhere.
> > (Apart from obscure ABI where structure have at least 4 byte alignment!)
>
> For instance, x86 didn't complain, nor did m68k. Both of them have
> compile-time checks for the layout (I'm adding those for Arm
> elsewhere).
[...]
> > Much as I hate __packed, you could add __packed to the
> > definition of the structure member _perf.
> > The compiler will remove the padding before it and will
> > assume it has the alignment of the previous item.
> >
> > So it will never use byte accesses.
>
> Sure __packed works for Arm. But I think there's no precedent using
> this on siginfo_t, possibly for good reasons? I simply can't find
> evidence that this is portable on *all* architectures and for *all*
> possible definitions of siginfo_t, including those that live in things
> like glibc.
>
> Can we confirm that __packed is fine to add to siginfo_t on *all*
> architectures for *all* possible definitions of siginfo_t? I currently
> can't. And given it's outside the scope of the C standard (as of C11
> we got _Alignas, but that doesn't help I think), I'd vote to not
> venture too far for code that should be portable especially things as
> important as siginfo_t, and has definitions *outside* the kernel (I
> know we do lots of non-standard things, but others might not).

After thinking about this all afternoon, you convinced me that the
commit message wasn't great, and this should be in the commit message,
too: https://lkml.kernel.org/r/20210422191823.79012-1-elver@xxxxxxxxxx

Thanks,
-- Marco