linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v4 0/5] Align atomic storage
@ 2025-10-20 22:28 Finn Thain
  2025-10-20 22:28 ` [RFC v4 5/5] atomic: Add option for weaker alignment check Finn Thain
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Finn Thain @ 2025-10-20 22:28 UTC (permalink / raw)
  To: James E.J. Bottomley, Helge Deller, Peter Zijlstra, Will Deacon
  Cc: Andrew Morton, Andrii Nakryiko, Arnd Bergmann, Alexei Starovoitov,
	Boqun Feng, bpf, Daniel Borkmann, Geert Uytterhoeven, linux-arch,
	linux-kernel, linux-m68k, linux-parisc, Mark Rutland

This series adds the __aligned attribute to atomic_t and atomic64_t
definitions in include/asm-generic.

It also adds Kconfig options to enable a new runtime warning to help
reveal misaligned atomic accesses on platforms which don't trap that.

This patch series is a Request For Comments because the alignment
change is a time/space tradeoff. Its costs and benefits are expected
to vary across platforms and workloads. More measurements are needed.

---

Changed since v3:
 - Rebased on v6.17.
 - New patch to resolve header dependency issue on parisc.
 - Dropped documentation patch.

Changed since v2:
 - Specify natural alignment for atomic64_t.
 - CONFIG_DEBUG_ATOMIC checks for natural alignment again.
 - New patch to add weakened alignment check.
 - New patch for explicit alignment in BPF header.

---

Finn Thain (4):
  bpf: Explicitly align bpf_res_spin_lock
  parisc: Drop linux/kernel.h include from asm/bug.h header
  atomic: Specify alignment for atomic_t and atomic64_t
  atomic: Add option for weaker alignment check

Peter Zijlstra (1):
  atomic: Add alignment check to instrumented atomic operations

 arch/parisc/include/asm/bug.h    |  2 --
 include/asm-generic/atomic64.h   |  2 +-
 include/asm-generic/rqspinlock.h |  2 +-
 include/linux/instrumented.h     | 15 +++++++++++++++
 include/linux/types.h            |  2 +-
 kernel/bpf/rqspinlock.c          |  1 -
 lib/Kconfig.debug                | 18 ++++++++++++++++++
 7 files changed, 36 insertions(+), 6 deletions(-)

-- 
2.49.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC v4 1/5] bpf: Explicitly align bpf_res_spin_lock
  2025-10-20 22:28 [RFC v4 0/5] Align atomic storage Finn Thain
                   ` (2 preceding siblings ...)
  2025-10-20 22:28 ` [RFC v4 2/5] parisc: Drop linux/kernel.h include from asm/bug.h header Finn Thain
@ 2025-10-20 22:28 ` Finn Thain
  2025-10-20 22:28 ` [RFC v4 3/5] atomic: Specify alignment for atomic_t and atomic64_t Finn Thain
  4 siblings, 0 replies; 9+ messages in thread
From: Finn Thain @ 2025-10-20 22:28 UTC (permalink / raw)
  To: Peter Zijlstra, Will Deacon
  Cc: Andrew Morton, Arnd Bergmann, Boqun Feng, Geert Uytterhoeven,
	linux-arch, linux-kernel, linux-m68k, Mark Rutland,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf

Align bpf_res_spin_lock to avoid a BUILD_BUG_ON() when the alignment
changes, as it will do on m68k when, in a subsequent patch, the minimum
alignment of the atomic_t member of struct rqspinlock gets increased
from 2 to 4. Drop the BUILD_BUG_ON() as it is now redundant.

Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 include/asm-generic/rqspinlock.h | 2 +-
 kernel/bpf/rqspinlock.c          | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/asm-generic/rqspinlock.h b/include/asm-generic/rqspinlock.h
index 6d4244d643df..eac2dcd31b83 100644
--- a/include/asm-generic/rqspinlock.h
+++ b/include/asm-generic/rqspinlock.h
@@ -28,7 +28,7 @@ struct rqspinlock {
  */
 struct bpf_res_spin_lock {
 	u32 val;
-};
+} __aligned(__alignof__(struct rqspinlock));
 
 struct qspinlock;
 #ifdef CONFIG_QUEUED_SPINLOCKS
diff --git a/kernel/bpf/rqspinlock.c b/kernel/bpf/rqspinlock.c
index a00561b1d3e5..02f1f671e624 100644
--- a/kernel/bpf/rqspinlock.c
+++ b/kernel/bpf/rqspinlock.c
@@ -692,7 +692,6 @@ __bpf_kfunc int bpf_res_spin_lock(struct bpf_res_spin_lock *lock)
 	int ret;
 
 	BUILD_BUG_ON(sizeof(rqspinlock_t) != sizeof(struct bpf_res_spin_lock));
-	BUILD_BUG_ON(__alignof__(rqspinlock_t) != __alignof__(struct bpf_res_spin_lock));
 
 	preempt_disable();
 	ret = res_spin_lock((rqspinlock_t *)lock);
-- 
2.49.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC v4 2/5] parisc: Drop linux/kernel.h include from asm/bug.h header
  2025-10-20 22:28 [RFC v4 0/5] Align atomic storage Finn Thain
  2025-10-20 22:28 ` [RFC v4 5/5] atomic: Add option for weaker alignment check Finn Thain
  2025-10-20 22:28 ` [RFC v4 4/5] atomic: Add alignment check to instrumented atomic operations Finn Thain
@ 2025-10-20 22:28 ` Finn Thain
  2025-11-08 22:39   ` Helge Deller
  2025-10-20 22:28 ` [RFC v4 1/5] bpf: Explicitly align bpf_res_spin_lock Finn Thain
  2025-10-20 22:28 ` [RFC v4 3/5] atomic: Specify alignment for atomic_t and atomic64_t Finn Thain
  4 siblings, 1 reply; 9+ messages in thread
From: Finn Thain @ 2025-10-20 22:28 UTC (permalink / raw)
  To: Peter Zijlstra, Will Deacon, James E.J. Bottomley, Helge Deller
  Cc: Andrew Morton, Arnd Bergmann, Boqun Feng, Geert Uytterhoeven,
	linux-arch, linux-kernel, linux-m68k, Mark Rutland, linux-parisc

This patch series will add WARN_ON_ONCE() calls to the header file
linux/instrumented.h. That requires including linux/bug.h,
but doing so causes the following compiler error on parisc:

In file included from ./include/linux/atomic/atomic-instrumented.h:17,
                 from ./include/linux/atomic.h:82,
                 from ./arch/parisc/include/asm/bitops.h:13,
                 from ./include/linux/bitops.h:67,
                 from ./include/linux/kernel.h:23,
                 from ./arch/parisc/include/asm/bug.h:5,
                 from ./include/linux/bug.h:5,
                 from ./include/linux/page-flags.h:10,
                 from kernel/bounds.c:10:
./include/linux/instrumented.h: In function 'instrument_atomic_alignment_check':
./include/linux/instrumented.h:69:9: error: implicit declaration of function 'WARN_ON_ONCE' [-Werror=implicit-function-declaration]
   69 |         WARN_ON_ONCE((unsigned long)v & (size - 1));
      |         ^~~~~~~~~~~~
cc1: some warnings being treated as errors
make[3]: *** [scripts/Makefile.build:182: kernel/bounds.s] Error 1

The problem is, asm/bug.h indirectly includes atomic-instrumented.h,
which means a new cycle appeared in the graph of #includes. And because
some headers in the cycle can't see all definitions, WARN_ON_ONCE()
appears to be an undeclared function.

This only happens on parisc and it's easy to fix. In the error
message above, linux/kernel.h is included by asm/bug.h. But it's no
longer needed there, so remove it.

The comment about needing BUGFLAG_TAINT seems to be incorrect as of
commit 19d436268dde ("debug: Add _ONCE() logic to report_bug()"). Also,
a comment in linux/kernel.h strongly discourages its use here.

Compile-tested only.
---
 arch/parisc/include/asm/bug.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/parisc/include/asm/bug.h b/arch/parisc/include/asm/bug.h
index 833555f74ffa..dbf65623c513 100644
--- a/arch/parisc/include/asm/bug.h
+++ b/arch/parisc/include/asm/bug.h
@@ -2,8 +2,6 @@
 #ifndef _PARISC_BUG_H
 #define _PARISC_BUG_H
 
-#include <linux/kernel.h>	/* for BUGFLAG_TAINT */
-
 /*
  * Tell the user there is some problem.
  * The offending file and line are encoded in the __bug_table section.
-- 
2.49.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC v4 3/5] atomic: Specify alignment for atomic_t and atomic64_t
  2025-10-20 22:28 [RFC v4 0/5] Align atomic storage Finn Thain
                   ` (3 preceding siblings ...)
  2025-10-20 22:28 ` [RFC v4 1/5] bpf: Explicitly align bpf_res_spin_lock Finn Thain
@ 2025-10-20 22:28 ` Finn Thain
  2025-11-24  0:23   ` Daniel Palmer
  4 siblings, 1 reply; 9+ messages in thread
From: Finn Thain @ 2025-10-20 22:28 UTC (permalink / raw)
  To: Peter Zijlstra, Will Deacon
  Cc: Andrew Morton, Arnd Bergmann, Boqun Feng, Geert Uytterhoeven,
	linux-arch, linux-kernel, linux-m68k, Mark Rutland

Some recent commits incorrectly assumed 4-byte alignment of locks.
That assumption fails on Linux/m68k (and, interestingly, would have
failed on Linux/cris also). Specify the minimum alignment of atomic
variables for fewer surprises and (hopefully) better performance.

On an m68k system with 14 MB of RAM, this patch reduces the available
memory by a couple of percent. On a 64 MB system, the cost is under 1%
but still significant. I don't know whether there is sufficient
performance gain to justify the memory cost; it still has to be measured.

Link: https://lore.kernel.org/lkml/CAMuHMdW7Ab13DdGs2acMQcix5ObJK0O2dG_Fxzr8_g58Rc1_0g@mail.gmail.com/
---
Changed since v2:
 - Specify natural alignment for atomic64_t.
Changed since v1:
 - atomic64_t now gets an __aligned attribute too.
 - The 'Fixes' tag has been dropped because Lance sent a different fix
   for commit e711faaafbe5 ("hung_task: replace blocker_mutex with encoded
   blocker") that's suitable for -stable.
---
 include/asm-generic/atomic64.h | 2 +-
 include/linux/types.h          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/atomic64.h b/include/asm-generic/atomic64.h
index 100d24b02e52..f22ccfc0df98 100644
--- a/include/asm-generic/atomic64.h
+++ b/include/asm-generic/atomic64.h
@@ -10,7 +10,7 @@
 #include <linux/types.h>
 
 typedef struct {
-	s64 counter;
+	s64 __aligned(sizeof(s64)) counter;
 } atomic64_t;
 
 #define ATOMIC64_INIT(i)	{ (i) }
diff --git a/include/linux/types.h b/include/linux/types.h
index 6dfdb8e8e4c3..a225a518c2c3 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -179,7 +179,7 @@ typedef phys_addr_t resource_size_t;
 typedef unsigned long irq_hw_number_t;
 
 typedef struct {
-	int counter;
+	int __aligned(sizeof(int)) counter;
 } atomic_t;
 
 #define ATOMIC_INIT(i) { (i) }
-- 
2.49.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC v4 4/5] atomic: Add alignment check to instrumented atomic operations
  2025-10-20 22:28 [RFC v4 0/5] Align atomic storage Finn Thain
  2025-10-20 22:28 ` [RFC v4 5/5] atomic: Add option for weaker alignment check Finn Thain
