* [PATCH v5 0/4] Switch get/put unaligned to use memcpy
@ 2025-10-16 20:51 Ian Rogers
2025-10-16 20:51 ` [PATCH v5 1/4] parisc: Inline a type punning version of get_unaligned_le32 Ian Rogers
` (3 more replies)
0 siblings, 4 replies; 36+ messages in thread
From: Ian Rogers @ 2025-10-16 20:51 UTC (permalink / raw)
To: James E.J. Bottomley, Helge Deller, Andy Lutomirski,
Thomas Gleixner, Vincenzo Frascino, Ian Rogers,
Arnaldo Carvalho de Melo, linux-parisc, linux-kernel,
Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld
The existing type punning approach with packed structs requires
-fno-strict-aliasing to be passed to the compiler for
correctness. This is true in the kernel tree but was not true in the
tools directory until this patch from Eric Biggers <ebiggers@google.com>:
https://lore.kernel.org/lkml/20250625202311.23244-2-ebiggers@kernel.org/
Requiring -fno-strict-aliasing seems unfortunate and so this patch
makes the unaligned code work via memcpy rather than type punning with
the packed attribute.
v5: add a patch to make parisc still use a punned version of
get_unaligned_le32 for an unusual boot case they have. This is
untested but suggested as necessary by:
https://lore.kernel.org/lkml/202509051042.7KOze0fZ-lkp@intel.com/
I wasn't clear if this work was picked up, but I don't see it in
v6.18-rc1 and so I'm resending rebased as v5.
v4: switch the type/expression variable __get_unaligned_ctrl_type that
is used by _Generic to be a pointer to avoid 0 vs NULL usage
warnings - always use NULL and dereference the type. This should
also hopefully address analysis bots complaints.
v3: switch to __unqual_scalar_typeof, reducing the code, and use an
uninitialized variable rather than a cast of 0 to try to avoid a
sparse warning about not using NULL. The code is trying to
navigate a minefield of uninitialized and casting warnings,
hopefully the best balance has been struck, but the code will fail
for cases like:
const void *val = get_unaligned((const void * const *)ptr);
due to __unqual_scalar_typeof leaving the 2nd const of the cast in
place. Thankfully no code does this - tested with an
allyesconfig. Support would be achievable by using void* as a
default case in __unqual_scalar_typeof, it just doesn't seem worth
it for a fairly unusual const case.
v2: switch memcpy to __builtin_memcpy to avoid potential/disallowed
memcpy calls in vdso caused by -fno-builtin. Reported by
Christophe Leroy <christophe.leroy@csgroup.eu>:
https://lore.kernel.org/lkml/c57de5bf-d55c-48c5-9dfa-e2fb844dafe9@csgroup.eu/
Ian Rogers (4):
parisc: Inline a type punning version of get_unaligned_le32
vdso: Switch get/put unaligned from packed struct to memcpy
tools headers: Update the linux/unaligned.h copy with the kernel
sources
tools headers: Remove unneeded ignoring of warnings in unaligned.h
arch/parisc/boot/compressed/misc.c | 15 +++++++++-
include/vdso/unaligned.h | 41 ++++++++++++++++++++++++----
tools/include/linux/compiler_types.h | 22 +++++++++++++++
tools/include/linux/unaligned.h | 4 ---
tools/include/vdso/unaligned.h | 41 ++++++++++++++++++++++++----
5 files changed, 106 insertions(+), 17 deletions(-)
--
2.51.0.858.gf9c4a03a3a-goog
^ permalink raw reply [flat|nested] 36+ messages in thread* [PATCH v5 1/4] parisc: Inline a type punning version of get_unaligned_le32 2025-10-16 20:51 [PATCH v5 0/4] Switch get/put unaligned to use memcpy Ian Rogers @ 2025-10-16 20:51 ` Ian Rogers 2026-01-13 13:47 ` [tip: timers/vdso] parisc: Inline a type punning version of get_unaligned_le32() tip-bot2 for Ian Rogers 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 2025-10-16 20:51 ` [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Ian Rogers ` (2 subsequent siblings) 3 siblings, 2 replies; 36+ messages in thread From: Ian Rogers @ 2025-10-16 20:51 UTC (permalink / raw) To: James E.J. Bottomley, Helge Deller, Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, Ian Rogers, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld Reading the byte/char output_len with get_unaligned_le32 can trigger compiler warnings due to the size read. Avoid these warnings by using type punning. This avoids issues when switching get_unaligned_t to __builtin_memcpy. Signed-off-by: Ian Rogers <irogers@google.com> --- arch/parisc/boot/compressed/misc.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/arch/parisc/boot/compressed/misc.c b/arch/parisc/boot/compressed/misc.c index 9c83bd06ef15..111f267230a1 100644 --- a/arch/parisc/boot/compressed/misc.c +++ b/arch/parisc/boot/compressed/misc.c @@ -278,6 +278,19 @@ static void parse_elf(void *output) free(phdrs); } +/* + * The regular get_unaligned_le32 uses __builtin_memcpy which can trigger + * warnings when reading a byte/char output_len as an integer, as the size of a + * char is less than that of an integer. Use type punning and the packed + * attribute, which requires -fno-strict-aliasing, to work around the problem. + */ +static u32 punned_get_unaligned_le32(const void *p) +{ + const struct { __le32 x; } __packed * __get_pptr = p; + + return le32_to_cpu(__get_pptr->x); +} + asmlinkage unsigned long __visible decompress_kernel(unsigned int started_wide, unsigned int command_line, const unsigned int rd_start, @@ -309,7 +322,7 @@ asmlinkage unsigned long __visible decompress_kernel(unsigned int started_wide, * leave 2 MB for the stack. */ vmlinux_addr = (unsigned long) &_ebss + 2*1024*1024; - vmlinux_len = get_unaligned_le32(&output_len); + vmlinux_len = punned_get_unaligned_le32(&output_len); output = (char *) vmlinux_addr; /* -- 2.51.0.858.gf9c4a03a3a-goog ^ permalink raw reply [flat|nested] 36+ messages in thread
* [tip: timers/vdso] parisc: Inline a type punning version of get_unaligned_le32() 2025-10-16 20:51 ` [PATCH v5 1/4] parisc: Inline a type punning version of get_unaligned_le32 Ian Rogers @ 2026-01-13 13:47 ` tip-bot2 for Ian Rogers 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 1 sibling, 0 replies; 36+ messages in thread From: tip-bot2 for Ian Rogers @ 2026-01-13 13:47 UTC (permalink / raw) To: linux-tip-commits; +Cc: Ian Rogers, Thomas Gleixner, x86, linux-kernel The following commit has been merged into the timers/vdso branch of tip: Commit-ID: feffe3a9d4b0d1672db4c16457f029f1c22b35da Gitweb: https://git.kernel.org/tip/feffe3a9d4b0d1672db4c16457f029f1c22b35da Author: Ian Rogers <irogers@google.com> AuthorDate: Thu, 16 Oct 2025 13:51:23 -07:00 Committer: Thomas Gleixner <tglx@kernel.org> CommitterDate: Tue, 13 Jan 2026 14:46:00 +01:00 parisc: Inline a type punning version of get_unaligned_le32() Reading the byte/char output_len with get_unaligned_le32() can trigger compiler warnings due to the size read. Avoid these warnings by using type punning. This avoids issues when switching get_unaligned_t() to __builtin_memcpy(). Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Thomas Gleixner <tglx@kernel.org> Link: https://patch.msgid.link/20251016205126.2882625-2-irogers@google.com --- arch/parisc/boot/compressed/misc.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/arch/parisc/boot/compressed/misc.c b/arch/parisc/boot/compressed/misc.c index 9c83bd0..111f267 100644 --- a/arch/parisc/boot/compressed/misc.c +++ b/arch/parisc/boot/compressed/misc.c @@ -278,6 +278,19 @@ static void parse_elf(void *output) free(phdrs); } +/* + * The regular get_unaligned_le32 uses __builtin_memcpy which can trigger + * warnings when reading a byte/char output_len as an integer, as the size of a + * char is less than that of an integer. Use type punning and the packed + * attribute, which requires -fno-strict-aliasing, to work around the problem. + */ +static u32 punned_get_unaligned_le32(const void *p) +{ + const struct { __le32 x; } __packed * __get_pptr = p; + + return le32_to_cpu(__get_pptr->x); +} + asmlinkage unsigned long __visible decompress_kernel(unsigned int started_wide, unsigned int command_line, const unsigned int rd_start, @@ -309,7 +322,7 @@ asmlinkage unsigned long __visible decompress_kernel(unsigned int started_wide, * leave 2 MB for the stack. */ vmlinux_addr = (unsigned long) &_ebss + 2*1024*1024; - vmlinux_len = get_unaligned_le32(&output_len); + vmlinux_len = punned_get_unaligned_le32(&output_len); output = (char *) vmlinux_addr; /* ^ permalink raw reply [flat|nested] 36+ messages in thread
* [tip: timers/vdso] parisc: Inline a type punning version of get_unaligned_le32() 2025-10-16 20:51 ` [PATCH v5 1/4] parisc: Inline a type punning version of get_unaligned_le32 Ian Rogers 2026-01-13 13:47 ` [tip: timers/vdso] parisc: Inline a type punning version of get_unaligned_le32() tip-bot2 for Ian Rogers @ 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 1 sibling, 0 replies; 36+ messages in thread From: tip-bot2 for Ian Rogers @ 2026-01-14 8:01 UTC (permalink / raw) To: linux-tip-commits; +Cc: Ian Rogers, Thomas Gleixner, x86, linux-kernel The following commit has been merged into the timers/vdso branch of tip: Commit-ID: df0f9a664be55a8529362a1ada847a19a91e4807 Gitweb: https://git.kernel.org/tip/df0f9a664be55a8529362a1ada847a19a91e4807 Author: Ian Rogers <irogers@google.com> AuthorDate: Thu, 16 Oct 2025 13:51:23 -07:00 Committer: Thomas Gleixner <tglx@kernel.org> CommitterDate: Wed, 14 Jan 2026 08:56:41 +01:00 parisc: Inline a type punning version of get_unaligned_le32() Reading the byte/char output_len with get_unaligned_le32() can trigger compiler warnings due to the size read. Avoid these warnings by using type punning. This avoids issues when switching get_unaligned_t() to __builtin_memcpy(). Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Thomas Gleixner <tglx@kernel.org> Link: https://patch.msgid.link/20251016205126.2882625-2-irogers@google.com --- arch/parisc/boot/compressed/misc.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/arch/parisc/boot/compressed/misc.c b/arch/parisc/boot/compressed/misc.c index 9c83bd0..111f267 100644 --- a/arch/parisc/boot/compressed/misc.c +++ b/arch/parisc/boot/compressed/misc.c @@ -278,6 +278,19 @@ static void parse_elf(void *output) free(phdrs); } +/* + * The regular get_unaligned_le32 uses __builtin_memcpy which can trigger + * warnings when reading a byte/char output_len as an integer, as the size of a + * char is less than that of an integer. Use type punning and the packed + * attribute, which requires -fno-strict-aliasing, to work around the problem. + */ +static u32 punned_get_unaligned_le32(const void *p) +{ + const struct { __le32 x; } __packed * __get_pptr = p; + + return le32_to_cpu(__get_pptr->x); +} + asmlinkage unsigned long __visible decompress_kernel(unsigned int started_wide, unsigned int command_line, const unsigned int rd_start, @@ -309,7 +322,7 @@ asmlinkage unsigned long __visible decompress_kernel(unsigned int started_wide, * leave 2 MB for the stack. */ vmlinux_addr = (unsigned long) &_ebss + 2*1024*1024; - vmlinux_len = get_unaligned_le32(&output_len); + vmlinux_len = punned_get_unaligned_le32(&output_len); output = (char *) vmlinux_addr; /* ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2025-10-16 20:51 [PATCH v5 0/4] Switch get/put unaligned to use memcpy Ian Rogers 2025-10-16 20:51 ` [PATCH v5 1/4] parisc: Inline a type punning version of get_unaligned_le32 Ian Rogers @ 2025-10-16 20:51 ` Ian Rogers 2025-10-19 17:24 ` David Laight ` (3 more replies) 2025-10-16 20:51 ` [PATCH v5 3/4] tools headers: Update the linux/unaligned.h copy with the kernel sources Ian Rogers 2025-10-16 20:51 ` [PATCH v5 4/4] tools headers: Remove unneeded ignoring of warnings in unaligned.h Ian Rogers 3 siblings, 4 replies; 36+ messages in thread From: Ian Rogers @ 2025-10-16 20:51 UTC (permalink / raw) To: James E.J. Bottomley, Helge Deller, Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, Ian Rogers, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld 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> --- include/vdso/unaligned.h | 41 ++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/include/vdso/unaligned.h b/include/vdso/unaligned.h index ff0c06b6513e..9076483c9fbb 100644 --- a/include/vdso/unaligned.h +++ b/include/vdso/unaligned.h @@ -2,14 +2,43 @@ #ifndef __VDSO_UNALIGNED_H #define __VDSO_UNALIGNED_H -#define __get_unaligned_t(type, ptr) ({ \ - const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr); \ - __get_pptr->x; \ +#include <linux/compiler_types.h> + +/** + * __get_unaligned_t - read an unaligned value from memory. + * @type: the type to load from the pointer. + * @ptr: the pointer to load from. + * + * Use memcpy to affect an unaligned type sized load avoiding undefined behavior + * from approaches like type punning that require -fno-strict-aliasing in order + * to be correct. As type may be const, use __unqual_scalar_typeof to map to a + * non-const type - you can't memcpy into a const type. The + * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required + * expression rather than type, a pointer is used to avoid warnings about mixing + * the use of 0 and NULL. The void* cast silences ubsan warnings. + */ +#define __get_unaligned_t(type, ptr) ({ \ + type *__get_unaligned_ctrl_type __always_unused = NULL; \ + __unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \ + __builtin_memcpy(&__get_unaligned_val, (void *)(ptr), \ + sizeof(__get_unaligned_val)); \ + __get_unaligned_val; \ }) -#define __put_unaligned_t(type, val, ptr) do { \ - struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr); \ - __put_pptr->x = (val); \ +/** + * __put_unaligned_t - write an unaligned value to memory. + * @type: the type of the value to store. + * @val: the value to store. + * @ptr: the pointer to store to. + * + * Use memcpy to affect an unaligned type sized store avoiding undefined + * behavior from approaches like type punning that require -fno-strict-aliasing + * in order to be correct. The void* cast silences ubsan warnings. + */ +#define __put_unaligned_t(type, val, ptr) do { \ + type __put_unaligned_val = (val); \ + __builtin_memcpy((void *)(ptr), &__put_unaligned_val, \ + sizeof(__put_unaligned_val)); \ } while (0) #endif /* __VDSO_UNALIGNED_H */ -- 2.51.0.858.gf9c4a03a3a-goog ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2025-10-16 20:51 ` [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Ian Rogers @ 2025-10-19 17:24 ` David Laight 2026-01-13 13:47 ` [tip: timers/vdso] vdso: Switch get/put_unaligned() from packed struct to memcpy() tip-bot2 for Ian Rogers ` (2 subsequent siblings) 3 siblings, 0 replies; 36+ messages in thread From: David Laight @ 2025-10-19 17:24 UTC (permalink / raw) To: Ian Rogers Cc: James E.J. Bottomley, Helge Deller, Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld On Thu, 16 Oct 2025 13:51:24 -0700 Ian Rogers <irogers@google.com> 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. Does the compiler always manage to optimise everything away? You really do need it to generate the code for a misaligned memory access. You might be better off removing the 'strict-aliasing' warning by 'laundering' the pointer through an integer type (probably long). David > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > include/vdso/unaligned.h | 41 ++++++++++++++++++++++++++++++++++------ > 1 file changed, 35 insertions(+), 6 deletions(-) > > diff --git a/include/vdso/unaligned.h b/include/vdso/unaligned.h > index ff0c06b6513e..9076483c9fbb 100644 > --- a/include/vdso/unaligned.h > +++ b/include/vdso/unaligned.h > @@ -2,14 +2,43 @@ > #ifndef __VDSO_UNALIGNED_H > #define __VDSO_UNALIGNED_H > > -#define __get_unaligned_t(type, ptr) ({ \ > - const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr); \ > - __get_pptr->x; \ > +#include <linux/compiler_types.h> > + > +/** > + * __get_unaligned_t - read an unaligned value from memory. > + * @type: the type to load from the pointer. > + * @ptr: the pointer to load from. > + * > + * Use memcpy to affect an unaligned type sized load avoiding undefined behavior > + * from approaches like type punning that require -fno-strict-aliasing in order > + * to be correct. As type may be const, use __unqual_scalar_typeof to map to a > + * non-const type - you can't memcpy into a const type. The > + * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required > + * expression rather than type, a pointer is used to avoid warnings about mixing > + * the use of 0 and NULL. The void* cast silences ubsan warnings. > + */ > +#define __get_unaligned_t(type, ptr) ({ \ > + type *__get_unaligned_ctrl_type __always_unused = NULL; \ > + __unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \ > + __builtin_memcpy(&__get_unaligned_val, (void *)(ptr), \ > + sizeof(__get_unaligned_val)); \ > + __get_unaligned_val; \ > }) > > -#define __put_unaligned_t(type, val, ptr) do { \ > - struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr); \ > - __put_pptr->x = (val); \ > +/** > + * __put_unaligned_t - write an unaligned value to memory. > + * @type: the type of the value to store. > + * @val: the value to store. > + * @ptr: the pointer to store to. > + * > + * Use memcpy to affect an unaligned type sized store avoiding undefined > + * behavior from approaches like type punning that require -fno-strict-aliasing > + * in order to be correct. The void* cast silences ubsan warnings. > + */ > +#define __put_unaligned_t(type, val, ptr) do { \ > + type __put_unaligned_val = (val); \ > + __builtin_memcpy((void *)(ptr), &__put_unaligned_val, \ > + sizeof(__put_unaligned_val)); \ > } while (0) > > #endif /* __VDSO_UNALIGNED_H */ ^ permalink raw reply [flat|nested] 36+ messages in thread
* [tip: timers/vdso] vdso: Switch get/put_unaligned() from packed struct to memcpy() 2025-10-16 20:51 ` [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Ian Rogers 2025-10-19 17:24 ` David Laight @ 2026-01-13 13:47 ` tip-bot2 for Ian Rogers 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 2026-09-28 15:39 ` [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Stefan Kerkmann 3 siblings, 0 replies; 36+ messages in thread From: tip-bot2 for Ian Rogers @ 2026-01-13 13:47 UTC (permalink / raw) To: linux-tip-commits; +Cc: Ian Rogers, Thomas Gleixner, x86, linux-kernel The following commit has been merged into the timers/vdso branch of tip: Commit-ID: e04a494143bab7ea804fe1ebe286701ee8288e4a Gitweb: https://git.kernel.org/tip/e04a494143bab7ea804fe1ebe286701ee8288e4a Author: Ian Rogers <irogers@google.com> AuthorDate: Thu, 16 Oct 2025 13:51:24 -07:00 Committer: Thomas Gleixner <tglx@kernel.org> CommitterDate: Tue, 13 Jan 2026 14:46:00 +01:00 vdso: Switch get/put_unaligned() from packed struct to memcpy() 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> Signed-off-by: Thomas Gleixner <tglx@kernel.org> Link: https://patch.msgid.link/20251016205126.2882625-3-irogers@google.com --- include/vdso/unaligned.h | 41 +++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/include/vdso/unaligned.h b/include/vdso/unaligned.h index ff0c06b..9076483 100644 --- a/include/vdso/unaligned.h +++ b/include/vdso/unaligned.h @@ -2,14 +2,43 @@ #ifndef __VDSO_UNALIGNED_H #define __VDSO_UNALIGNED_H -#define __get_unaligned_t(type, ptr) ({ \ - const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr); \ - __get_pptr->x; \ +#include <linux/compiler_types.h> + +/** + * __get_unaligned_t - read an unaligned value from memory. + * @type: the type to load from the pointer. + * @ptr: the pointer to load from. + * + * Use memcpy to affect an unaligned type sized load avoiding undefined behavior + * from approaches like type punning that require -fno-strict-aliasing in order + * to be correct. As type may be const, use __unqual_scalar_typeof to map to a + * non-const type - you can't memcpy into a const type. The + * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required + * expression rather than type, a pointer is used to avoid warnings about mixing + * the use of 0 and NULL. The void* cast silences ubsan warnings. + */ +#define __get_unaligned_t(type, ptr) ({ \ + type *__get_unaligned_ctrl_type __always_unused = NULL; \ + __unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \ + __builtin_memcpy(&__get_unaligned_val, (void *)(ptr), \ + sizeof(__get_unaligned_val)); \ + __get_unaligned_val; \ }) -#define __put_unaligned_t(type, val, ptr) do { \ - struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr); \ - __put_pptr->x = (val); \ +/** + * __put_unaligned_t - write an unaligned value to memory. + * @type: the type of the value to store. + * @val: the value to store. + * @ptr: the pointer to store to. + * + * Use memcpy to affect an unaligned type sized store avoiding undefined + * behavior from approaches like type punning that require -fno-strict-aliasing + * in order to be correct. The void* cast silences ubsan warnings. + */ +#define __put_unaligned_t(type, val, ptr) do { \ + type __put_unaligned_val = (val); \ + __builtin_memcpy((void *)(ptr), &__put_unaligned_val, \ + sizeof(__put_unaligned_val)); \ } while (0) #endif /* __VDSO_UNALIGNED_H */ ^ permalink raw reply [flat|nested] 36+ messages in thread
* [tip: timers/vdso] vdso: Switch get/put_unaligned() from packed struct to memcpy() 2025-10-16 20:51 ` [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Ian Rogers 2025-10-19 17:24 ` David Laight 2026-01-13 13:47 ` [tip: timers/vdso] vdso: Switch get/put_unaligned() from packed struct to memcpy() tip-bot2 for Ian Rogers @ 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 2026-09-28 15:39 ` [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Stefan Kerkmann 3 siblings, 0 replies; 36+ messages in thread From: tip-bot2 for Ian Rogers @ 2026-01-14 8:01 UTC (permalink / raw) To: linux-tip-commits; +Cc: Ian Rogers, Thomas Gleixner, x86, linux-kernel The following commit has been merged into the timers/vdso branch of tip: Commit-ID: a339671db64b12bb02492557d2b0658811286277 Gitweb: https://git.kernel.org/tip/a339671db64b12bb02492557d2b0658811286277 Author: Ian Rogers <irogers@google.com> AuthorDate: Thu, 16 Oct 2025 13:51:24 -07:00 Committer: Thomas Gleixner <tglx@kernel.org> CommitterDate: Wed, 14 Jan 2026 08:56:41 +01:00 vdso: Switch get/put_unaligned() from packed struct to memcpy() 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> Signed-off-by: Thomas Gleixner <tglx@kernel.org> Link: https://patch.msgid.link/20251016205126.2882625-3-irogers@google.com --- include/vdso/unaligned.h | 41 +++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/include/vdso/unaligned.h b/include/vdso/unaligned.h index ff0c06b..9076483 100644 --- a/include/vdso/unaligned.h +++ b/include/vdso/unaligned.h @@ -2,14 +2,43 @@ #ifndef __VDSO_UNALIGNED_H #define __VDSO_UNALIGNED_H -#define __get_unaligned_t(type, ptr) ({ \ - const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr); \ - __get_pptr->x; \ +#include <linux/compiler_types.h> + +/** + * __get_unaligned_t - read an unaligned value from memory. + * @type: the type to load from the pointer. + * @ptr: the pointer to load from. + * + * Use memcpy to affect an unaligned type sized load avoiding undefined behavior + * from approaches like type punning that require -fno-strict-aliasing in order + * to be correct. As type may be const, use __unqual_scalar_typeof to map to a + * non-const type - you can't memcpy into a const type. The + * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required + * expression rather than type, a pointer is used to avoid warnings about mixing + * the use of 0 and NULL. The void* cast silences ubsan warnings. + */ +#define __get_unaligned_t(type, ptr) ({ \ + type *__get_unaligned_ctrl_type __always_unused = NULL; \ + __unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \ + __builtin_memcpy(&__get_unaligned_val, (void *)(ptr), \ + sizeof(__get_unaligned_val)); \ + __get_unaligned_val; \ }) -#define __put_unaligned_t(type, val, ptr) do { \ - struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr); \ - __put_pptr->x = (val); \ +/** + * __put_unaligned_t - write an unaligned value to memory. + * @type: the type of the value to store. + * @val: the value to store. + * @ptr: the pointer to store to. + * + * Use memcpy to affect an unaligned type sized store avoiding undefined + * behavior from approaches like type punning that require -fno-strict-aliasing + * in order to be correct. The void* cast silences ubsan warnings. + */ +#define __put_unaligned_t(type, val, ptr) do { \ + type __put_unaligned_val = (val); \ + __builtin_memcpy((void *)(ptr), &__put_unaligned_val, \ + sizeof(__put_unaligned_val)); \ } while (0) #endif /* __VDSO_UNALIGNED_H */ ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2025-10-16 20:51 ` [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Ian Rogers ` (2 preceding siblings ...) 2026-01-14 8:01 ` tip-bot2 for Ian Rogers @ 2026-09-28 15:39 ` Stefan Kerkmann 2026-09-28 15:58 ` Ian Rogers 2026-10-06 12:31 ` Marc Kleine-Budde 3 siblings, 2 replies; 36+ messages in thread From: Stefan Kerkmann @ 2026-09-28 15:39 UTC (permalink / raw) To: Ian Rogers, James E.J. Bottomley, Helge Deller, Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld 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> > --- > include/vdso/unaligned.h | 41 ++++++++++++++++++++++++++++++++++------ > 1 file changed, 35 insertions(+), 6 deletions(-) > > diff --git a/include/vdso/unaligned.h b/include/vdso/unaligned.h > index ff0c06b6513e..9076483c9fbb 100644 > --- a/include/vdso/unaligned.h > +++ b/include/vdso/unaligned.h > @@ -2,14 +2,43 @@ > #ifndef __VDSO_UNALIGNED_H > #define __VDSO_UNALIGNED_H > > -#define __get_unaligned_t(type, ptr) ({ \ > - const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr); \ > - __get_pptr->x; \ > +#include <linux/compiler_types.h> > + > +/** > + * __get_unaligned_t - read an unaligned value from memory. > + * @type: the type to load from the pointer. > + * @ptr: the pointer to load from. > + * > + * Use memcpy to affect an unaligned type sized load avoiding undefined behavior > + * from approaches like type punning that require -fno-strict-aliasing in order > + * to be correct. As type may be const, use __unqual_scalar_typeof to map to a > + * non-const type - you can't memcpy into a const type. The > + * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required > + * expression rather than type, a pointer is used to avoid warnings about mixing > + * the use of 0 and NULL. The void* cast silences ubsan warnings. > + */ > +#define __get_unaligned_t(type, ptr) ({ \ > + type *__get_unaligned_ctrl_type __always_unused = NULL; \ > + __unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \ > + __builtin_memcpy(&__get_unaligned_val, (void *)(ptr), \ > + sizeof(__get_unaligned_val)); \ > + __get_unaligned_val; \ > }) > > -#define __put_unaligned_t(type, val, ptr) do { \ > - struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr); \ > - __put_pptr->x = (val); \ > +/** > + * __put_unaligned_t - write an unaligned value to memory. > + * @type: the type of the value to store. > + * @val: the value to store. > + * @ptr: the pointer to store to. > + * > + * Use memcpy to affect an unaligned type sized store avoiding undefined > + * behavior from approaches like type punning that require -fno-strict-aliasing > + * in order to be correct. The void* cast silences ubsan warnings. > + */ > +#define __put_unaligned_t(type, val, ptr) do { \ > + type __put_unaligned_val = (val); \ > + __builtin_memcpy((void *)(ptr), &__put_unaligned_val, \ > + sizeof(__put_unaligned_val)); \ > } while (0) > > #endif /* __VDSO_UNALIGNED_H */ commit a339671db64b ("vdso: Switch get/put_unaligned() from packed struct to memcpy()"), which landed in 7.0, causes a performance regression on an NXP i.MX25 (ARMv5TE) SoC. I found it while updating a client's board from 6.12 to 7.0. A fio 4k randwrite benchmark on a NAND storage with UBI and UBIFS filesystem was the only workload that showed a clear regression between those two versions, so I bisected with it: perf stat -e irq:irq_handler_entry --filter 'irq == 49' -a \ -- \ fio --name=rw \ --filename=/var/stat/testfile \ --size=8M \ --rw=randwrite \ --bs=4k \ --direct=0 \ --fsync=1 \ --numjobs=4 \ --group_reporting | kernel | irq_handler_entry | fio bw | | ------ | ----------------- | -------- | | 6.12 | 155783 | 401KiB/s | | 6.13 | 162317 | 395KiB/s | | 6.14 | 168741 | 392KiB/s | | 6.15 | 168556 | 401KiB/s | | 6.16 | 166090 | 403KiB/s | | 6.17 | 162541 | 385KiB/s | | 6.18 | 157527 | 386KiB/s | | 6.19 | 183675 | 381KiB/s | | 7.0 | 190372 | 297KiB/s | The bisect targeted the large drop between 6.19 and 7.0; the smaller 6.17 regression predates this commit and is unrelated. Reverting a339671db64b restores throughput to the 6.17 level (~385 KiB/s). The commit is still present in 7.3-rc5, and the same codegen problem reproduces there. Digging deeper, I built 7.3-rc5 with my config and GCC 16.2, with and without the commit, and compared the object files: 114 of them differ. As <vdso/unaligned.h> is included by <linux/unaligned.h>, every get/put_unaligned() call site depends on it transitively. GCC did not inline __builtin_memcpy() and turned it into a function call, e.g. in crypto/crc32c.c (__chksum_finup(), inlined into chksum_digest()): Without the commit: <chksum_digest>: str lr, [sp, #-0x4]! sub sp, sp, #12 str lr, [sp, #-0x4]! bl 0xc0 <chksum_digest+0xc> @ imm = #-0x8 R_ARM_CALL __gnu_mcount_nc ldr r0, [r0] str r3, [sp, #0x4] ldr r0, [r0, #0x20] bl 0xd0 <chksum_digest+0x1c> @ imm = #-0x8 R_ARM_CALL crc32c mvn r2, r0 mov r0, #0 ldr r3, [sp, #0x4] lsr r12, r2, #8 lsr r1, r2, #16 strb r2, [r3] lsr r2, r2, #24 strb r12, [r3, #0x1] strb r1, [r3, #0x2] strb r2, [r3, #0x3] add sp, sp, #12 ldr pc, [sp], #4 With the commit: <chksum_digest>: push {r4, lr} sub sp, sp, #8 str lr, [sp, #-0x4]! bl 0x124 <chksum_digest+0xc> @ imm = #-0x8 R_ARM_CALL __gnu_mcount_nc ldr r0, [r0] ldr r12, [pc, #0x54] @ 0x188 <chksum_digest+0x70> ldr r0, [r0, #0x20] mov r4, r3 ldr r12, [r12] str r12, [sp, #0x4] mov r12, #0 bl 0x144 <chksum_digest+0x2c> @ imm = #-0x8 R_ARM_CALL crc32c mvn r3, r0 mov r2, #4 mov r0, r4 mov r1, sp str r3, [sp] bl 0x15c <chksum_digest+0x44> @ imm = #-0x8 R_ARM_CALL memcpy ldr r3, [pc, #0x20] @ 0x188 <chksum_digest+0x70> ldr r2, [r3] ldr r3, [sp, #0x4] eors r2, r3, r2 mov r3, #0 bne 0x184 <chksum_digest+0x6c> @ imm = #0x8 mov r0, #0 add sp, sp, #8 pop {r4, pc} bl 0x184 <chksum_digest+0x6c> @ imm = #-0x8 R_ARM_CALL __stack_chk_fail 188: 00 00 00 00 .word 0x00000000 R_ARM_ABS32 __stack_chk_guard 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. Best regards, Stefan -- Pengutronix e.K. | Stefan Kerkmann | Steuerwalder Str. 21 | https://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-128 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 | ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-09-28 15:39 ` [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Stefan Kerkmann @ 2026-09-28 15:58 ` Ian Rogers 2026-10-06 12:31 ` Marc Kleine-Budde 1 sibling, 0 replies; 36+ messages in thread From: Ian Rogers @ 2026-09-28 15:58 UTC (permalink / raw) To: Stefan Kerkmann Cc: James E.J. Bottomley, Helge Deller, Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld On Mon, Sep 28, 2026 at 8:39 AM Stefan Kerkmann <s.kerkmann@pengutronix.de> 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> > > --- > > include/vdso/unaligned.h | 41 ++++++++++++++++++++++++++++++++++------ > > 1 file changed, 35 insertions(+), 6 deletions(-) > > > > diff --git a/include/vdso/unaligned.h b/include/vdso/unaligned.h > > index ff0c06b6513e..9076483c9fbb 100644 > > --- a/include/vdso/unaligned.h > > +++ b/include/vdso/unaligned.h > > @@ -2,14 +2,43 @@ > > #ifndef __VDSO_UNALIGNED_H > > #define __VDSO_UNALIGNED_H > > > > -#define __get_unaligned_t(type, ptr) ({ \ > > - const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr); \ > > - __get_pptr->x; \ > > +#include <linux/compiler_types.h> > > + > > +/** > > + * __get_unaligned_t - read an unaligned value from memory. > > + * @type: the type to load from the pointer. > > + * @ptr: the pointer to load from. > > + * > > + * Use memcpy to affect an unaligned type sized load avoiding undefined behavior > > + * from approaches like type punning that require -fno-strict-aliasing in order > > + * to be correct. As type may be const, use __unqual_scalar_typeof to map to a > > + * non-const type - you can't memcpy into a const type. The > > + * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required > > + * expression rather than type, a pointer is used to avoid warnings about mixing > > + * the use of 0 and NULL. The void* cast silences ubsan warnings. > > + */ > > +#define __get_unaligned_t(type, ptr) ({ \ > > + type *__get_unaligned_ctrl_type __always_unused = NULL; \ > > + __unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \ > > + __builtin_memcpy(&__get_unaligned_val, (void *)(ptr), \ > > + sizeof(__get_unaligned_val)); \ > > + __get_unaligned_val; \ > > }) > > > > -#define __put_unaligned_t(type, val, ptr) do { \ > > - struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr); \ > > - __put_pptr->x = (val); \ > > +/** > > + * __put_unaligned_t - write an unaligned value to memory. > > + * @type: the type of the value to store. > > + * @val: the value to store. > > + * @ptr: the pointer to store to. > > + * > > + * Use memcpy to affect an unaligned type sized store avoiding undefined > > + * behavior from approaches like type punning that require -fno-strict-aliasing > > + * in order to be correct. The void* cast silences ubsan warnings. > > + */ > > +#define __put_unaligned_t(type, val, ptr) do { \ > > + type __put_unaligned_val = (val); \ > > + __builtin_memcpy((void *)(ptr), &__put_unaligned_val, \ > > + sizeof(__put_unaligned_val)); \ > > } while (0) > > > > #endif /* __VDSO_UNALIGNED_H */ > > commit a339671db64b ("vdso: Switch get/put_unaligned() from packed struct to > memcpy()"), which landed in 7.0, causes a performance regression on an NXP > i.MX25 (ARMv5TE) SoC. > > I found it while updating a client's board from 6.12 to 7.0. A fio 4k randwrite > benchmark on a NAND storage with UBI and UBIFS filesystem was the only workload > that showed a clear regression between those two versions, so I bisected with > it: > > perf stat -e irq:irq_handler_entry --filter 'irq == 49' -a \ > -- \ > fio --name=rw \ > --filename=/var/stat/testfile \ > --size=8M \ > --rw=randwrite \ > --bs=4k \ > --direct=0 \ > --fsync=1 \ > --numjobs=4 \ > --group_reporting > > | kernel | irq_handler_entry | fio bw | > | ------ | ----------------- | -------- | > | 6.12 | 155783 | 401KiB/s | > | 6.13 | 162317 | 395KiB/s | > | 6.14 | 168741 | 392KiB/s | > | 6.15 | 168556 | 401KiB/s | > | 6.16 | 166090 | 403KiB/s | > | 6.17 | 162541 | 385KiB/s | > | 6.18 | 157527 | 386KiB/s | > | 6.19 | 183675 | 381KiB/s | > | 7.0 | 190372 | 297KiB/s | > > The bisect targeted the large drop between 6.19 and 7.0; the smaller 6.17 > regression predates this commit and is unrelated. Reverting a339671db64b > restores throughput to the 6.17 level (~385 KiB/s). The commit is still > present in 7.3-rc5, and the same codegen problem reproduces there. > > Digging deeper, I built 7.3-rc5 with my config and GCC 16.2, with and without > the commit, and compared the object files: 114 of them differ. As > <vdso/unaligned.h> is included by <linux/unaligned.h>, every > get/put_unaligned() call site depends on it transitively. GCC did not inline > __builtin_memcpy() and turned it into a function call, e.g. in crypto/crc32c.c > (__chksum_finup(), inlined into chksum_digest()): > > Without the commit: > > <chksum_digest>: > str lr, [sp, #-0x4]! > sub sp, sp, #12 > str lr, [sp, #-0x4]! > bl 0xc0 <chksum_digest+0xc> @ imm = #-0x8 > R_ARM_CALL __gnu_mcount_nc > ldr r0, [r0] > str r3, [sp, #0x4] > ldr r0, [r0, #0x20] > bl 0xd0 <chksum_digest+0x1c> @ imm = #-0x8 > R_ARM_CALL crc32c > mvn r2, r0 > mov r0, #0 > ldr r3, [sp, #0x4] > lsr r12, r2, #8 > lsr r1, r2, #16 > strb r2, [r3] > lsr r2, r2, #24 > strb r12, [r3, #0x1] > strb r1, [r3, #0x2] > strb r2, [r3, #0x3] > add sp, sp, #12 > ldr pc, [sp], #4 > > With the commit: > > <chksum_digest>: > push {r4, lr} > sub sp, sp, #8 > str lr, [sp, #-0x4]! > bl 0x124 <chksum_digest+0xc> @ imm = #-0x8 > R_ARM_CALL __gnu_mcount_nc > ldr r0, [r0] > ldr r12, [pc, #0x54] @ 0x188 <chksum_digest+0x70> > ldr r0, [r0, #0x20] > mov r4, r3 > ldr r12, [r12] > str r12, [sp, #0x4] > mov r12, #0 > bl 0x144 <chksum_digest+0x2c> @ imm = #-0x8 > R_ARM_CALL crc32c > mvn r3, r0 > mov r2, #4 > mov r0, r4 > mov r1, sp > str r3, [sp] > bl 0x15c <chksum_digest+0x44> @ imm = #-0x8 > R_ARM_CALL memcpy > ldr r3, [pc, #0x20] @ 0x188 <chksum_digest+0x70> > ldr r2, [r3] > ldr r3, [sp, #0x4] > eors r2, r3, r2 > mov r3, #0 > bne 0x184 <chksum_digest+0x6c> @ imm = #0x8 > mov r0, #0 > add sp, sp, #8 > pop {r4, pc} > bl 0x184 <chksum_digest+0x6c> @ imm = #-0x8 > R_ARM_CALL __stack_chk_fail > 188: 00 00 00 00 .word 0x00000000 > R_ARM_ABS32 __stack_chk_guard > > 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. Hi Stefan, and sorry for the performance regression! I largely work on the perf tool which is in the tools/ directory but uses kernel header files, such as for unaligned accesses. My work was motivated by trying to avoid using -fno-strict-aliasing in the perf tool. As the compiler is given __builtin_memcpy of a fixed memory size then the lowering should match that of using the packed struct. Are there compiler flags you are using that disable compiler optimizations? This feels like a compiler bug, a workaround is to use the packed struct helpers in linux/unaligned/packed_struct.h Thanks, Ian > Best regards, > Stefan > > -- > Pengutronix e.K. | Stefan Kerkmann | > Steuerwalder Str. 21 | https://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-128 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 | > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-09-28 15:39 ` [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Stefan Kerkmann 2026-09-28 15:58 ` Ian Rogers @ 2026-10-06 12:31 ` Marc Kleine-Budde 2026-10-07 10:59 ` Arnd Bergmann 1 sibling, 1 reply; 36+ messages in thread From: Marc Kleine-Budde @ 2026-10-06 12:31 UTC (permalink / raw) To: Stefan Kerkmann, arnd Cc: Ian Rogers, James E.J. Bottomley, Helge Deller, Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld [-- Attachment #1: Type: text/plain, Size: 8561 bytes --] Cc+ Arnd 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> > > --- > > include/vdso/unaligned.h | 41 ++++++++++++++++++++++++++++++++++------ > > 1 file changed, 35 insertions(+), 6 deletions(-) > > > > diff --git a/include/vdso/unaligned.h b/include/vdso/unaligned.h > > index ff0c06b6513e..9076483c9fbb 100644 > > --- a/include/vdso/unaligned.h > > +++ b/include/vdso/unaligned.h > > @@ -2,14 +2,43 @@ > > #ifndef __VDSO_UNALIGNED_H > > #define __VDSO_UNALIGNED_H > > > > -#define __get_unaligned_t(type, ptr) ({ \ > > - const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr); \ > > - __get_pptr->x; \ > > +#include <linux/compiler_types.h> > > + > > +/** > > + * __get_unaligned_t - read an unaligned value from memory. > > + * @type: the type to load from the pointer. > > + * @ptr: the pointer to load from. > > + * > > + * Use memcpy to affect an unaligned type sized load avoiding undefined behavior > > + * from approaches like type punning that require -fno-strict-aliasing in order > > + * to be correct. As type may be const, use __unqual_scalar_typeof to map to a > > + * non-const type - you can't memcpy into a const type. The > > + * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required > > + * expression rather than type, a pointer is used to avoid warnings about mixing > > + * the use of 0 and NULL. The void* cast silences ubsan warnings. > > + */ > > +#define __get_unaligned_t(type, ptr) ({ \ > > + type *__get_unaligned_ctrl_type __always_unused = NULL; \ > > + __unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \ > > + __builtin_memcpy(&__get_unaligned_val, (void *)(ptr), \ > > + sizeof(__get_unaligned_val)); \ > > + __get_unaligned_val; \ > > }) > > > > -#define __put_unaligned_t(type, val, ptr) do { \ > > - struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr); \ > > - __put_pptr->x = (val); \ > > +/** > > + * __put_unaligned_t - write an unaligned value to memory. > > + * @type: the type of the value to store. > > + * @val: the value to store. > > + * @ptr: the pointer to store to. > > + * > > + * Use memcpy to affect an unaligned type sized store avoiding undefined > > + * behavior from approaches like type punning that require -fno-strict-aliasing > > + * in order to be correct. The void* cast silences ubsan warnings. > > + */ > > +#define __put_unaligned_t(type, val, ptr) do { \ > > + type __put_unaligned_val = (val); \ > > + __builtin_memcpy((void *)(ptr), &__put_unaligned_val, \ > > + sizeof(__put_unaligned_val)); \ > > } while (0) > > > > #endif /* __VDSO_UNALIGNED_H */ > > commit a339671db64b ("vdso: Switch get/put_unaligned() from packed struct to > memcpy()"), which landed in 7.0, causes a performance regression on an NXP > i.MX25 (ARMv5TE) SoC. > > I found it while updating a client's board from 6.12 to 7.0. A fio 4k randwrite > benchmark on a NAND storage with UBI and UBIFS filesystem was the only workload > that showed a clear regression between those two versions, so I bisected with > it: > > perf stat -e irq:irq_handler_entry --filter 'irq == 49' -a \ > -- \ > fio --name=rw \ > --filename=/var/stat/testfile \ > --size=8M \ > --rw=randwrite \ > --bs=4k \ > --direct=0 \ > --fsync=1 \ > --numjobs=4 \ > --group_reporting > > | kernel | irq_handler_entry | fio bw | > | ------ | ----------------- | -------- | > | 6.12 | 155783 | 401KiB/s | > | 6.13 | 162317 | 395KiB/s | > | 6.14 | 168741 | 392KiB/s | > | 6.15 | 168556 | 401KiB/s | > | 6.16 | 166090 | 403KiB/s | > | 6.17 | 162541 | 385KiB/s | > | 6.18 | 157527 | 386KiB/s | > | 6.19 | 183675 | 381KiB/s | > | 7.0 | 190372 | 297KiB/s | > > The bisect targeted the large drop between 6.19 and 7.0; the smaller 6.17 > regression predates this commit and is unrelated. Reverting a339671db64b > restores throughput to the 6.17 level (~385 KiB/s). The commit is still > present in 7.3-rc5, and the same codegen problem reproduces there. > > Digging deeper, I built 7.3-rc5 with my config and GCC 16.2, with and without > the commit, and compared the object files: 114 of them differ. As > <vdso/unaligned.h> is included by <linux/unaligned.h>, every > get/put_unaligned() call site depends on it transitively. GCC did not inline > __builtin_memcpy() and turned it into a function call, e.g. in crypto/crc32c.c > (__chksum_finup(), inlined into chksum_digest()): > > Without the commit: > > <chksum_digest>: > str lr, [sp, #-0x4]! > sub sp, sp, #12 > str lr, [sp, #-0x4]! > bl 0xc0 <chksum_digest+0xc> @ imm = #-0x8 > R_ARM_CALL __gnu_mcount_nc > ldr r0, [r0] > str r3, [sp, #0x4] > ldr r0, [r0, #0x20] > bl 0xd0 <chksum_digest+0x1c> @ imm = #-0x8 > R_ARM_CALL crc32c > mvn r2, r0 > mov r0, #0 > ldr r3, [sp, #0x4] > lsr r12, r2, #8 > lsr r1, r2, #16 > strb r2, [r3] > lsr r2, r2, #24 > strb r12, [r3, #0x1] > strb r1, [r3, #0x2] > strb r2, [r3, #0x3] > add sp, sp, #12 > ldr pc, [sp], #4 > > With the commit: > > <chksum_digest>: > push {r4, lr} > sub sp, sp, #8 > str lr, [sp, #-0x4]! > bl 0x124 <chksum_digest+0xc> @ imm = #-0x8 > R_ARM_CALL __gnu_mcount_nc > ldr r0, [r0] > ldr r12, [pc, #0x54] @ 0x188 <chksum_digest+0x70> > ldr r0, [r0, #0x20] > mov r4, r3 > ldr r12, [r12] > str r12, [sp, #0x4] > mov r12, #0 > bl 0x144 <chksum_digest+0x2c> @ imm = #-0x8 > R_ARM_CALL crc32c > mvn r3, r0 > mov r2, #4 > mov r0, r4 > mov r1, sp > str r3, [sp] > bl 0x15c <chksum_digest+0x44> @ imm = #-0x8 > R_ARM_CALL memcpy > ldr r3, [pc, #0x20] @ 0x188 <chksum_digest+0x70> > ldr r2, [r3] > ldr r3, [sp, #0x4] > eors r2, r3, r2 > mov r3, #0 > bne 0x184 <chksum_digest+0x6c> @ imm = #0x8 > mov r0, #0 > add sp, sp, #8 > pop {r4, pc} > bl 0x184 <chksum_digest+0x6c> @ imm = #-0x8 > R_ARM_CALL __stack_chk_fail > 188: 00 00 00 00 .word 0x00000000 > R_ARM_ABS32 __stack_chk_guard > > 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. > > Best regards, > Stefan > > -- > Pengutronix e.K. | Stefan Kerkmann | > Steuerwalder Str. 21 | https://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-128 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 | > > -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung Nürnberg | Phone: +49-5121-206917-129 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-06 12:31 ` Marc Kleine-Budde @ 2026-10-07 10:59 ` Arnd Bergmann 2026-10-09 21:28 ` Ian Rogers 0 siblings, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2026-10-07 10:59 UTC (permalink / raw) To: Marc Kleine-Budde, Stefan Kerkmann Cc: Ian Rogers, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld 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. Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-07 10:59 ` Arnd Bergmann @ 2026-10-09 21:28 ` Ian Rogers 2026-10-10 10:26 ` Christophe Leroy (CS GROUP) 2026-10-10 14:06 ` Arnd Bergmann 0 siblings, 2 replies; 36+ messages in thread From: Ian Rogers @ 2026-10-09 21:28 UTC (permalink / raw) To: Arnd Bergmann, Thomas Gleixner Cc: Marc Kleine-Budde, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld 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! Am i going to be able to convince people builtin_memcpy is a better unaligned choice than a packed struct? Well memcpy is the expected C way to solve this problem, and C compilers like clang implicitly emit memcpy intrinsics when copying things like aggregate values. The packed struct requires a cast to a pointer type violating strict aliasing rules, but has been sound for many years because of -fno-strict-aliasing. We'd like the perf tool to be clean for things like undefined behavior sanitizers, but separate userland code could support this. I was trying to be as useful as possible and I don't think the patches were rushed at any point. What the bug report shows is that there is a lowering problem for memcpy with GCC on ARMv5, presumably as ARMv5 lacks unaligned memory operations. It seems the packed code shows how we can teach the faster unaligned lowering to GCC for ARMv5. Fixing the lowering issue in GCC will likely win performance improvements elsewhere on code compiled for ARMv5, as optimal memcpy codegen is an expectation for things like copying an aggregate value. Fixing GCC would be best but a workaround is to use the packed unaligned functions: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/unaligned/packed_struct.h I guess this could be a pain if there are regressions elsewhere in the kernel that also need fixing. I believe I spoke to Stefan at LPC on Tuesday and explained all of this, suggesting the packed unaligned functions as a workaround. It would be interesting to hear of other motivations for memcpy that Thomas may know. Perhaps some config value defaulted to memcpy and switching to packed structs works for everyone. Maybe separating the user and kernel code makes most sense. Thanks, Ian > Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-09 21:28 ` Ian Rogers @ 2026-10-10 10:26 ` Christophe Leroy (CS GROUP) 2026-10-10 12:20 ` Ian Rogers 2026-10-10 14:06 ` Arnd Bergmann 1 sibling, 1 reply; 36+ messages in thread From: Christophe Leroy (CS GROUP) @ 2026-10-10 10:26 UTC (permalink / raw) To: Ian Rogers, Arnd Bergmann, Thomas Gleixner Cc: Marc Kleine-Budde, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Jason A . Donenfeld 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-10 10:26 ` Christophe Leroy (CS GROUP) @ 2026-10-10 12:20 ` Ian Rogers 0 siblings, 0 replies; 36+ messages in thread From: Ian Rogers @ 2026-10-10 12:20 UTC (permalink / raw) To: Christophe Leroy (CS GROUP) Cc: Arnd Bergmann, Thomas Gleixner, Marc Kleine-Budde, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Jason A . Donenfeld On Sat, Oct 10, 2026 at 3:26 AM Christophe Leroy (CS GROUP) <chleroy@kernel.org> wrote: > > 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. 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@mail.gmail.com/ PA-RISC workaround: https://lore.kernel.org/lkml/20251016205126.2882625-2-irogers@google.com/ MIPS failure with suggested workaround: https://lore.kernel.org/lkml/CAP-5=fVBZPBz6J1omfvSS4JLkueRGqCdouLji6gsFLTeu9nJTw@mail.gmail.com/ 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-09 21:28 ` Ian Rogers 2026-10-10 10:26 ` Christophe Leroy (CS GROUP) @ 2026-10-10 14:06 ` Arnd Bergmann 2026-10-10 15:04 ` Ian Rogers 1 sibling, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2026-10-10 14:06 UTC (permalink / raw) To: Ian Rogers, Thomas Gleixner Cc: Marc Kleine-Budde, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Fri, Oct 9, 2026, at 23:28, Ian Rogers wrote: > 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: >> > On 28.09.2026 17:39:15, Stefan Kerkmann wrote: >> 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! Ok, thanks for explaining the background. > Am i going to be able to convince people builtin_memcpy is a better > unaligned choice than a packed struct? Well memcpy is the expected C > way to solve this problem, and C compilers like clang implicitly emit > memcpy intrinsics when copying things like aggregate values. The > packed struct requires a cast to a pointer type violating strict > aliasing rules, but has been sound for many years because of > -fno-strict-aliasing. We'd like the perf tool to be clean for things > like undefined behavior sanitizers, but separate userland code could > support this. I was trying to be as useful as possible and I don't > think the patches were rushed at any point. Aside from the performance regression, I see more issues with using __builtin_memcpy() here: - since the compiler is allowed to always turn __builtin_memcpy() into an extern memcpy() call, it looks invalid to use this in the vdso, which is not allowed to call any functions. - the original version used memmove() instead of memcpy(). While this was removed in bf067edf5d2f ("openrisc: always use unaligned-struct header"), this was surely intentional at the time. - the use of __unqual_scalar_typeof() turns a relatively simple expression into a much larger amount of preprocessed code, which tends to confuse the inlining choices and compile speed, especially when this is mixed with other macros that expand the arguments multiple times. > What the bug report shows is that there is a lowering problem for > memcpy with GCC on ARMv5, presumably as ARMv5 lacks unaligned memory > operations. It seems the packed code shows how we can teach the faster > unaligned lowering to GCC for ARMv5. Fixing the lowering issue in GCC > will likely win performance improvements elsewhere on code compiled > for ARMv5, as optimal memcpy codegen is an expectation for things like > copying an aggregate value. As far as I can tell, this only happens when building with -Os, and I don't even think the decision to call the external memcpy() is necessarily wrong here. The same thing happens on mips32, mips64, openrisc, riscv32, riscv64, sh4, sparc32, and sparc64. Some of these also use an out-of-line bswap32 in put_unaligned_be32() when building with -Os. > Fixing GCC would be best but a workaround is to use the packed > unaligned functions: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/unaligned/packed_struct.h > I guess this could be a pain if there are regressions elsewhere in the > kernel that also need fixing. I hadn't realized that we still have the extra copy for those, my guess is that we had planned to remove that but somehow never converted the last remaining user in tools/include/linux/jhash.h after commit 50b4233a22b1 ("include/linux/jhash.h: replace __get_unaligned_cpu32 in jhash function") did the second-to-last. > I believe I spoke to Stefan at LPC on Tuesday and explained all of > this, suggesting the packed unaligned functions as a workaround. It > would be interesting to hear of other motivations for memcpy that > Thomas may know. Perhaps some config value defaulted to memcpy and > switching to packed structs works for everyone. Maybe separating the > user and kernel code makes most sense. Right, that seems easy enough to do: the include/vdso/ headers are not meant for user consumption in the first place, so it would make sense to use the struct variant there, and the include/linux/unaligned/packed_struct.h version could provide the memcpy() based code for tools/ and get deleted from the kernel internal version. It's just complicated a bit by the fact that the current code does the exact oppposite ;-) Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-10 14:06 ` Arnd Bergmann @ 2026-10-10 15:04 ` Ian Rogers 2026-10-19 12:37 ` Marc Kleine-Budde 0 siblings, 1 reply; 36+ messages in thread From: Ian Rogers @ 2026-10-10 15:04 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Marc Kleine-Budde, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Sat, Oct 10, 2026 at 7:06 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Fri, Oct 9, 2026, at 23:28, Ian Rogers wrote: > > 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: > >> > On 28.09.2026 17:39:15, Stefan Kerkmann wrote: > >> 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! > > Ok, thanks for explaining the background. > > > Am i going to be able to convince people builtin_memcpy is a better > > unaligned choice than a packed struct? Well memcpy is the expected C > > way to solve this problem, and C compilers like clang implicitly emit > > memcpy intrinsics when copying things like aggregate values. The > > packed struct requires a cast to a pointer type violating strict > > aliasing rules, but has been sound for many years because of > > -fno-strict-aliasing. We'd like the perf tool to be clean for things > > like undefined behavior sanitizers, but separate userland code could > > support this. I was trying to be as useful as possible and I don't > > think the patches were rushed at any point. > > Aside from the performance regression, I see more issues with > using __builtin_memcpy() here: > > - since the compiler is allowed to always turn __builtin_memcpy() > into an extern memcpy() call, it looks invalid to use this > in the vdso, which is not allowed to call any functions. > > - the original version used memmove() instead of memcpy(). While > this was removed in bf067edf5d2f ("openrisc: always use > unaligned-struct header"), this was surely intentional at the time. > > - the use of __unqual_scalar_typeof() turns a relatively simple > expression into a much larger amount of preprocessed code, which > tends to confuse the inlining choices and compile speed, especially > when this is mixed with other macros that expand the arguments > multiple times. > > > What the bug report shows is that there is a lowering problem for > > memcpy with GCC on ARMv5, presumably as ARMv5 lacks unaligned memory > > operations. It seems the packed code shows how we can teach the faster > > unaligned lowering to GCC for ARMv5. Fixing the lowering issue in GCC > > will likely win performance improvements elsewhere on code compiled > > for ARMv5, as optimal memcpy codegen is an expectation for things like > > copying an aggregate value. > > As far as I can tell, this only happens when building with -Os, > and I don't even think the decision to call the external memcpy() > is necessarily wrong here. The same thing happens on mips32, mips64, > openrisc, riscv32, riscv64, sh4, sparc32, and sparc64. Some of these > also use an out-of-line bswap32 in put_unaligned_be32() when building > with -Os. > > > Fixing GCC would be best but a workaround is to use the packed > > unaligned functions: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/unaligned/packed_struct.h > > I guess this could be a pain if there are regressions elsewhere in the > > kernel that also need fixing. > > I hadn't realized that we still have the extra copy for those, > my guess is that we had planned to remove that but somehow never > converted the last remaining user in tools/include/linux/jhash.h > after commit 50b4233a22b1 ("include/linux/jhash.h: replace > __get_unaligned_cpu32 in jhash function") did the second-to-last. > > > I believe I spoke to Stefan at LPC on Tuesday and explained all of > > this, suggesting the packed unaligned functions as a workaround. It > > would be interesting to hear of other motivations for memcpy that > > Thomas may know. Perhaps some config value defaulted to memcpy and > > switching to packed structs works for everyone. Maybe separating the > > user and kernel code makes most sense. > > Right, that seems easy enough to do: the include/vdso/ headers > are not meant for user consumption in the first place, so it would > make sense to use the struct variant there, and the > include/linux/unaligned/packed_struct.h version could provide the > memcpy() based code for tools/ and get deleted from the kernel > internal version. > > It's just complicated a bit by the fact that the current code does > the exact oppposite ;-) So completely fwiw, I speak to the compiler folks within Google and there has always been unanimous support for code using memcpy over type punning. I'm not sure where the inlining decisions would be impacted, as to my understanding those are made later in the compiler pipeline and things like __unqual_scalar_typeof are almost at the stage of the preprocessor. If -Os is not inlining small fixed size memcpys, this sounds like another compiler bug. It would also be completely valid for the compiler to lower packed struct accesses to memcpys. I was trying to reach an implementation that avoided undefined behavior and benefited both user and kernel code. Performance regressions aren't a benefit and fixing compilers can be tricky, so work remains. I'd prefer to avoid undefined behavior at least in user land. I'd also prefer to avoid config values, so using packed structs for __KERNEL__ code is a possibility. It seems a little sad that we've lived with a strict-aliasing-compliant implementation for a year, and we could regress that for the sake of ARMv5. I don't feel strongly enough to argue for kernel land; hopefully, Thomas can say what motivated moving the patch series forward as I'm pretty certain it wouldn't be undefined behavior in the perf tool :-) Thanks, Ian > Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-10 15:04 ` Ian Rogers @ 2026-10-19 12:37 ` Marc Kleine-Budde 2026-10-19 15:10 ` Ian Rogers 0 siblings, 1 reply; 36+ messages in thread From: Marc Kleine-Budde @ 2026-10-19 12:37 UTC (permalink / raw) To: Ian Rogers Cc: Arnd Bergmann, Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld [-- Attachment #1: Type: text/plain, Size: 1941 bytes --] On 10.10.2026 08:04:42, Ian Rogers wrote: [...] > So completely fwiw, I speak to the compiler folks within Google and > there has always been unanimous support for code using memcpy over > type punning. I'm not sure where the inlining decisions would be > impacted, as to my understanding those are made later in the compiler > pipeline and things like __unqual_scalar_typeof are almost at the > stage of the preprocessor. If -Os is not inlining small fixed size > memcpys, this sounds like another compiler bug. It would also be > completely valid for the compiler to lower packed struct accesses to > memcpys. > > I was trying to reach an implementation that avoided undefined > behavior and benefited both user and kernel code. Performance > regressions aren't a benefit and fixing compilers can be tricky, so > work remains. I'd prefer to avoid undefined behavior at least in user > land. I'd also prefer to avoid config values, so using packed structs > for __KERNEL__ code is a possibility. It seems a little sad that we've > lived with a strict-aliasing-compliant implementation for a year, and > we could regress that for the sake of ARMv5. I don't feel strongly > enough to argue for kernel land; hopefully, Thomas can say what > motivated moving the patch series forward as I'm pretty certain it > wouldn't be undefined behavior in the perf tool :-) Can we revert commit a339671db64b ("vdso: Switch get/put_unaligned() from packed struct to memcpy()")? It touches "include/vdso/unaligned.h" only, so the perf header will be unaffected. Or is there an automagic copying to "tools/include/vdso/unaligned.h" happening? regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung Nürnberg | Phone: +49-5121-206917-129 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-19 12:37 ` Marc Kleine-Budde @ 2026-10-19 15:10 ` Ian Rogers 2026-10-19 16:27 ` Arnd Bergmann 0 siblings, 1 reply; 36+ messages in thread From: Ian Rogers @ 2026-10-19 15:10 UTC (permalink / raw) To: Marc Kleine-Budde Cc: Arnd Bergmann, Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Mon, Oct 19, 2026 at 5:37 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote: > > On 10.10.2026 08:04:42, Ian Rogers wrote: > [...] > > So completely fwiw, I speak to the compiler folks within Google and > > there has always been unanimous support for code using memcpy over > > type punning. I'm not sure where the inlining decisions would be > > impacted, as to my understanding those are made later in the compiler > > pipeline and things like __unqual_scalar_typeof are almost at the > > stage of the preprocessor. If -Os is not inlining small fixed size > > memcpys, this sounds like another compiler bug. It would also be > > completely valid for the compiler to lower packed struct accesses to > > memcpys. > > > > I was trying to reach an implementation that avoided undefined > > behavior and benefited both user and kernel code. Performance > > regressions aren't a benefit and fixing compilers can be tricky, so > > work remains. I'd prefer to avoid undefined behavior at least in user > > land. I'd also prefer to avoid config values, so using packed structs > > for __KERNEL__ code is a possibility. It seems a little sad that we've > > lived with a strict-aliasing-compliant implementation for a year, and > > we could regress that for the sake of ARMv5. I don't feel strongly > > enough to argue for kernel land; hopefully, Thomas can say what > > motivated moving the patch series forward as I'm pretty certain it > > wouldn't be undefined behavior in the perf tool :-) > > Can we revert commit a339671db64b ("vdso: Switch get/put_unaligned() > from packed struct to memcpy()")? It touches "include/vdso/unaligned.h" > only, so the perf header will be unaffected. Or is there an automagic > copying to "tools/include/vdso/unaligned.h" happening? It is a little more manual than automagic :-) When perf builds there is a script that complains if the tools' version of files is out-of-sync with the kernel's: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/check-headers.sh#n28 It doesn't break the perf build, it just adds noise to it. My understanding, biased by my opinions, is that the preferred option is to add a config variable controlling whether to use memcpy or a packed struct. My preference, to avoid undefined behavior, is to default to memcpy but set it to use packed structs for ARMv5. I will vibe code this up, and apologize in advance for any offense such a patch series may cause (ie I'm sure someone on LKML will want to complain at me for using memcpy at all and give me a lecture about strict aliasing being nonsense, etc.). Arnd or someone else may have a different preferred solution, to which I'm more than happy to defer. Thanks and apologies again for your ongoing problems, Ian > regards, > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Embedded Linux | https://www.pengutronix.de | > Vertretung Nürnberg | Phone: +49-5121-206917-129 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 | ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-19 15:10 ` Ian Rogers @ 2026-10-19 16:27 ` Arnd Bergmann 2026-10-19 17:04 ` Ian Rogers 2026-10-20 8:36 ` David Laight 0 siblings, 2 replies; 36+ messages in thread From: Arnd Bergmann @ 2026-10-19 16:27 UTC (permalink / raw) To: Ian Rogers, Marc Kleine-Budde Cc: Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Mon, Oct 19, 2026, at 17:10, Ian Rogers wrote: > On Mon, Oct 19, 2026 at 5:37 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote: >> On 10.10.2026 08:04:42, Ian Rogers wrote: > > It is a little more manual than automagic :-) When perf builds there > is a script that complains if the tools' version of files is > out-of-sync with the kernel's: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/check-headers.sh#n28 > It doesn't break the perf build, it just adds noise to it. I see that the include/linux/unaligned.h and tools/include/linux/unaligned.h headers have not been in sync for a long time, so maybe it's just not a big deal there. > My understanding, biased by my opinions, is that the preferred option > is to add a config variable controlling whether to use memcpy or a > packed struct. My preference, to avoid undefined behavior, is to > default to memcpy but set it to use packed structs for ARMv5. I will > vibe code this up, and apologize in advance for any offense such a > patch series may cause (ie I'm sure someone on LKML will want to > complain at me for using memcpy at all and give me a lecture about > strict aliasing being nonsense, etc.). Arnd or someone else may have a > different preferred solution, to which I'm more than happy to defer. As I wrote, we need the packed struct version at least on mips32, mips64, openrisc, riscv32, riscv64, sh4, sparc32, and sparc64 to avoid the external library calls the same as on armv5, but I would prefer to just always use that in the kernel. My guess is that this list is actually the same as the architectures that lack unaligned word access instructions in hardware, and armv5 is certainly not a special case. For 7.4 (and stable kernels) reverting the in-kernel header seems best to me, and then we can think about what to use for the tools. Looking at the uses of get_unaligned() in the tools directory, I notice that at least some of them should just not exist, e.g. struct debug_entry in tools/perf/util/jitdump.h is just needs a __packed attribute to correctly describe what it does, and tools/testing/cxl/test/mem.c does not need the unaligned helpers at all because the structs are all packed already. I'm also a bit suspicious of the usage in tools/testing/selftests/bpf/test_kmods/bpf_testmod.c, which looks like it may run into a corner case: __bpf_kfunc int bpf_kfunc_test_uninit_alias(int *out__uninit, const int *in) { int value = get_unaligned(in); put_unaligned(42, out__uninit); return value; } The current version of get_unaligned() turns into __builtin_memcpy(), and the function prototype defines both arguments as pointers to naturally aligned objects. Do you see anything that would stop the compiler from replacing the __builtin_memcpy() with an invalid pointer dereference? This bit is probably fine because of the way gcc handles __builtin_memcpy() optimization, but I'm not sure if that's an implementation detail or required by the C standard, especially if strict aliasing rules say that the int* argument cannot overlap a misaligned int*. Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-19 16:27 ` Arnd Bergmann @ 2026-10-19 17:04 ` Ian Rogers 2026-10-19 21:00 ` Arnd Bergmann 2026-10-20 8:36 ` David Laight 1 sibling, 1 reply; 36+ messages in thread From: Ian Rogers @ 2026-10-19 17:04 UTC (permalink / raw) To: Arnd Bergmann Cc: Marc Kleine-Budde, Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Mon, Oct 19, 2026 at 9:28 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Mon, Oct 19, 2026, at 17:10, Ian Rogers wrote: > > On Mon, Oct 19, 2026 at 5:37 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote: > >> On 10.10.2026 08:04:42, Ian Rogers wrote: > > > > It is a little more manual than automagic :-) When perf builds there > > is a script that complains if the tools' version of files is > > out-of-sync with the kernel's: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/check-headers.sh#n28 > > It doesn't break the perf build, it just adds noise to it. > > I see that the include/linux/unaligned.h and tools/include/linux/unaligned.h > headers have not been in sync for a long time, so maybe it's just not > a big deal there. > > > My understanding, biased by my opinions, is that the preferred option > > is to add a config variable controlling whether to use memcpy or a > > packed struct. My preference, to avoid undefined behavior, is to > > default to memcpy but set it to use packed structs for ARMv5. I will > > vibe code this up, and apologize in advance for any offense such a > > patch series may cause (ie I'm sure someone on LKML will want to > > complain at me for using memcpy at all and give me a lecture about > > strict aliasing being nonsense, etc.). Arnd or someone else may have a > > different preferred solution, to which I'm more than happy to defer. > > As I wrote, we need the packed struct version at least on mips32, > mips64, openrisc, riscv32, riscv64, sh4, sparc32, and sparc64 to avoid > the external library calls the same as on armv5, but I would prefer > to just always use that in the kernel. My guess is that this list > is actually the same as the architectures that lack unaligned word > access instructions in hardware, and armv5 is certainly not > a special case. Is it more than conjecture that these architectures have regressed? I have a hard time believing the codegen on say riscv64 is lagging aarch64. When the series originally landed the issue was looked at and workarounds were added, for example, to PA-RISC. > For 7.4 (and stable kernels) reverting the in-kernel header seems > best to me, and then we can think about what to use for the tools. > > Looking at the uses of get_unaligned() in the tools directory, > I notice that at least some of them should just not exist, e.g. > struct debug_entry in tools/perf/util/jitdump.h is just > needs a __packed attribute to correctly describe what it does, > and tools/testing/cxl/test/mem.c does not need the unaligned > helpers at all because the structs are all packed already. There was supposed to be an 8-byte alignment requirement for perf events and unfortunately some events broke this and we added __packed as a workaround. The unaligned accesses wouldn't trip a sanitizer in our tests because we sadly lack testing on old perf.data files. There is a difference in using __packed as an attribute and using type punning to achieve an unaligned access. > I'm also a bit suspicious of the usage in > tools/testing/selftests/bpf/test_kmods/bpf_testmod.c, which > looks like it may run into a corner case: > > __bpf_kfunc int bpf_kfunc_test_uninit_alias(int *out__uninit, const int *in) > { > int value = get_unaligned(in); > put_unaligned(42, out__uninit); > return value; > } > > The current version of get_unaligned() turns into __builtin_memcpy(), > and the function prototype defines both arguments as pointers > to naturally aligned objects. Do you see anything that would > stop the compiler from replacing the __builtin_memcpy() with > an invalid pointer dereference? This bit is probably fine > because of the way gcc handles __builtin_memcpy() optimization, > but I'm not sure if that's an implementation detail or required > by the C standard, especially if strict aliasing rules say > that the int* argument cannot overlap a misaligned int*. I agree the pointers should be aligned given their types, ubsan doesn't currently fail for this and waits for use of the pointer. So there is the specification and also what sanitizers enforce. For clang, when lowering from parsed C to machine code the compiler will use alloca to model local variables and memcpy as an intrinsic for aggregate copies. So: struct point { int x, y; } a, b; a = b; // Compiler will generate a builtin memcpy to copy b into a This is why I have a hard time believing the lowering is broken on so many architectures. Calling memcpy to copy aggregates everywhere would be madness. I believe, especially given the period of time this code has been in the kernel, that this problem is specific to GCC and ARMv5. Clearly fixing a bug in GCC and ARMv5 should be done, but in the meantime, a config option workaround in the kernel is pragmatic. I also don't want to start a religious war, a solid -fno-strict-aliasing stance, including with packed type punning quirks, I'm cool with. It is better than libbpf that has a no -fno-strict-aliasing but let's violate strict aliasing stance [1]. Thanks, Ian [1] https://lore.kernel.org/linux-perf-users/CAADnVQLKv6LOQw5=8_ZagRXdU7dRPYA+fxJsXiGncqJjZ2hK1w@mail.gmail.com/ > Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-19 17:04 ` Ian Rogers @ 2026-10-19 21:00 ` Arnd Bergmann 2026-10-19 21:14 ` Ian Rogers 0 siblings, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2026-10-19 21:00 UTC (permalink / raw) To: Ian Rogers Cc: Marc Kleine-Budde, Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Mon, Oct 19, 2026, at 19:04, Ian Rogers wrote: > On Mon, Oct 19, 2026 at 9:28 AM Arnd Bergmann <arnd@arndb.de> wrote: >> On Mon, Oct 19, 2026, at 17:10, Ian Rogers wrote: >> > On Mon, Oct 19, 2026 at 5:37 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote: >> As I wrote, we need the packed struct version at least on mips32, >> mips64, openrisc, riscv32, riscv64, sh4, sparc32, and sparc64 to avoid >> the external library calls the same as on armv5, but I would prefer >> to just always use that in the kernel. My guess is that this list >> is actually the same as the architectures that lack unaligned word >> access instructions in hardware, and armv5 is certainly not >> a special case. > > Is it more than conjecture that these architectures have regressed? I > have a hard time believing the codegen on say riscv64 is lagging > aarch64. When the series originally landed the issue was looked at and > workarounds were added, for example, to PA-RISC. I tried compiling a trivial function that calls get_unaligned() and looked at the assembler output for all architectures. The list is everything that does a function call to memcpy() rather than open-coding the data access. By default (but depending on the target CPU), riscv64-gcc avoids unaligned access because this is not a required hardware feature and may be emulated by machine mode firmware even when it does work, so a function call is cheaper than a trap. On arm64, armv7 and most other modern architectures, unaligned non-atomic word access is always faster than byte-at-a-time, so the compiler defaults to -mno-strict-align or -munaligned-access. >> For 7.4 (and stable kernels) reverting the in-kernel header seems >> best to me, and then we can think about what to use for the tools. >> >> Looking at the uses of get_unaligned() in the tools directory, >> I notice that at least some of them should just not exist, e.g. >> struct debug_entry in tools/perf/util/jitdump.h is just >> needs a __packed attribute to correctly describe what it does, >> and tools/testing/cxl/test/mem.c does not need the unaligned >> helpers at all because the structs are all packed already. > > There was supposed to be an 8-byte alignment requirement for perf > events and unfortunately some events broke this and we added __packed > as a workaround. The unaligned accesses wouldn't trip a sanitizer in > our tests because we sadly lack testing on old perf.data files. There > is a difference in using __packed as an attribute and using type > punning to achieve an unaligned access. Yes, that was my point: if the unaligned data is annotated correctly as __packed, you don't need any type punning to access it -- you can rely on the compiler to pick sensible instructions for the target architecture, either an unaligned load/store a byte-at-a-time. >> I'm also a bit suspicious of the usage in >> tools/testing/selftests/bpf/test_kmods/bpf_testmod.c, which >> looks like it may run into a corner case: >> >> __bpf_kfunc int bpf_kfunc_test_uninit_alias(int *out__uninit, const int *in) >> { >> int value = get_unaligned(in); >> put_unaligned(42, out__uninit); >> return value; >> } >> >> The current version of get_unaligned() turns into __builtin_memcpy(), >> and the function prototype defines both arguments as pointers >> to naturally aligned objects. Do you see anything that would >> stop the compiler from replacing the __builtin_memcpy() with >> an invalid pointer dereference? This bit is probably fine >> because of the way gcc handles __builtin_memcpy() optimization, >> but I'm not sure if that's an implementation detail or required >> by the C standard, especially if strict aliasing rules say >> that the int* argument cannot overlap a misaligned int*. > > I agree the pointers should be aligned given their types, ubsan > doesn't currently fail for this and waits for use of the pointer. So > there is the specification and also what sanitizers enforce. > For clang, when lowering from parsed C to machine code the compiler > will use alloca to model local variables and memcpy as an intrinsic > for aggregate copies. So: > > struct point { int x, y; } a, b; > a = b; // Compiler will generate a builtin memcpy to copy b into a The problem I am referring to is specifically where the object itself is unaligned and passed into a function through a non-__packed reference. In this case, the get_unaligned() must ensure both that all address bits of the pointer are used and that each load/store is done according to the target CPU definition and -m{no-,}unaligned-access flag. We have had cases where x86_64-gcc optimized away the low-order pointer bits based on the psABI guarantees, and it wouldn't surprise me if the inlined memcpy tried to make similar assumptions in some corner cases on -mno-unaligned-access targets. On ARMv5 and similar architectures this would still appear to work correctly but cause trapped loads that would be absolutely detrimental to performance, much worse than a few added memcpy() calls. > This is why I have a hard time believing the lowering is broken on so > many architectures. Calling memcpy to copy aggregates everywhere would > be madness. I believe, especially given the period of time this code > has been in the kernel, that this problem is specific to GCC and > ARMv5. Clearly fixing a bug in GCC and ARMv5 should be done, but in > the meantime, a config option workaround in the kernel is pragmatic. I'm not sure what you expect the compiler to do on ARMv5 and other targets with -mno-unaligned-access, or why you'd consider calling memcpy() a bug when that is literally what the code asks for and there is no easy way to convert it to word load/store. Comparing the gcc and clang output in https://godbolt.org/z/r8aq4n87z it's clear that the gcc version is at least shorter but depending on the CPU and caller it can also be faster, e.g. when the data is usually naturally aligned and the memcpy() library function does a single store while the clang version fails to write-combine the multiple bytes being written in the wrong order. Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-19 21:00 ` Arnd Bergmann @ 2026-10-19 21:14 ` Ian Rogers 2026-10-19 22:05 ` Arnd Bergmann 0 siblings, 1 reply; 36+ messages in thread From: Ian Rogers @ 2026-10-19 21:14 UTC (permalink / raw) To: Arnd Bergmann Cc: Marc Kleine-Budde, Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Mon, Oct 19, 2026 at 2:00 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Mon, Oct 19, 2026, at 19:04, Ian Rogers wrote: > > On Mon, Oct 19, 2026 at 9:28 AM Arnd Bergmann <arnd@arndb.de> wrote: > >> On Mon, Oct 19, 2026, at 17:10, Ian Rogers wrote: > >> > On Mon, Oct 19, 2026 at 5:37 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote: > >> As I wrote, we need the packed struct version at least on mips32, > >> mips64, openrisc, riscv32, riscv64, sh4, sparc32, and sparc64 to avoid > >> the external library calls the same as on armv5, but I would prefer > >> to just always use that in the kernel. My guess is that this list > >> is actually the same as the architectures that lack unaligned word > >> access instructions in hardware, and armv5 is certainly not > >> a special case. > > > > Is it more than conjecture that these architectures have regressed? I > > have a hard time believing the codegen on say riscv64 is lagging > > aarch64. When the series originally landed the issue was looked at and > > workarounds were added, for example, to PA-RISC. > > I tried compiling a trivial function that calls get_unaligned() and > looked at the assembler output for all architectures. The list is > everything that does a function call to memcpy() rather than open-coding > the data access. > > By default (but depending on the target CPU), riscv64-gcc avoids unaligned > access because this is not a required hardware feature and may be > emulated by machine mode firmware even when it does work, so a > function call is cheaper than a trap. > On arm64, armv7 and most other modern architectures, unaligned non-atomic > word access is always faster than byte-at-a-time, so the compiler > defaults to -mno-strict-align or -munaligned-access. > > >> For 7.4 (and stable kernels) reverting the in-kernel header seems > >> best to me, and then we can think about what to use for the tools. > >> > >> Looking at the uses of get_unaligned() in the tools directory, > >> I notice that at least some of them should just not exist, e.g. > >> struct debug_entry in tools/perf/util/jitdump.h is just > >> needs a __packed attribute to correctly describe what it does, > >> and tools/testing/cxl/test/mem.c does not need the unaligned > >> helpers at all because the structs are all packed already. > > > > There was supposed to be an 8-byte alignment requirement for perf > > events and unfortunately some events broke this and we added __packed > > as a workaround. The unaligned accesses wouldn't trip a sanitizer in > > our tests because we sadly lack testing on old perf.data files. There > > is a difference in using __packed as an attribute and using type > > punning to achieve an unaligned access. > > Yes, that was my point: if the unaligned data is annotated correctly > as __packed, you don't need any type punning to access it -- you > can rely on the compiler to pick sensible instructions for the > target architecture, either an unaligned load/store a byte-at-a-time. > > >> I'm also a bit suspicious of the usage in > >> tools/testing/selftests/bpf/test_kmods/bpf_testmod.c, which > >> looks like it may run into a corner case: > >> > >> __bpf_kfunc int bpf_kfunc_test_uninit_alias(int *out__uninit, const int *in) > >> { > >> int value = get_unaligned(in); > >> put_unaligned(42, out__uninit); > >> return value; > >> } > >> > >> The current version of get_unaligned() turns into __builtin_memcpy(), > >> and the function prototype defines both arguments as pointers > >> to naturally aligned objects. Do you see anything that would > >> stop the compiler from replacing the __builtin_memcpy() with > >> an invalid pointer dereference? This bit is probably fine > >> because of the way gcc handles __builtin_memcpy() optimization, > >> but I'm not sure if that's an implementation detail or required > >> by the C standard, especially if strict aliasing rules say > >> that the int* argument cannot overlap a misaligned int*. > > > > I agree the pointers should be aligned given their types, ubsan > > doesn't currently fail for this and waits for use of the pointer. So > > there is the specification and also what sanitizers enforce. > > For clang, when lowering from parsed C to machine code the compiler > > will use alloca to model local variables and memcpy as an intrinsic > > for aggregate copies. So: > > > > struct point { int x, y; } a, b; > > a = b; // Compiler will generate a builtin memcpy to copy b into a > > The problem I am referring to is specifically where the object > itself is unaligned and passed into a function through a non-__packed > reference. In this case, the get_unaligned() must ensure both that > all address bits of the pointer are used and that each load/store > is done according to the target CPU definition and > -m{no-,}unaligned-access flag. We have had cases where x86_64-gcc > optimized away the low-order pointer bits based on the psABI > guarantees, and it wouldn't surprise me if the inlined memcpy > tried to make similar assumptions in some corner cases on > -mno-unaligned-access targets. > > On ARMv5 and similar architectures this would still appear to > work correctly but cause trapped loads that would be absolutely > detrimental to performance, much worse than a few added memcpy() > calls. > > > This is why I have a hard time believing the lowering is broken on so > > many architectures. Calling memcpy to copy aggregates everywhere would > > be madness. I believe, especially given the period of time this code > > has been in the kernel, that this problem is specific to GCC and > > ARMv5. Clearly fixing a bug in GCC and ARMv5 should be done, but in > > the meantime, a config option workaround in the kernel is pragmatic. > > I'm not sure what you expect the compiler to do on ARMv5 and other > targets with -mno-unaligned-access, or why you'd consider calling > memcpy() a bug when that is literally what the code asks for and > there is no easy way to convert it to word load/store. > > Comparing the gcc and clang output in https://godbolt.org/z/r8aq4n87z > it's clear that the gcc version is at least shorter but depending > on the CPU and caller it can also be faster, e.g. when the data > is usually naturally aligned and the memcpy() library function > does a single store while the clang version fails to write-combine > the multiple bytes being written in the wrong order. Is there a reason that the compiler's instruction selection can't make the same decision for the memcpy case as the __packed case? We can work around the compiler bug with a config option. The intent with using __builtin_memcpy, as opposed to memcpy, is to avoid a library call. It looks like all the unaligned code is using void* so I'm not sure where the low bit concerns are stemming from. If anything, type punning seems to be a worse problem, but the hope is that the __packed attribute avoids it. I know this assumption makes compiler people uncomfortable and memcpy is their preferred go to. Thanks, Ian > Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-19 21:14 ` Ian Rogers @ 2026-10-19 22:05 ` Arnd Bergmann 2026-10-20 16:21 ` Ian Rogers 0 siblings, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2026-10-19 22:05 UTC (permalink / raw) To: Ian Rogers Cc: Marc Kleine-Budde, Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Mon, Oct 19, 2026, at 23:14, Ian Rogers wrote: > On Mon, Oct 19, 2026 at 2:00 PM Arnd Bergmann <arnd@arndb.de> wrote: >> On Mon, Oct 19, 2026, at 19:04, Ian Rogers wrote: > > Is there a reason that the compiler's instruction selection can't make > the same decision for the memcpy case as the __packed case? We can > work around the compiler bug with a config option. > The intent with using __builtin_memcpy, as opposed to memcpy, is to > avoid a library call. As far as I understand, the only difference between calling __builtin_memcpy() and memcpy() is the need for a prototype for the latter, I would expect the compiler to behave the exact same for for both otherwise. Whether any of the string.h functions are replaced by an inline alternative is a complex optimizer decision, not something you can enforce by calling the function by one name rather than the other. If you try this one on riscv64, you will find that both gcc and clang use the byte-at-a-time version at -O2 but the out-of-line one for -Os (regardless of __builtin_*): static inline int f(void *p) { int i; __builtin_memcpy(p, &i, sizeof(i)); return i; } int g(int *p) { return f(p); } > It looks like all the unaligned code is using void* so I'm not sure > where the low bit concerns are stemming from. If anything, type > punning seems to be a worse problem, but the hope is that the __packed > attribute avoids it. I know this assumption makes compiler people > uncomfortable and memcpy is their preferred go to. The problem with the bpf_kfunc_test_uninit_alias() function is that passing an unaligned pointer into it is undefined behavior, and the only way I know to make it safe is type-punning to a __packed pointer in combination with traditional aliasing rules, which tells the compiler exactly how to access the data behind the pointer. Whether memcpy() is strong enough to have the same effect in that example is something I don't know as I never tried to program with strict aliasing rules, so maybe I'm just be overly cautious: The include/linux/unaligned.h functions are all 'inline', so the compiler still has knowledge about the alignment of the accessed objects from the caller and with -fstrict-aliasing may be able to assume that the object is aligned. Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-19 22:05 ` Arnd Bergmann @ 2026-10-20 16:21 ` Ian Rogers 2026-10-20 19:27 ` David Laight 0 siblings, 1 reply; 36+ messages in thread From: Ian Rogers @ 2026-10-20 16:21 UTC (permalink / raw) To: Arnd Bergmann Cc: Marc Kleine-Budde, Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Mon, Oct 19, 2026 at 3:05 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Mon, Oct 19, 2026, at 23:14, Ian Rogers wrote: > > On Mon, Oct 19, 2026 at 2:00 PM Arnd Bergmann <arnd@arndb.de> wrote: > >> On Mon, Oct 19, 2026, at 19:04, Ian Rogers wrote: > > > > Is there a reason that the compiler's instruction selection can't make > > the same decision for the memcpy case as the __packed case? We can > > work around the compiler bug with a config option. > > The intent with using __builtin_memcpy, as opposed to memcpy, is to > > avoid a library call. > > As far as I understand, the only difference between calling > __builtin_memcpy() and memcpy() is the need for a prototype > for the latter, I would expect the compiler to behave the exact > same for for both otherwise. Whether any of the string.h functions > are replaced by an inline alternative is a complex optimizer > decision, not something you can enforce by calling the function > by one name rather than the other. So to repeat, relying on __packed and assuming __packed won't be lowered to a memcpy(), memset(), etc. call by the compiler is false. Both GCC and Clang lower __packed accesses and assignments to out-of-line memcpy()/memset() calls in a variety of cases on -mno-unaligned-access targets: https://godbolt.org/z/qKWv6Gdav Using the __packed definitions of get_unaligned() and put_unaligned(): #define __get_unaligned_t(type, ptr) ({ \ const struct { type x; } __packed *__get_pptr = (typeof(__get_pptr))(ptr); \ __get_pptr->x; \ }) #define __put_unaligned_t(type, val, ptr) do { \ struct { type x; } __packed *__put_pptr = (typeof(__put_pptr))(ptr); \ __put_pptr->x = (val); \ } while (0) 1. 4-byte aggregate with __packed put_unaligned(): struct s4 { short a, b; }; void put_s4(struct s4 *p, struct s4 v) { put_unaligned(v, p); } Even though struct s4 is 4 bytes and passed in a single register (r1), GCC with -O2 -mno-unaligned-access spills r1 to the stack and calls memcpy() for 4 bytes: put_s4: str lr, [sp, #-4]! mov r2, #4 sub sp, sp, #12 str r1, [sp, #4] add r1, sp, r2 bl memcpy add sp, sp, #12 ldr pc, [sp], #4 2. Copying a 4-byte __packed struct containing a single int: struct packed_int { int x; } __packed; void copy_packed_int(struct packed_int *dst, const struct packed_int *src) { *dst = *src; } Clang's frontend lowers *dst = *src directly to @llvm.memcpy(..., align 1), and GCC with -O2 -mno-unaligned-access emits an out-of-line call to memcpy(..., 4) (as does riscv64-linux-gnu-gcc -Os). 3. struct debug_entry __packed (from tools/perf/util/jitdump.h): struct debug_entry { unsigned long long addr; int lineno; int discrim; } __packed; struct debug_entry get_entry(const void *p) { return get_unaligned((const struct debug_entry *)p); } void copy_entry(struct debug_entry *dst, const struct debug_entry *src) { *dst = *src; } Whether accessed via the __packed get_unaligned() macro or via direct assignment on a __packed struct, both GCC and Clang emit memcpy() calls. In fact, with the __packed statement-expression macro, GCC emits two back-to-back memcpy() calls in get_entry() (one into the statement expression temporary on the stack, and a second from the stack into the return slot). 4. Loops of scalar int __packed get_unaligned() / put_unaligned(): void copy_8_ints(int * restrict dst, const int * restrict src) { for (int i = 0; i < 8; i++) put_unaligned(get_unaligned(&src[i]), &dst[i]); } Even for a scalar int, loop idiom recognition in both GCC and Clang at -O2 turns the __packed loads/stores into a call to memcpy(dst, src, 32). 5. Adjacent scalar __packed zero stores promoted to memset(): void zero_2_u64(long long *dst) { put_unaligned(0LL, &dst[0]); put_unaligned(0LL, &dst[1]); } Even in straight-line code without a loop, Clang's MemCpyOpt merges two adjacent 64-bit (or four 32-bit) __packed zero stores into @llvm.memset(..., align 1), which lowers under -O2 -mno-unaligned-access to an out-of-line memset() call: zero_2_u64: mov r1, #0 mov r2, #16 b memset On the Kconfig patch, I have updated v2 so CONFIG_VDSO_UNALIGNED_PACKED defaults to !HAVE_EFFICIENT_UNALIGNED_ACCESS to cover all strict-alignment targets rather than just CPU_32v5. I continue to think that: 1) given __packed implicitly can be turned into a memcpy, it is no better than a memcpy; 2) the __builtin_memcpy for efficient unaligned targets avoids type punning, is more intention revealing, is clearer for compiler writers, and is what compiler writers tell me they want. That said, the code with a __packed or with a memcpy can be made to work, optimized, etc. I'd like to be pragmatic and address Stefan and Marc's problem so a revert works, I just think we can try to do a little better. Thanks, Ian ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-20 16:21 ` Ian Rogers @ 2026-10-20 19:27 ` David Laight 2026-10-20 21:20 ` David Laight 0 siblings, 1 reply; 36+ messages in thread From: David Laight @ 2026-10-20 19:27 UTC (permalink / raw) To: Ian Rogers Cc: Arnd Bergmann, Marc Kleine-Budde, Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Tue, 20 Oct 2026 09:21:47 -0700 Ian Rogers <irogers@google.com> wrote: > On Mon, Oct 19, 2026 at 3:05 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Mon, Oct 19, 2026, at 23:14, Ian Rogers wrote: > > > On Mon, Oct 19, 2026 at 2:00 PM Arnd Bergmann <arnd@arndb.de> wrote: > > >> On Mon, Oct 19, 2026, at 19:04, Ian Rogers wrote: > > > > > > Is there a reason that the compiler's instruction selection can't make > > > the same decision for the memcpy case as the __packed case? We can > > > work around the compiler bug with a config option. > > > The intent with using __builtin_memcpy, as opposed to memcpy, is to > > > avoid a library call. > > > > As far as I understand, the only difference between calling > > __builtin_memcpy() and memcpy() is the need for a prototype > > for the latter, I would expect the compiler to behave the exact > > same for for both otherwise. Whether any of the string.h functions > > are replaced by an inline alternative is a complex optimizer > > decision, not something you can enforce by calling the function > > by one name rather than the other. > > So to repeat, relying on __packed and assuming __packed won't be > lowered to a memcpy(), memset(), etc. call by the compiler is false. > Both GCC and Clang lower __packed accesses and assignments to > out-of-line memcpy()/memset() calls in a variety of cases on > -mno-unaligned-access targets: > > https://godbolt.org/z/qKWv6Gdav > > Using the __packed definitions of get_unaligned() and put_unaligned(): > > #define __get_unaligned_t(type, ptr) ({ \ > const struct { type x; } __packed *__get_pptr = > (typeof(__get_pptr))(ptr); \ > __get_pptr->x; \ > }) > > #define __put_unaligned_t(type, val, ptr) do { \ > struct { type x; } __packed *__put_pptr = > (typeof(__put_pptr))(ptr); \ > __put_pptr->x = (val); \ > } while (0) > > 1. 4-byte aggregate with __packed put_unaligned(): > > struct s4 { short a, b; }; > void put_s4(struct s4 *p, struct s4 v) > { > put_unaligned(v, p); > } > > Even though struct s4 is 4 bytes and passed in a single register (r1), > GCC with -O2 -mno-unaligned-access spills r1 to the stack and calls > memcpy() for 4 bytes: > > put_s4: > str lr, [sp, #-4]! > mov r2, #4 > sub sp, sp, #12 > str r1, [sp, #4] > add r1, sp, r2 > bl memcpy > add sp, sp, #12 > ldr pc, [sp], #4 > > 2. Copying a 4-byte __packed struct containing a single int: > > struct packed_int { int x; } __packed; > void copy_packed_int(struct packed_int *dst, const struct packed_int *src) > { > *dst = *src; > } > > Clang's frontend lowers *dst = *src directly to @llvm.memcpy(..., > align 1), and GCC with -O2 -mno-unaligned-access emits an out-of-line > call to memcpy(..., 4) (as does riscv64-linux-gnu-gcc -Os). > > 3. struct debug_entry __packed (from tools/perf/util/jitdump.h): > > struct debug_entry { > unsigned long long addr; > int lineno; > int discrim; > } __packed; > > struct debug_entry get_entry(const void *p) > { > return get_unaligned((const struct debug_entry *)p); > } > > void copy_entry(struct debug_entry *dst, const struct debug_entry *src) > { > *dst = *src; > } > > Whether accessed via the __packed get_unaligned() macro or via direct > assignment on a __packed struct, both GCC and Clang emit memcpy() > calls. In fact, with the __packed statement-expression macro, GCC > emits two back-to-back memcpy() calls in get_entry() (one into the > statement expression temporary on the stack, and a second from the > stack into the return slot). > > 4. Loops of scalar int __packed get_unaligned() / put_unaligned(): > > void copy_8_ints(int * restrict dst, const int * restrict src) > { > for (int i = 0; i < 8; i++) > put_unaligned(get_unaligned(&src[i]), &dst[i]); > } > > Even for a scalar int, loop idiom recognition in both GCC and Clang at > -O2 turns the __packed loads/stores into a call to memcpy(dst, src, > 32). > > 5. Adjacent scalar __packed zero stores promoted to memset(): > > void zero_2_u64(long long *dst) > { > put_unaligned(0LL, &dst[0]); > put_unaligned(0LL, &dst[1]); > } > > Even in straight-line code without a loop, Clang's MemCpyOpt merges > two adjacent 64-bit (or four 32-bit) __packed zero stores into > @llvm.memset(..., align 1), which lowers under -O2 > -mno-unaligned-access to an out-of-line memset() call: > > zero_2_u64: > mov r1, #0 > mov r2, #16 > b memset > > On the Kconfig patch, I have updated v2 so > CONFIG_VDSO_UNALIGNED_PACKED defaults to > !HAVE_EFFICIENT_UNALIGNED_ACCESS to cover all strict-alignment targets > rather than just CPU_32v5. > > I continue to think that: > > 1) given __packed implicitly can be turned into a memcpy, it is no > better than a memcpy; I suspect it happens less often with __packed. You can probably add volatile to stop the other merging. It really might be necessary to give up on the C versions and use asm() (at least for the unaligned access case). David > 2) the __builtin_memcpy for efficient unaligned targets avoids type > punning, is more intention revealing, is clearer for compiler writers, > and is what compiler writers tell me they want. > > That said, the code with a __packed or with a memcpy can be made to > work, optimized, etc. I'd like to be pragmatic and address Stefan and > Marc's problem so a revert works, I just think we can try to do a > little better. > > Thanks, > Ian > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-20 19:27 ` David Laight @ 2026-10-20 21:20 ` David Laight 0 siblings, 0 replies; 36+ messages in thread From: David Laight @ 2026-10-20 21:20 UTC (permalink / raw) To: Ian Rogers Cc: Arnd Bergmann, Marc Kleine-Budde, Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Tue, 20 Oct 2026 20:27:44 +0100 David Laight <david.laight.linux@gmail.com> wrote: > I suspect it happens less often with __packed. > You can probably add volatile to stop the other merging. > > It really might be necessary to give up on the C versions and use asm() > (at least for the unaligned access case). This probably works for unaligned 64bit - and is much better than the code the compiler generates: unsigned long get_u64_le(void *s) { unsigned long p = (unsigned long)s; unsigned int shift = 8 * (p & 7); return *(unsigned long *)(p & ~7) >> shift | *(unsigned long *)((p + 7) & ~7) << (-shift & 63); } For native BE the two shifts need swapping. Not worth it for 32bit unless the alignment is known. (And neither gcc nor clang do that for misaligned members in aligned struct.) > > David ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-19 16:27 ` Arnd Bergmann 2026-10-19 17:04 ` Ian Rogers @ 2026-10-20 8:36 ` David Laight 2026-10-20 8:41 ` Arnd Bergmann 1 sibling, 1 reply; 36+ messages in thread From: David Laight @ 2026-10-20 8:36 UTC (permalink / raw) To: Arnd Bergmann Cc: Ian Rogers, Marc Kleine-Budde, Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Mon, 19 Oct 2026 18:27:24 +0200 "Arnd Bergmann" <arnd@arndb.de> wrote: > On Mon, Oct 19, 2026, at 17:10, Ian Rogers wrote: > > On Mon, Oct 19, 2026 at 5:37 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote: > >> On 10.10.2026 08:04:42, Ian Rogers wrote: > > > > It is a little more manual than automagic :-) When perf builds there > > is a script that complains if the tools' version of files is > > out-of-sync with the kernel's: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/check-headers.sh#n28 > > It doesn't break the perf build, it just adds noise to it. > > I see that the include/linux/unaligned.h and tools/include/linux/unaligned.h > headers have not been in sync for a long time, so maybe it's just not > a big deal there. > > > My understanding, biased by my opinions, is that the preferred option > > is to add a config variable controlling whether to use memcpy or a > > packed struct. My preference, to avoid undefined behavior, is to > > default to memcpy but set it to use packed structs for ARMv5. I will > > vibe code this up, and apologize in advance for any offense such a > > patch series may cause (ie I'm sure someone on LKML will want to > > complain at me for using memcpy at all and give me a lecture about > > strict aliasing being nonsense, etc.). Arnd or someone else may have a > > different preferred solution, to which I'm more than happy to defer. > > As I wrote, we need the packed struct version at least on mips32, > mips64, openrisc, riscv32, riscv64, sh4, sparc32, and sparc64 to avoid > the external library calls the same as on armv5, but I would prefer > to just always use that in the kernel. My guess is that this list > is actually the same as the architectures that lack unaligned word > access instructions in hardware, and armv5 is certainly not > a special case. > > For 7.4 (and stable kernels) reverting the in-kernel header seems > best to me, and then we can think about what to use for the tools. > > Looking at the uses of get_unaligned() in the tools directory, > I notice that at least some of them should just not exist, e.g. > struct debug_entry in tools/perf/util/jitdump.h is just > needs a __packed attribute to correctly describe what it does, > and tools/testing/cxl/test/mem.c does not need the unaligned > helpers at all because the structs are all packed already. > > I'm also a bit suspicious of the usage in > tools/testing/selftests/bpf/test_kmods/bpf_testmod.c, which > looks like it may run into a corner case: > > __bpf_kfunc int bpf_kfunc_test_uninit_alias(int *out__uninit, const int *in) > { > int value = get_unaligned(in); > put_unaligned(42, out__uninit); > return value; > } > > The current version of get_unaligned() turns into __builtin_memcpy(), > and the function prototype defines both arguments as pointers > to naturally aligned objects. Do you see anything that would > stop the compiler from replacing the __builtin_memcpy() with > an invalid pointer dereference? Historically it has definitely done that. And you can't 'cast away' the alignment implicit in a type. The alignment can get tracked into an inlined function. Plenty of crashes on sparc32 from that one. Any structure that might be misaligned in memory better be packed. But what is actually wrong with using byte access? eg: u32 get_le32(u8 *p) { return p[0] | p[1] << 8 | p[2] << 16 | p[3] << 24; } On x86, arm64, ppc (etc) both gcc and clang generate a single memory read (byteswapped if necessary). https://godbolt.org/z/d33hvesr8 David > This bit is probably fine > because of the way gcc handles __builtin_memcpy() optimization, > but I'm not sure if that's an implementation detail or required > by the C standard, especially if strict aliasing rules say > that the int* argument cannot overlap a misaligned int*. > > Arnd > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-20 8:36 ` David Laight @ 2026-10-20 8:41 ` Arnd Bergmann 2026-10-20 10:01 ` David Laight 0 siblings, 1 reply; 36+ messages in thread From: Arnd Bergmann @ 2026-10-20 8:41 UTC (permalink / raw) To: David Laight Cc: Ian Rogers, Marc Kleine-Budde, Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Tue, Oct 20, 2026, at 10:36, David Laight wrote: > > Historically it has definitely done that. > And you can't 'cast away' the alignment implicit in a type. > The alignment can get tracked into an inlined function. > Plenty of crashes on sparc32 from that one. > > Any structure that might be misaligned in memory better be packed. > > But what is actually wrong with using byte access? eg: > u32 get_le32(u8 *p) { return p[0] | p[1] << 8 | p[2] << 16 | p[3] << 24; } > On x86, arm64, ppc (etc) both gcc and clang generate a single memory read > (byteswapped if necessary). > > https://godbolt.org/z/d33hvesr8 IIRC this is one of the versions we had in the past on some architectures, but the packed struct version ended up being better in some cases and never worse. It is possible that on new compilers are just as good with this version, and if someone can show that being true I think we can do that here as well. Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy 2026-10-20 8:41 ` Arnd Bergmann @ 2026-10-20 10:01 ` David Laight 0 siblings, 0 replies; 36+ messages in thread From: David Laight @ 2026-10-20 10:01 UTC (permalink / raw) To: Arnd Bergmann Cc: Ian Rogers, Marc Kleine-Budde, Thomas Gleixner, Stefan Kerkmann, James E . J . Bottomley, Helge Deller, Andy Lutomirski, Vincenzo Frascino, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Alexander Viro, Christophe Leroy, Jason A . Donenfeld On Tue, 20 Oct 2026 10:41:27 +0200 "Arnd Bergmann" <arnd@arndb.de> wrote: > On Tue, Oct 20, 2026, at 10:36, David Laight wrote: > > > > Historically it has definitely done that. > > And you can't 'cast away' the alignment implicit in a type. > > The alignment can get tracked into an inlined function. > > Plenty of crashes on sparc32 from that one. > > > > Any structure that might be misaligned in memory better be packed. > > > > But what is actually wrong with using byte access? eg: > > u32 get_le32(u8 *p) { return p[0] | p[1] << 8 | p[2] << 16 | p[3] << 24; } > > On x86, arm64, ppc (etc) both gcc and clang generate a single memory read > > (byteswapped if necessary). > > > > https://godbolt.org/z/d33hvesr8 > > IIRC this is one of the versions we had in the past on some architectures, > but the packed struct version ended up being better in some cases and > never worse. It is possible that on new compilers are just as good with this > version, and if someone can show that being true I think we can do > that here as well. I was only looking that is generated reasonable code on x86. (Ok today, but I'm sure gcc didn't optimise it last time I looked, my memory must be going.) An alternative would be to use OPTIMIZER_HIDE_VAR() on the pointer with a (partial) memory clobber and the packed struct version. (You need the memory clobber after the write as well as before.) That might add a register-register move (and so a spill). David > > Arnd ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v5 3/4] tools headers: Update the linux/unaligned.h copy with the kernel sources 2025-10-16 20:51 [PATCH v5 0/4] Switch get/put unaligned to use memcpy Ian Rogers 2025-10-16 20:51 ` [PATCH v5 1/4] parisc: Inline a type punning version of get_unaligned_le32 Ian Rogers 2025-10-16 20:51 ` [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Ian Rogers @ 2025-10-16 20:51 ` Ian Rogers 2026-01-13 13:47 ` [tip: timers/vdso] " tip-bot2 for Ian Rogers 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 2025-10-16 20:51 ` [PATCH v5 4/4] tools headers: Remove unneeded ignoring of warnings in unaligned.h Ian Rogers 3 siblings, 2 replies; 36+ messages in thread From: Ian Rogers @ 2025-10-16 20:51 UTC (permalink / raw) To: James E.J. Bottomley, Helge Deller, Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, Ian Rogers, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld To pick up the changes in: vdso: Switch get/put unaligned from packed struct to memcpy As the code is dependent on __unqual_scalar_typeof, update the tools version of compiler_types.h to include this. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/include/linux/compiler_types.h | 22 +++++++++++++++ tools/include/vdso/unaligned.h | 41 ++++++++++++++++++++++++---- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/tools/include/linux/compiler_types.h b/tools/include/linux/compiler_types.h index d09f9dc172a4..890982283a5e 100644 --- a/tools/include/linux/compiler_types.h +++ b/tools/include/linux/compiler_types.h @@ -40,4 +40,26 @@ #define asm_goto_output(x...) asm goto(x) #endif +/* + * __unqual_scalar_typeof(x) - Declare an unqualified scalar type, leaving + * non-scalar types unchanged. + */ +/* + * Prefer C11 _Generic for better compile-times and simpler code. Note: 'char' + * is not type-compatible with 'signed char', and we define a separate case. + */ +#define __scalar_type_to_expr_cases(type) \ + unsigned type: (unsigned type)0, \ + signed type: (signed type)0 + +#define __unqual_scalar_typeof(x) typeof( \ + _Generic((x), \ + char: (char)0, \ + __scalar_type_to_expr_cases(char), \ + __scalar_type_to_expr_cases(short), \ + __scalar_type_to_expr_cases(int), \ + __scalar_type_to_expr_cases(long), \ + __scalar_type_to_expr_cases(long long), \ + default: (x))) + #endif /* __LINUX_COMPILER_TYPES_H */ diff --git a/tools/include/vdso/unaligned.h b/tools/include/vdso/unaligned.h index ff0c06b6513e..9076483c9fbb 100644 --- a/tools/include/vdso/unaligned.h +++ b/tools/include/vdso/unaligned.h @@ -2,14 +2,43 @@ #ifndef __VDSO_UNALIGNED_H #define __VDSO_UNALIGNED_H -#define __get_unaligned_t(type, ptr) ({ \ - const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr); \ - __get_pptr->x; \ +#include <linux/compiler_types.h> + +/** + * __get_unaligned_t - read an unaligned value from memory. + * @type: the type to load from the pointer. + * @ptr: the pointer to load from. + * + * Use memcpy to affect an unaligned type sized load avoiding undefined behavior + * from approaches like type punning that require -fno-strict-aliasing in order + * to be correct. As type may be const, use __unqual_scalar_typeof to map to a + * non-const type - you can't memcpy into a const type. The + * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required + * expression rather than type, a pointer is used to avoid warnings about mixing + * the use of 0 and NULL. The void* cast silences ubsan warnings. + */ +#define __get_unaligned_t(type, ptr) ({ \ + type *__get_unaligned_ctrl_type __always_unused = NULL; \ + __unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \ + __builtin_memcpy(&__get_unaligned_val, (void *)(ptr), \ + sizeof(__get_unaligned_val)); \ + __get_unaligned_val; \ }) -#define __put_unaligned_t(type, val, ptr) do { \ - struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr); \ - __put_pptr->x = (val); \ +/** + * __put_unaligned_t - write an unaligned value to memory. + * @type: the type of the value to store. + * @val: the value to store. + * @ptr: the pointer to store to. + * + * Use memcpy to affect an unaligned type sized store avoiding undefined + * behavior from approaches like type punning that require -fno-strict-aliasing + * in order to be correct. The void* cast silences ubsan warnings. + */ +#define __put_unaligned_t(type, val, ptr) do { \ + type __put_unaligned_val = (val); \ + __builtin_memcpy((void *)(ptr), &__put_unaligned_val, \ + sizeof(__put_unaligned_val)); \ } while (0) #endif /* __VDSO_UNALIGNED_H */ -- 2.51.0.858.gf9c4a03a3a-goog ^ permalink raw reply [flat|nested] 36+ messages in thread
* [tip: timers/vdso] tools headers: Update the linux/unaligned.h copy with the kernel sources 2025-10-16 20:51 ` [PATCH v5 3/4] tools headers: Update the linux/unaligned.h copy with the kernel sources Ian Rogers @ 2026-01-13 13:47 ` tip-bot2 for Ian Rogers 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 1 sibling, 0 replies; 36+ messages in thread From: tip-bot2 for Ian Rogers @ 2026-01-13 13:47 UTC (permalink / raw) To: linux-tip-commits; +Cc: Ian Rogers, Thomas Gleixner, x86, linux-kernel The following commit has been merged into the timers/vdso branch of tip: Commit-ID: 029a9504d871ce72adf9a13e5f1ce66f1db48be3 Gitweb: https://git.kernel.org/tip/029a9504d871ce72adf9a13e5f1ce66f1db48be3 Author: Ian Rogers <irogers@google.com> AuthorDate: Thu, 16 Oct 2025 13:51:25 -07:00 Committer: Thomas Gleixner <tglx@kernel.org> CommitterDate: Tue, 13 Jan 2026 14:46:00 +01:00 tools headers: Update the linux/unaligned.h copy with the kernel sources To pick up the changes in: vdso: Switch get/put_unaligned() from packed struct to memcpy As the code is dependent on __unqual_scalar_typeof, update also the tools version of compiler_types.h to include this. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Thomas Gleixner <tglx@kernel.org> Link: https://patch.msgid.link/20251016205126.2882625-4-irogers@google.com --- tools/include/linux/compiler_types.h | 22 ++++++++++++++- tools/include/vdso/unaligned.h | 41 +++++++++++++++++++++++---- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/tools/include/linux/compiler_types.h b/tools/include/linux/compiler_types.h index d09f9dc..8909822 100644 --- a/tools/include/linux/compiler_types.h +++ b/tools/include/linux/compiler_types.h @@ -40,4 +40,26 @@ #define asm_goto_output(x...) asm goto(x) #endif +/* + * __unqual_scalar_typeof(x) - Declare an unqualified scalar type, leaving + * non-scalar types unchanged. + */ +/* + * Prefer C11 _Generic for better compile-times and simpler code. Note: 'char' + * is not type-compatible with 'signed char', and we define a separate case. + */ +#define __scalar_type_to_expr_cases(type) \ + unsigned type: (unsigned type)0, \ + signed type: (signed type)0 + +#define __unqual_scalar_typeof(x) typeof( \ + _Generic((x), \ + char: (char)0, \ + __scalar_type_to_expr_cases(char), \ + __scalar_type_to_expr_cases(short), \ + __scalar_type_to_expr_cases(int), \ + __scalar_type_to_expr_cases(long), \ + __scalar_type_to_expr_cases(long long), \ + default: (x))) + #endif /* __LINUX_COMPILER_TYPES_H */ diff --git a/tools/include/vdso/unaligned.h b/tools/include/vdso/unaligned.h index ff0c06b..9076483 100644 --- a/tools/include/vdso/unaligned.h +++ b/tools/include/vdso/unaligned.h @@ -2,14 +2,43 @@ #ifndef __VDSO_UNALIGNED_H #define __VDSO_UNALIGNED_H -#define __get_unaligned_t(type, ptr) ({ \ - const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr); \ - __get_pptr->x; \ +#include <linux/compiler_types.h> + +/** + * __get_unaligned_t - read an unaligned value from memory. + * @type: the type to load from the pointer. + * @ptr: the pointer to load from. + * + * Use memcpy to affect an unaligned type sized load avoiding undefined behavior + * from approaches like type punning that require -fno-strict-aliasing in order + * to be correct. As type may be const, use __unqual_scalar_typeof to map to a + * non-const type - you can't memcpy into a const type. The + * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required + * expression rather than type, a pointer is used to avoid warnings about mixing + * the use of 0 and NULL. The void* cast silences ubsan warnings. + */ +#define __get_unaligned_t(type, ptr) ({ \ + type *__get_unaligned_ctrl_type __always_unused = NULL; \ + __unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \ + __builtin_memcpy(&__get_unaligned_val, (void *)(ptr), \ + sizeof(__get_unaligned_val)); \ + __get_unaligned_val; \ }) -#define __put_unaligned_t(type, val, ptr) do { \ - struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr); \ - __put_pptr->x = (val); \ +/** + * __put_unaligned_t - write an unaligned value to memory. + * @type: the type of the value to store. + * @val: the value to store. + * @ptr: the pointer to store to. + * + * Use memcpy to affect an unaligned type sized store avoiding undefined + * behavior from approaches like type punning that require -fno-strict-aliasing + * in order to be correct. The void* cast silences ubsan warnings. + */ +#define __put_unaligned_t(type, val, ptr) do { \ + type __put_unaligned_val = (val); \ + __builtin_memcpy((void *)(ptr), &__put_unaligned_val, \ + sizeof(__put_unaligned_val)); \ } while (0) #endif /* __VDSO_UNALIGNED_H */ ^ permalink raw reply [flat|nested] 36+ messages in thread
* [tip: timers/vdso] tools headers: Update the linux/unaligned.h copy with the kernel sources 2025-10-16 20:51 ` [PATCH v5 3/4] tools headers: Update the linux/unaligned.h copy with the kernel sources Ian Rogers 2026-01-13 13:47 ` [tip: timers/vdso] " tip-bot2 for Ian Rogers @ 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 1 sibling, 0 replies; 36+ messages in thread From: tip-bot2 for Ian Rogers @ 2026-01-14 8:01 UTC (permalink / raw) To: linux-tip-commits; +Cc: Ian Rogers, Thomas Gleixner, x86, linux-kernel The following commit has been merged into the timers/vdso branch of tip: Commit-ID: 1d7cf255eefbb479d0eea9aa3b6372a1e52f8c62 Gitweb: https://git.kernel.org/tip/1d7cf255eefbb479d0eea9aa3b6372a1e52f8c62 Author: Ian Rogers <irogers@google.com> AuthorDate: Thu, 16 Oct 2025 13:51:25 -07:00 Committer: Thomas Gleixner <tglx@kernel.org> CommitterDate: Wed, 14 Jan 2026 08:56:41 +01:00 tools headers: Update the linux/unaligned.h copy with the kernel sources To pick up the changes in: vdso: Switch get/put_unaligned() from packed struct to memcpy As the code is dependent on __unqual_scalar_typeof, update also the tools version of compiler_types.h to include this. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Thomas Gleixner <tglx@kernel.org> Link: https://patch.msgid.link/20251016205126.2882625-4-irogers@google.com --- tools/include/linux/compiler_types.h | 22 ++++++++++++++- tools/include/vdso/unaligned.h | 41 +++++++++++++++++++++++---- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/tools/include/linux/compiler_types.h b/tools/include/linux/compiler_types.h index d09f9dc..8909822 100644 --- a/tools/include/linux/compiler_types.h +++ b/tools/include/linux/compiler_types.h @@ -40,4 +40,26 @@ #define asm_goto_output(x...) asm goto(x) #endif +/* + * __unqual_scalar_typeof(x) - Declare an unqualified scalar type, leaving + * non-scalar types unchanged. + */ +/* + * Prefer C11 _Generic for better compile-times and simpler code. Note: 'char' + * is not type-compatible with 'signed char', and we define a separate case. + */ +#define __scalar_type_to_expr_cases(type) \ + unsigned type: (unsigned type)0, \ + signed type: (signed type)0 + +#define __unqual_scalar_typeof(x) typeof( \ + _Generic((x), \ + char: (char)0, \ + __scalar_type_to_expr_cases(char), \ + __scalar_type_to_expr_cases(short), \ + __scalar_type_to_expr_cases(int), \ + __scalar_type_to_expr_cases(long), \ + __scalar_type_to_expr_cases(long long), \ + default: (x))) + #endif /* __LINUX_COMPILER_TYPES_H */ diff --git a/tools/include/vdso/unaligned.h b/tools/include/vdso/unaligned.h index ff0c06b..9076483 100644 --- a/tools/include/vdso/unaligned.h +++ b/tools/include/vdso/unaligned.h @@ -2,14 +2,43 @@ #ifndef __VDSO_UNALIGNED_H #define __VDSO_UNALIGNED_H -#define __get_unaligned_t(type, ptr) ({ \ - const struct { type x; } __packed * __get_pptr = (typeof(__get_pptr))(ptr); \ - __get_pptr->x; \ +#include <linux/compiler_types.h> + +/** + * __get_unaligned_t - read an unaligned value from memory. + * @type: the type to load from the pointer. + * @ptr: the pointer to load from. + * + * Use memcpy to affect an unaligned type sized load avoiding undefined behavior + * from approaches like type punning that require -fno-strict-aliasing in order + * to be correct. As type may be const, use __unqual_scalar_typeof to map to a + * non-const type - you can't memcpy into a const type. The + * __get_unaligned_ctrl_type gives __unqual_scalar_typeof its required + * expression rather than type, a pointer is used to avoid warnings about mixing + * the use of 0 and NULL. The void* cast silences ubsan warnings. + */ +#define __get_unaligned_t(type, ptr) ({ \ + type *__get_unaligned_ctrl_type __always_unused = NULL; \ + __unqual_scalar_typeof(*__get_unaligned_ctrl_type) __get_unaligned_val; \ + __builtin_memcpy(&__get_unaligned_val, (void *)(ptr), \ + sizeof(__get_unaligned_val)); \ + __get_unaligned_val; \ }) -#define __put_unaligned_t(type, val, ptr) do { \ - struct { type x; } __packed * __put_pptr = (typeof(__put_pptr))(ptr); \ - __put_pptr->x = (val); \ +/** + * __put_unaligned_t - write an unaligned value to memory. + * @type: the type of the value to store. + * @val: the value to store. + * @ptr: the pointer to store to. + * + * Use memcpy to affect an unaligned type sized store avoiding undefined + * behavior from approaches like type punning that require -fno-strict-aliasing + * in order to be correct. The void* cast silences ubsan warnings. + */ +#define __put_unaligned_t(type, val, ptr) do { \ + type __put_unaligned_val = (val); \ + __builtin_memcpy((void *)(ptr), &__put_unaligned_val, \ + sizeof(__put_unaligned_val)); \ } while (0) #endif /* __VDSO_UNALIGNED_H */ ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v5 4/4] tools headers: Remove unneeded ignoring of warnings in unaligned.h 2025-10-16 20:51 [PATCH v5 0/4] Switch get/put unaligned to use memcpy Ian Rogers ` (2 preceding siblings ...) 2025-10-16 20:51 ` [PATCH v5 3/4] tools headers: Update the linux/unaligned.h copy with the kernel sources Ian Rogers @ 2025-10-16 20:51 ` Ian Rogers 2026-01-13 13:47 ` [tip: timers/vdso] " tip-bot2 for Ian Rogers 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 3 siblings, 2 replies; 36+ messages in thread From: Ian Rogers @ 2025-10-16 20:51 UTC (permalink / raw) To: James E.J. Bottomley, Helge Deller, Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, Ian Rogers, Arnaldo Carvalho de Melo, linux-parisc, linux-kernel, Eric Biggers, Al Viro, Christophe Leroy, Jason A. Donenfeld Now the get/put unaligned use memcpy the -Wpacked and -Wattributes warnings don't need disabling. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/include/linux/unaligned.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tools/include/linux/unaligned.h b/tools/include/linux/unaligned.h index 395a4464fe73..d51ddafed138 100644 --- a/tools/include/linux/unaligned.h +++ b/tools/include/linux/unaligned.h @@ -6,9 +6,6 @@ * This is the most generic implementation of unaligned accesses * and should work almost anywhere. */ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wpacked" -#pragma GCC diagnostic ignored "-Wattributes" #include <vdso/unaligned.h> #define get_unaligned(ptr) __get_unaligned_t(typeof(*(ptr)), (ptr)) @@ -143,6 +140,5 @@ static inline u64 get_unaligned_be48(const void *p) { return __get_unaligned_be48(p); } -#pragma GCC diagnostic pop #endif /* __LINUX_UNALIGNED_H */ -- 2.51.0.858.gf9c4a03a3a-goog ^ permalink raw reply [flat|nested] 36+ messages in thread
* [tip: timers/vdso] tools headers: Remove unneeded ignoring of warnings in unaligned.h 2025-10-16 20:51 ` [PATCH v5 4/4] tools headers: Remove unneeded ignoring of warnings in unaligned.h Ian Rogers @ 2026-01-13 13:47 ` tip-bot2 for Ian Rogers 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 1 sibling, 0 replies; 36+ messages in thread From: tip-bot2 for Ian Rogers @ 2026-01-13 13:47 UTC (permalink / raw) To: linux-tip-commits; +Cc: Ian Rogers, Thomas Gleixner, x86, linux-kernel The following commit has been merged into the timers/vdso branch of tip: Commit-ID: 576d8a7a985dde4f51a4561dddc0d8734494d3de Gitweb: https://git.kernel.org/tip/576d8a7a985dde4f51a4561dddc0d8734494d3de Author: Ian Rogers <irogers@google.com> AuthorDate: Thu, 16 Oct 2025 13:51:26 -07:00 Committer: Thomas Gleixner <tglx@kernel.org> CommitterDate: Tue, 13 Jan 2026 14:46:01 +01:00 tools headers: Remove unneeded ignoring of warnings in unaligned.h Now that get/put_unaligned() use memcpy() the -Wpacked and -Wattributes warnings don't need disabling anymore. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Thomas Gleixner <tglx@kernel.org> Link: https://patch.msgid.link/20251016205126.2882625-5-irogers@google.com --- tools/include/linux/unaligned.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tools/include/linux/unaligned.h b/tools/include/linux/unaligned.h index 395a446..d51ddaf 100644 --- a/tools/include/linux/unaligned.h +++ b/tools/include/linux/unaligned.h @@ -6,9 +6,6 @@ * This is the most generic implementation of unaligned accesses * and should work almost anywhere. */ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wpacked" -#pragma GCC diagnostic ignored "-Wattributes" #include <vdso/unaligned.h> #define get_unaligned(ptr) __get_unaligned_t(typeof(*(ptr)), (ptr)) @@ -143,6 +140,5 @@ static inline u64 get_unaligned_be48(const void *p) { return __get_unaligned_be48(p); } -#pragma GCC diagnostic pop #endif /* __LINUX_UNALIGNED_H */ ^ permalink raw reply [flat|nested] 36+ messages in thread
* [tip: timers/vdso] tools headers: Remove unneeded ignoring of warnings in unaligned.h 2025-10-16 20:51 ` [PATCH v5 4/4] tools headers: Remove unneeded ignoring of warnings in unaligned.h Ian Rogers 2026-01-13 13:47 ` [tip: timers/vdso] " tip-bot2 for Ian Rogers @ 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 1 sibling, 0 replies; 36+ messages in thread From: tip-bot2 for Ian Rogers @ 2026-01-14 8:01 UTC (permalink / raw) To: linux-tip-commits; +Cc: Ian Rogers, Thomas Gleixner, x86, linux-kernel The following commit has been merged into the timers/vdso branch of tip: Commit-ID: 10a62a0611f5544d209446acfde5beb7b27773c7 Gitweb: https://git.kernel.org/tip/10a62a0611f5544d209446acfde5beb7b27773c7 Author: Ian Rogers <irogers@google.com> AuthorDate: Thu, 16 Oct 2025 13:51:26 -07:00 Committer: Thomas Gleixner <tglx@kernel.org> CommitterDate: Wed, 14 Jan 2026 08:56:41 +01:00 tools headers: Remove unneeded ignoring of warnings in unaligned.h Now that get/put_unaligned() use memcpy() the -Wpacked and -Wattributes warnings don't need disabling anymore. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Thomas Gleixner <tglx@kernel.org> Link: https://patch.msgid.link/20251016205126.2882625-5-irogers@google.com --- tools/include/linux/unaligned.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tools/include/linux/unaligned.h b/tools/include/linux/unaligned.h index 395a446..d51ddaf 100644 --- a/tools/include/linux/unaligned.h +++ b/tools/include/linux/unaligned.h @@ -6,9 +6,6 @@ * This is the most generic implementation of unaligned accesses * and should work almost anywhere. */ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wpacked" -#pragma GCC diagnostic ignored "-Wattributes" #include <vdso/unaligned.h> #define get_unaligned(ptr) __get_unaligned_t(typeof(*(ptr)), (ptr)) @@ -143,6 +140,5 @@ static inline u64 get_unaligned_be48(const void *p) { return __get_unaligned_be48(p); } -#pragma GCC diagnostic pop #endif /* __LINUX_UNALIGNED_H */ ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2026-10-20 21:20 UTC | newest] Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-10-16 20:51 [PATCH v5 0/4] Switch get/put unaligned to use memcpy Ian Rogers 2025-10-16 20:51 ` [PATCH v5 1/4] parisc: Inline a type punning version of get_unaligned_le32 Ian Rogers 2026-01-13 13:47 ` [tip: timers/vdso] parisc: Inline a type punning version of get_unaligned_le32() tip-bot2 for Ian Rogers 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 2025-10-16 20:51 ` [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Ian Rogers 2025-10-19 17:24 ` David Laight 2026-01-13 13:47 ` [tip: timers/vdso] vdso: Switch get/put_unaligned() from packed struct to memcpy() tip-bot2 for Ian Rogers 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 2026-09-28 15:39 ` [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy Stefan Kerkmann 2026-09-28 15:58 ` Ian Rogers 2026-10-06 12:31 ` Marc Kleine-Budde 2026-10-07 10:59 ` Arnd Bergmann 2026-10-09 21:28 ` Ian Rogers 2026-10-10 10:26 ` Christophe Leroy (CS GROUP) 2026-10-10 12:20 ` Ian Rogers 2026-10-10 14:06 ` Arnd Bergmann 2026-10-10 15:04 ` Ian Rogers 2026-10-19 12:37 ` Marc Kleine-Budde 2026-10-19 15:10 ` Ian Rogers 2026-10-19 16:27 ` Arnd Bergmann 2026-10-19 17:04 ` Ian Rogers 2026-10-19 21:00 ` Arnd Bergmann 2026-10-19 21:14 ` Ian Rogers 2026-10-19 22:05 ` Arnd Bergmann 2026-10-20 16:21 ` Ian Rogers 2026-10-20 19:27 ` David Laight 2026-10-20 21:20 ` David Laight 2026-10-20 8:36 ` David Laight 2026-10-20 8:41 ` Arnd Bergmann 2026-10-20 10:01 ` David Laight 2025-10-16 20:51 ` [PATCH v5 3/4] tools headers: Update the linux/unaligned.h copy with the kernel sources Ian Rogers 2026-01-13 13:47 ` [tip: timers/vdso] " tip-bot2 for Ian Rogers 2026-01-14 8:01 ` tip-bot2 for Ian Rogers 2025-10-16 20:51 ` [PATCH v5 4/4] tools headers: Remove unneeded ignoring of warnings in unaligned.h Ian Rogers 2026-01-13 13:47 ` [tip: timers/vdso] " tip-bot2 for Ian Rogers 2026-01-14 8:01 ` tip-bot2 for Ian Rogers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®