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

["Christophe Leroy (CS GROUP)" <chleroy@kernel.org>]

Le 09/10/2026 à 23:28, Ian Rogers a écrit :
> On Wed, Oct 7, 2026 at 4:00 AM Arnd Bergmann <arnd@arndb.de> 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@google.com>
>>>>> ---
>>>
>>>> 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