Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy

[Christophe Leroy (CS GROUP)]

Le 09/10/2026 à 23:28, Ian Rogers a écrit :
> On Wed, Oct 7, 2026 at 4:00 AM Arnd Bergmann <arnd@xxxxxxxx> wrote:
> > On Tue, Oct 6, 2026, at 14:31, Marc Kleine-Budde wrote:
> > > Cc+ Arnd
> >
> > Thanks for the Cc!
> >
> > > On 28.09.2026 17:39:15, Stefan Kerkmann wrote:
> > > > Hi Ian,
> > > >
> > > > On 10/16/25 22:51, Ian Rogers wrote:
> > > > > Type punning is necessary for get/put unaligned but the use of a
> > > > > packed struct violates strict aliasing rules, requiring
> > > > > -fno-strict-aliasing to be passed to the C compiler. Switch to using
> > > > > memcpy so that -fno-strict-aliasing isn't necessary.
> > > > >
> > > > > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> > > > > ---
> > >
> > > > Is this an accepted trade-off? My understanding is that the kernel is always
> > > > built with -fno-strict-aliasing, so the packed-struct type punning was well
> > > > defined there, and the __packed annotation is what lets GCC generate valid
> > > > code for the unaligned access.
> >
> > The previous upstream version was the result of endless discussions, and it
> > looks like changing it to the memcpy version was premature. At the time we
> > unified all architectures to use a common implentation, this was the only one
> > that resulted in correct and fast code on all architectures, so I don't
> > understand why this was just applied without including everyone who was
> > involved in coming up with the version that was replaced.
> >
> > My feeling is that we should just revert this. I'm not sure about the
> > motivation for the change. It sounds like this was meant to be
> > used in userland code, and that clashed with assumptions we make
> > in the kernel, but I don't think that is sufficient reason for
> > regressing kernel code.
>
> So the original motivation for the change was that the perf tool had
> an OpenSSL dependency for the sake of doing a hash when copying jitted
> code into a fake ELF file for the purpose of disassembly and
> symbolization. The kernel contained the same hash function, and using
> the kernel function allowed the perf tool to avoid an OpenSSL
> dependency. Linus asked the perf tool to minimize its dependencies, so
> we pursued this change. The kernel hash function used the
> get_unaligned code for unaligned memory accesses, meaning the change
> required the perf tool to add -fno-strict-aliasing to its build flags
> until we could have a strict aliasing safe get_unaligned. The strict
> aliasing safe code is what we're discussing here and when originally
> written it sat on the mailing list not really doing anything. To my
> surprise Thomas Gleixner picked it up 6 months later, and I believe
> something other than the perf tool motivated this.
>
> There was an issue with the original series on Power IIRC, they had a
> char global variable that they knew was an int through linker tricks.
> The change caused a correct compiler error of a 4 byte copy to a byte
> sized address, and I forget if we used pragmas or a local packed
> struct get_unaligned implementation to work around this. I didn't have
> a way to replicate the problem locally, I was glad others were helping
> with the series!

I can't remember any special issue with powerpc, I've looked into the 
history and couldn't find anything either.

As far as I can see the only problem we had was with your version v1 
where you were using memcpy() instead of builtin_memcpy(), leading to a 
VDSO link failure due to missing memcpy() function.

But if you think about other problems with powerpc let me know and I'll 
look at it.

> VDSO build fails with this patch:
>
>    VDSO32L arch/powerpc/kernel/vdso/vdso32.so.dbg
> arch/powerpc/kernel/vdso/vdso32.so.dbg: dynamic relocations are not
> supported
> make[2]: *** [arch/powerpc/kernel/vdso/Makefile:79: arch/powerpc/kernel/
> vdso/vdso32.so.dbg] Error 1
>
> Behind the relocation issue, calling memcpy() for a single 4-bytes word
> kills performance.
>
>   170:   7f e4 fb 78     mr      r4,r31
>   174:   38 a0 00 04     li      r5,4
>   178:   38 61 00 10     addi    r3,r1,16
>   17c:   93 81 00 10     stw     r28,16(r1)
>   180:   48 00 00 01     bl      180 <__c_kernel_getrandom+0x180>
>                          180: R_PPC_REL24        memcpy
>   184:   38 81 00 10     addi    r4,r1,16

Christophe