[PATCH] fork: Honor task_struct's declared alignment

[PATCH] fork: Honor task_struct's declared alignment

[Karl Mehltretter]

Since commit cb7ca40a3882 ("x86/fpu: Make task_struct::thread constant
size"), struct task_struct is declared __attribute__((aligned(64))) on
all architectures.

But fork_init() still sets the task_struct slab cache's alignment to

	align = max(L1_CACHE_BYTES, ARCH_MIN_TASKALIGN)

which is smaller than 64 on architectures whose cache lines are below
64 bytes: e.g. 32 on ARMv5.

In practice plain SLUB happens to hand out 64-byte-aligned objects
anyway. With CONFIG_SLUB_DEBUG_ON the red-zone padding shifts objects
to the requested alignment.

With CONFIG_UBSAN_ALIGNMENT=y a boot on QEMU versatilepb (ARM926EJ-S,
v7.2-rc2, gcc 13.3) floods the console with reports like:

  UBSAN: misaligned-access in include/linux/sched.h:2087:9
  member access within misaligned address c295d7e0 for type 'struct task_struct'
  which requires 64 byte alignment
  CPU: 0 UID: 0 PID: 15 Comm: pr/ttyAMA-1 Not tainted 7.2.0-rc2 #1 VOLUNTARY

Set the slab alignment to at least the type's declared alignment.

Fixes: cb7ca40a3882 ("x86/fpu: Make task_struct::thread constant size")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 kernel/fork.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index f0e2e131a9a5..b85cf1343329 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -857,7 +857,8 @@ void __init fork_init(void)
 #ifndef ARCH_MIN_TASKALIGN
 #define ARCH_MIN_TASKALIGN	0
 #endif
-	int align = max_t(int, L1_CACHE_BYTES, ARCH_MIN_TASKALIGN);
+	int align = max3(L1_CACHE_BYTES, ARCH_MIN_TASKALIGN,
+			 __alignof__(struct task_struct));
 	unsigned long useroffset, usersize;
 
 	/* create a slab on which task_structs can be allocated */
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] fork: Honor task_struct's declared alignment

[Bradley Morgan]

> Since commit cb7ca40a3882 ("x86/fpu: Make task_struct::thread constant
> size"), struct task_struct is declared __attribute__((aligned(64))) on
> all architectures.
>
> But fork_init() still sets the task_struct slab cache's alignment to
>
> 	align = max(L1_CACHE_BYTES, ARCH_MIN_TASKALIGN)
>
> which is smaller than 64 on architectures whose cache lines are below
> 64 bytes: e.g. 32 on ARMv5.
>
> In practice plain SLUB happens to hand out 64-byte-aligned objects
> anyway. With CONFIG_SLUB_DEBUG_ON the red-zone padding shifts objects
> to the requested alignment.
>
> With CONFIG_UBSAN_ALIGNMENT=y a boot on QEMU versatilepb (ARM926EJ-S,
> v7.2-rc2, gcc 13.3) floods the console with reports like:
>
>   UBSAN: misaligned-access in include/linux/sched.h:2087:9
>   member access within misaligned address c295d7e0 for type 'struct task_struct'
>   which requires 64 byte alignment
>   CPU: 0 UID: 0 PID: 15 Comm: pr/ttyAMA-1 Not tainted 7.2.0-rc2 #1 VOLUNTARY
>
> Set the slab alignment to at least the type's declared alignment.
>
> Fixes: cb7ca40a3882 ("x86/fpu: Make task_struct::thread constant size")

Nice!

Please add 

Reviewed-by: Bradley Morgan <include@grrlz.net>

> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
>  kernel/fork.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index f0e2e131a9a5..b85cf1343329 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -857,7 +857,8 @@ void __init fork_init(void)
>  #ifndef ARCH_MIN_TASKALIGN
>  #define ARCH_MIN_TASKALIGN	0
>  #endif
> -	int align = max_t(int, L1_CACHE_BYTES, ARCH_MIN_TASKALIGN);
> +	int align = max3(L1_CACHE_BYTES, ARCH_MIN_TASKALIGN,
> +			 __alignof__(struct task_struct));
>  	unsigned long useroffset, usersize;
>  
>  	/* create a slab on which task_structs can be allocated */
> -- 
> 2.39.5 (Apple Git-154)

Thanks!

Re: [PATCH] fork: Honor task_struct's declared alignment

[Vlastimil Babka (SUSE)]

On 7/10/26 14:39, Karl Mehltretter wrote:
> Since commit cb7ca40a3882 ("x86/fpu: Make task_struct::thread constant
> size"), struct task_struct is declared __attribute__((aligned(64))) on
> all architectures.
> 
> But fork_init() still sets the task_struct slab cache's alignment to
> 
> 	align = max(L1_CACHE_BYTES, ARCH_MIN_TASKALIGN)
> 
> which is smaller than 64 on architectures whose cache lines are below
> 64 bytes: e.g. 32 on ARMv5.
> 
> In practice plain SLUB happens to hand out 64-byte-aligned objects
> anyway. With CONFIG_SLUB_DEBUG_ON the red-zone padding shifts objects
> to the requested alignment.
> 
> With CONFIG_UBSAN_ALIGNMENT=y a boot on QEMU versatilepb (ARM926EJ-S,
> v7.2-rc2, gcc 13.3) floods the console with reports like:
> 
>   UBSAN: misaligned-access in include/linux/sched.h:2087:9
>   member access within misaligned address c295d7e0 for type 'struct task_struct'
>   which requires 64 byte alignment
>   CPU: 0 UID: 0 PID: 15 Comm: pr/ttyAMA-1 Not tainted 7.2.0-rc2 #1 VOLUNTARY
> 
> Set the slab alignment to at least the type's declared alignment.
> 
> Fixes: cb7ca40a3882 ("x86/fpu: Make task_struct::thread constant size")
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>

LGTM. Additionally I think you can replace the manual L1_CACHE_BYTES
handling with a SLAB_HWCACHE_ALIGN flag. That will apply cache_line_size().

> ---
>  kernel/fork.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index f0e2e131a9a5..b85cf1343329 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -857,7 +857,8 @@ void __init fork_init(void)
>  #ifndef ARCH_MIN_TASKALIGN
>  #define ARCH_MIN_TASKALIGN	0
>  #endif
> -	int align = max_t(int, L1_CACHE_BYTES, ARCH_MIN_TASKALIGN);
> +	int align = max3(L1_CACHE_BYTES, ARCH_MIN_TASKALIGN,
> +			 __alignof__(struct task_struct));
>  	unsigned long useroffset, usersize;
>  
>  	/* create a slab on which task_structs can be allocated */