mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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

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

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

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

* 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] 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: 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] 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] 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] 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

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

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

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

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

* 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

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®