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

[Ian Rogers]

On Sat, Oct 10, 2026 at 3:26 AM Christophe Leroy (CS GROUP)
<chleroy@xxxxxxxxxx> wrote:
>
> 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.

Thanks Christophe! The issues weren't performance regressions, as here
with ARMv5, but build bots that failed due to warnings about the
memcpy source or destination being invalid.

You are right that PowerPC wasn't the architecture that failed; it was
PA-RISC. Here is what I could find in the LKML history:

PA-RISC build error:
https://lore.kernel.org/lkml/CAP-5=fVEp8UPS-B3X=66AwYbbbs8559AHEObpQZSVnSnVYSxhA@xxxxxxxxxxxxxx/
PA-RISC workaround:
https://lore.kernel.org/lkml/20251016205126.2882625-2-irogers@xxxxxxxxxx/
MIPS failure with suggested workaround:
https://lore.kernel.org/lkml/CAP-5=fVBZPBz6J1omfvSS4JLkueRGqCdouLji6gsFLTeu9nJTw@xxxxxxxxxxxxxx/

Thanks,
Ian

>  > 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