@ 2025-10-20 22:28 ` Finn Thain
  2025-10-20 22:28 ` [RFC v4 2/5] parisc: Drop linux/kernel.h include from asm/bug.h header Finn Thain
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Finn Thain @ 2025-10-20 22:28 UTC (permalink / raw)
  To: Peter Zijlstra, Will Deacon
  Cc: Andrew Morton, Arnd Bergmann, Boqun Feng, Geert Uytterhoeven,
	linux-arch, linux-kernel, linux-m68k, Mark Rutland

From: Peter Zijlstra <peterz@infradead.org>

Add a Kconfig option for debug builds which logs a warning when an
instrumented atomic operation takes place that's misaligned.
Some platforms don't trap for this.

Link: https://lore.kernel.org/lkml/20250901093600.GF4067720@noisy.programming.kicks-ass.net/
---
Changed since v2:
 - Always check for natural alignment.
---
To make this useful on those architectures that don't naturally align
scalars (x86-32, m68k and sh come to mind), please also use
"[PATCH] atomic: skip alignment check for try_cmpxchg() old arg"
https://lore.kernel.org/all/20251006110740.468309-1-arnd@kernel.org/
---
 include/linux/instrumented.h |  4 ++++
 lib/Kconfig.debug            | 10 ++++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/linux/instrumented.h b/include/linux/instrumented.h
