mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Finn Thain <fthain@linux-m68k.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Lance Yang <lance.yang@linux.dev>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Eero Tamminen <oak@helsinkinet.fi>, Will Deacon <will@kernel.org>,
	stable@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] atomic: Specify natural alignment for atomic_t
Date: Mon, 1 Sep 2025 11:36:00 +0200	[thread overview]
Message-ID: <20250901093600.GF4067720@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <10b5aaae-5947-53a9-88bb-802daafd83d4@linux-m68k.org>

On Thu, Aug 28, 2025 at 07:53:52PM +1000, Finn Thain wrote:

> > Anyway, I'm not opposed to adding an explicit alignment to atomic_t.
> > Isn't s32 or __s32 already having this?
> > 
> 
> For Linux/m68k, __alignof__(__s32) == 2 and __alignof__(s32) == 2.

Hmm, somehow I thought one of those enforced natural alignment. Oh well.

> > But I think it might make sense to have a DEBUG alignment check right
> > along with adding that alignment, just to make sure things are indeed /
> > stay aligned.
> > 
> 
> A run-time assertion seems surperfluous as long as other architectures 
> already trap for misaligned locks. 

Right, but those architectures have natural alignment. m68k is 'special'
in that it doesn't have this.

> For m68k, perhaps we could have a compile-time check:

I don't think build-time is sufficient. There is various code that casts
to atomic types and other funny stuff like that.

If you want to ensure 'atomics' are always naturally aligned, the only
sound way is to have a runtime test/trap.

Something like the completely untested below should do I suppose.

---
diff --git a/include/linux/instrumented.h b/include/linux/instrumented.h
index 711a1f0d1a73..e39cdfe5a59e 100644
--- a/include/linux/instrumented.h
+++ b/include/linux/instrumented.h
@@ -67,6 +67,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 & 3));
 }
 
 /**
@@ -81,6 +82,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 & 3));
 }
 
 /**
@@ -95,6 +97,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 & 3));
 }
 
 /**
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

  reply	other threads:[~2025-09-01  9:36 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-25  2:03 Finn Thain
2025-08-25  3:27 ` Lance Yang
2025-08-25  3:59   ` Finn Thain
2025-08-25  4:22     ` Lance Yang
2025-08-25  4:07   ` Finn Thain
2025-08-25  5:00     ` Lance Yang
2025-08-25  6:17       ` Finn Thain
2025-08-25  7:46         ` Lance Yang
2025-08-25 10:49           ` Finn Thain
2025-08-25 11:19             ` Lance Yang
2025-08-25 11:36               ` Lance Yang
2025-08-27 23:43                 ` Finn Thain
2025-08-28  2:05                   ` Lance Yang
2025-09-01  8:45                     ` Geert Uytterhoeven
2025-09-02 13:30                       ` Lance Yang
2025-09-02 14:14                         ` Geert Uytterhoeven
2025-09-06 11:43                       ` David Laight
2025-08-25 12:07           ` David Laight
2025-08-25 12:33             ` Lance Yang
2025-08-27  8:00               ` Finn Thain
2025-08-27  9:34                 ` Lance Yang
2025-09-01  8:48             ` Geert Uytterhoeven
2025-08-25  7:12 ` Peter Zijlstra
2025-08-25  8:03   ` Finn Thain
2025-08-25 11:41     ` Peter Zijlstra
2025-08-27  7:17       ` Finn Thain
2025-08-27 11:54         ` Peter Zijlstra
2025-08-28  9:53           ` Finn Thain
2025-09-01  9:36             ` Peter Zijlstra [this message]
2025-09-01  9:40               ` Peter Zijlstra
2025-09-08  1:30                 ` Finn Thain
2025-08-26 15:22 ` Eero Tamminen
2025-08-26 17:33   ` Lance Yang
2025-09-01  8:51   ` Geert Uytterhoeven
2025-09-01 15:12     ` Eero Tamminen
2025-09-06 11:50       ` David Laight
2025-08-27  2:45 ` Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250901093600.GF4067720@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=fthain@linux-m68k.org \
    --cc=geert@linux-m68k.org \
    --cc=lance.yang@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=oak@helsinkinet.fi \
    --cc=stable@vger.kernel.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®