index 711a1f0d1a73..402a999a0d6b 100644
--- a/include/linux/instrumented.h
+++ b/include/linux/instrumented.h
@@ -7,6 +7,7 @@
 #ifndef _LINUX_INSTRUMENTED_H
 #define _LINUX_INSTRUMENTED_H
 
+#include <linux/bug.h>
 #include <linux/compiler.h>
 #include <linux/kasan-checks.h>
 #include <linux/kcsan-checks.h>
@@ -67,6 +68,7 @@ static __always_inline void instrument_atomic_read(const volatile void *v, size_
 {
 	kasan_check_read(v, size);
 	kcsan_check_atomic_read(v, size);
+	WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ATOMIC) && ((unsigned long)v & (size - 1)));
 }
 
 /**
@@ -81,6 +83,7 @@ static __always_inline void instrument_atomic_write(const volatile void *v, size
 {
 	kasan_check_write(v, size);
 	kcsan_check_atomic_write(v, size);
+	WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ATOMIC) && ((unsigned long)v & (size - 1)));
 }
 
 /**
@@ -95,6 +98,7 @@ static __always_inline void instrument_atomic_read_write(const volatile void *v,
 {
 	kasan_check_write(v, size);
 	kcsan_check_atomic_read_write(v, size);
+	WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ATOMIC) && ((unsigned long)v & (size - 1)));
 }
 
 /**
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index dc0e0c6ed075..1c7e30cdfe04 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1363,6 +1363,16 @@ config DEBUG_PREEMPT
 	  depending on workload as it triggers debugging routines for each
 	  this_cpu operation. It should only be used for debugging purposes.
 
+config DEBUG_ATOMIC
+	bool "Debug atomic variables"
+	depends on DEBUG_KERNEL
+	help
+	  If you say Y here then the kernel will add a runtime alignment check
+	  to atomic accesses. Useful for architectures that do not have trap on
+	  mis-aligned access.
+
+	  This option has potentially significant overhead.
+
 menu "Lock Debugging (spinlocks, mutexes, etc...)"
 
 config LOCK_DEBUGGING_SUPPORT
-- 
2.49.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC v4 5/5] atomic: Add option for weaker alignment check
  2025-10-20 22:28 [RFC v4 0/5] Align atomic storage Finn Thain
@ 2025-10-20 22:28 ` Finn Thain
  2025-10-20 22:28 ` [RFC v4 4/5] atomic: Add alignment check to instrumented atomic operations Finn Thain
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Finn Thain @ 2025-10-20 22:28 UTC (permalink / raw)
  To: Peter Zijlstra, Will Deacon
  Cc: Andrew Morton, Arnd Bergmann, Boqun Feng, Geert Uytterhoeven,
	linux-arch, linux-kernel, linux-m68k, Mark Rutland

Add a new Kconfig symbol to make CONFIG_DEBUG_ATOMIC more useful on
those architectures which do not align dynamic allocations to 8-byte
boundaries. Without this, CONFIG_DEBUG_ATOMIC produces excessive
WARN splats.
---
Changed since v3:
 - Dropped #include <linux/cache.h> to avoid a build failure on arm64.
 - Rewrote Kconfig help text to better describe preferred alignment.
 - Refactored to avoid line-wrapping and duplication.
---
 include/linux/instrumented.h | 17 ++++++++++++++---
 lib/Kconfig.debug            |  8 ++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/include/linux/instrumented.h b/include/linux/instrumented.h
index 402a999a0d6b..ca946c5860be 100644
--- a/include/linux/instrumented.h
+++ b/include/linux/instrumented.h
@@ -56,6 +56,17 @@ static __always_inline void instrument_read_write(const volatile void *v, size_t
 	kcsan_check_read_write(v, size);
 }
 
+static __always_inline void instrument_atomic_check_alignment(const volatile void *v, size_t size)
+{
+	if (IS_ENABLED(CONFIG_DEBUG_ATOMIC)) {
+		unsigned int mask = size - 1;
+
+		if (IS_ENABLED(CONFIG_DEBUG_ATOMIC_LARGEST_ALIGN))
+			mask &= sizeof(struct { long x; } __aligned_largest) - 1;
+		WARN_ON_ONCE((unsigned long)v & mask);
+	}
+}
+
 /**
  * instrument_atomic_read - instrument atomic read access
  * @v: address of access
@@ -68,7 +79,7 @@ static __always_inline void instrument_atomic_read(const volatile void *v, size_
 {
 	kasan_check_read(v, size);
 	kcsan_check_atomic_read(v, size);
-	WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ATOMIC) && ((unsigned long)v & (size - 1)));
+	instrument_atomic_check_alignment(v, size);
 }
 
 /**
@@ -83,7 +94,7 @@ static __always_inline void instrument_atomic_write(const volatile void *v, size
 {
 	kasan_check_write(v, size);
 	kcsan_check_atomic_write(v, size);
-	WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ATOMIC) && ((unsigned long)v & (size - 1)));
+	instrument_atomic_check_alignment(v, size);
 }
 
 /**
@@ -98,7 +109,7 @@ static __always_inline void instrument_atomic_read_write(const volatile void *v,
 {
 	kasan_check_write(v, size);
 	kcsan_check_atomic_read_write(v, size);
-	WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ATOMIC) && ((unsigned long)v & (size - 1)));
+	instrument_atomic_check_alignment(v, size);
 }
 
 /**
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1c7e30cdfe04..4d3ebe501d17 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1373,6 +1373,14 @@ config DEBUG_ATOMIC
 
 	  This option has potentially significant overhead.
 
+config DEBUG_ATOMIC_LARGEST_ALIGN
+	bool "Check alignment only up to __aligned_largest"
+	depends on DEBUG_ATOMIC
+	help
+	  If you say Y here then the check for natural alignment of
+	  atomic accesses will be constrained to the compiler's largest
+	  alignment for scalar types.
+
 menu "Lock Debugging (spinlocks, mutexes, etc...)"
 
 config LOCK_DEBUGGING_SUPPORT
-- 
2.49.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC v4 2/5] parisc: Drop linux/kernel.h include from asm/bug.h header
  2025-10-20 22:28 ` [RFC v4 2/5] parisc: Drop linux/kernel.h include from asm/bug.h header Finn Thain
@ 2025-11-08 22:39   ` Helge Deller
  0 siblings, 0 replies; 9+ messages in thread
From: Helge Deller @ 2025-11-08 22:39 UTC (permalink / raw)
  To: Finn Thain; +Cc: linux-arch, linux-kernel, linux-m68k, linux-parisc

On 10/21/25 00:28, Finn Thain wrote:
> This patch series will add WARN_ON_ONCE() calls to the header file
> linux/instrumented.h. That requires including linux/bug.h,
> but doing so causes the following compiler error on parisc:
> 
> In file included from ./include/linux/atomic/atomic-instrumented.h:17,
>                   from ./include/linux/atomic.h:82,
>                   from ./arch/parisc/include/asm/bitops.h:13,
>                   from ./include/linux/bitops.h:67,
>                   from ./include/linux/kernel.h:23,
>                   from ./arch/parisc/include/asm/bug.h:5,
>                   from ./include/linux/bug.h:5,
>                   from ./include/linux/page-flags.h:10,
>                   from kernel/bounds.c:10:
> ./include/linux/instrumented.h: In function 'instrument_atomic_alignment_check':
> ./include/linux/instrumented.h:69:9: error: implicit declaration of function 'WARN_ON_ONCE' [-Werror=implicit-function-declaration]
>     69 |         WARN_ON_ONCE((unsigned long)v & (size - 1));
>        |         ^~~~~~~~~~~~
> cc1: some warnings being treated as errors
> make[3]: *** [scripts/Makefile.build:182: kernel/bounds.s] Error 1
> 
> The problem is, asm/bug.h indirectly includes atomic-instrumented.h,
> which means a new cycle appeared in the graph of #includes. And because
> some headers in the cycle can't see all definitions, WARN_ON_ONCE()
> appears to be an undeclared function.
> 
> This only happens on parisc and it's easy to fix. In the error
> message above, linux/kernel.h is included by asm/bug.h. But it's no
> longer needed there, so remove it.
> 
> The comment about needing BUGFLAG_TAINT seems to be incorrect as of
> commit 19d436268dde ("debug: Add _ONCE() logic to report_bug()"). Also,
> a comment in linux/kernel.h strongly discourages its use here.
> 
> Compile-tested only.
> ---
>   arch/parisc/include/asm/bug.h | 2 --
>   1 file changed, 2 deletions(-)

Acked-by: Helge Deller <deller@gmx.de> # parisc

Thanks!
Helge

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC v4 3/5] atomic: Specify alignment for atomic_t and atomic64_t
  2025-10-20 22:28 ` [RFC v4 3/5] atomic: Specify alignment for atomic_t and atomic64_t Finn Thain
@ 2025-11-24  0:23   ` Daniel Palmer
  2025-11-25  3:52     ` Finn Thain
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Palmer @ 2025-11-24  0:23 UTC (permalink / raw)
  To: Finn Thain
  Cc: Peter Zijlstra, Will Deacon, Andrew Morton, Arnd Bergmann,
	Boqun Feng, Geert Uytterhoeven, linux-arch, linux-kernel,
	linux-m68k, Mark Rutland

Hi Finn,

On Tue, 21 Oct 2025 at 07:39, Finn Thain <fthain@linux-m68k.org> wrote:
>
> Some recent commits incorrectly assumed 4-byte alignment of locks.
> That assumption fails on Linux/m68k (and, interestingly, would have
> failed on Linux/cris also). Specify the minimum alignment of atomic
> variables for fewer surprises and (hopefully) better performance.

FWIW I implemented jump labels for m68k and I think there is a problem
with this in there too.
jump_label_init() calls static_key_set_entries() and setting
key->entries in there is corrupting 'atomic_t enabled' at the start of
key.

With this patch the problem goes away.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC v4 3/5] atomic: Specify alignment for atomic_t and atomic64_t
  2025-11-24  0:23   ` Daniel Palmer
@ 2025-11-25  3:52     ` Finn Thain
  0 siblings, 0 replies; 9+ messages in thread
From: Finn Thain @ 2025-11-25  3:52 UTC (permalink / raw)
  To: Daniel Palmer
  Cc: Peter Zijlstra, Will Deacon, Andrew Morton, Arnd Bergmann,
	Boqun Feng, Geert Uytterhoeven, linux-arch, linux-kernel,
	linux-m68k, Mark Rutland

[-- Attachment #1: Type: text/plain, Size: 2133 bytes --]


On Mon, 24 Nov 2025, Daniel Palmer wrote:

> On Tue, 21 Oct 2025 at 07:39, Finn Thain <fthain@linux-m68k.org> wrote:
> >
> > Some recent commits incorrectly assumed 4-byte alignment of locks.
> > That assumption fails on Linux/m68k (and, interestingly, would have
> > failed on Linux/cris also). Specify the minimum alignment of atomic
> > variables for fewer surprises and (hopefully) better performance.
> 
> FWIW I implemented jump labels for m68k and I think there is a problem
> with this in there too.
> jump_label_init() calls static_key_set_entries() and setting
> key->entries in there is corrupting 'atomic_t enabled' at the start of
> key.
> 
> With this patch the problem goes away.
> 

That's interesting. I wonder whether the alignment requirements of machine 
instructions permitted the "appropriation" of the low bits from those 
pointers...

In anycase, a modified jump label algorithm that did not use/abuse pointer 
bits would need to execute as fast as the existing implementation. And 
that might be quite difficult (especially a portable algorithm).

Recently I had an opportunity to do some performance measurements on m68k 
for this atomic_t alignment patch. I tested some kernel stressors on an 
AWS 95 (33 MHz 68040, 128 MB RAM, 512 KiB L2$) and also on a Mac IIfx (40 
MHz 68030, 80 MB RAM, 32 KiB L2$).

The patch makes the kernel faster or slower, depending the workload. For 
example, the fifo, futex and shm stressors were consistently faster 
whereas the splice, signal and msg stressors were consistently slower.

There are no hardware counters for cache misses that might account for 
part of the slowdown. OTOH, alignment also reduces instances of locks 
split across page boundaries, which might account for the speed-up. (I 
didn't look at VM performance counters.)

Finally, I should note that the stress-ng man page says "do NOT use" as a 
benchmark. OK, well, if anyone wishes to reproduce my results, I can send 
you the statically linked binary I used. The job file is attached.

I wonder whether others have done any throughput measurement for this 
patch, using their favourite workloads?

[-- Attachment #2: Type: text/plain, Size: 1242 bytes --]

run sequential
metrics-brief
timeout 180s
no-rand-seed
oomable
temp-path /tmp

clone 1
clone-ops 4

dentry 1
dentry-ops 8192

#dev 1
#dev-ops 300

dev-shm 1
dev-shm-ops 20

dnotify 1
dnotify-ops 1200

fault 1
fault-ops 8000

fifo 1
fifo-ops 24000

file-ioctl 1
file-ioctl-ops 20000

futex 1
futex-ops 40000

get 1
get-ops 3000

getdent 1
getdent-ops 10000

icmp-flood 1
icmp-flood-ops 40000

inotify 1
inotify-ops 400

ioprio 1
ioprio-ops 8000

kill 1
kill-ops 150000

memfd 1
memfd-bytes 32m
memfd-ops 8

mmapfork 1
mmapfork-ops 4

msg 1
msg-ops 300000

nop 1
nop-ops 3000

poll 1
poll-ops 8000

ptrace 1
ptrace-ops 50000

pty 1
pty-ops 2

rawpkt 1
rawpkt-ops 80000

rawudp 1
rawudp-ops 15000

resources 1
resources-ops 300

revio 1
revio-ops 50000

seek 1
seek-ops 12000

#sem 1
#sem-ops 4000

sem-sysv 1
sem-sysv-ops 300000

sendfile 1
sendfile-ops 1500

set 1
set-ops 20000

shm 1
shm-ops 15

sigchld 1
sigchld-ops 5000

signal 1
signal-ops 150000

sigsegv 1
sigsegv-ops 100000

sock 1
sock-ops 50

splice 1
splice-ops 10000

tee 1
tee-ops 1500

udp 1
udp-ops 30000

utime 1
utime-ops 4000

vm 1
vm-ops 2500

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-11-25  3:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-20 22:28 [RFC v4 0/5] Align atomic storage Finn Thain
2025-10-20 22:28 ` [RFC v4 5/5] atomic: Add option for weaker alignment check Finn Thain
2025-10-20 22:28 ` [RFC v4 4/5] atomic: Add alignment check to instrumented atomic operations Finn Thain
2025-10-20 22:28 ` [RFC v4 2/5] parisc: Drop linux/kernel.h include from asm/bug.h header Finn Thain
2025-11-08 22:39   ` Helge Deller
2025-10-20 22:28 ` [RFC v4 1/5] bpf: Explicitly align bpf_res_spin_lock Finn Thain
2025-10-20 22:28 ` [RFC v4 3/5] atomic: Specify alignment for atomic_t and atomic64_t Finn Thain
2025-11-24  0:23   ` Daniel Palmer
2025-11-25  3:52     ` Finn Thain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).