linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/3] hung_task: extend blocking task stacktrace dump to semaphore
@ 2025-04-14 14:59 Lance Yang
  2025-04-14 14:59 ` [PATCH v5 1/3] hung_task: replace blocker_mutex with encoded blocker Lance Yang
                   ` (3 more replies)
  0 siblings, 4 replies; 27+ messages in thread
From: Lance Yang @ 2025-04-14 14:59 UTC (permalink / raw)
  To: akpm
  Cc: will, peterz, mingo, longman, mhiramat, anna.schumaker,
	boqun.feng, joel.granados, kent.overstreet, leonylgao,
	linux-kernel, rostedt, senozhatsky, tfiga, amaindex, jstultz,
	Lance Yang

Hi all,

Inspired by mutex blocker tracking[1], this patch series extend the
feature to not only dump the blocker task holding a mutex but also to
support semaphores. Unlike mutexes, semaphores lack explicit ownership
tracking, making it challenging to identify the root cause of hangs. To
address this, we introduce a last_holder field to the semaphore structure,
which is updated when a task successfully calls down() and cleared during
up().

The assumption is that if a task is blocked on a semaphore, the holders
must not have released it. While this does not guarantee that the last
holder is one of the current blockers, it likely provides a practical hint
for diagnosing semaphore-related stalls.

With this change, the hung task detector can now show blocker task's info
like below:

[Tue Apr  8 12:19:07 2025] INFO: task cat:945 blocked for more than 120 seconds.
[Tue Apr  8 12:19:07 2025]       Tainted: G            E      6.14.0-rc6+ #1
[Tue Apr  8 12:19:07 2025] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[Tue Apr  8 12:19:07 2025] task:cat             state:D stack:0     pid:945   tgid:945   ppid:828    task_flags:0x400000 flags:0x00000000
[Tue Apr  8 12:19:07 2025] Call Trace:
[Tue Apr  8 12:19:07 2025]  <TASK>
[Tue Apr  8 12:19:07 2025]  __schedule+0x491/0xbd0
[Tue Apr  8 12:19:07 2025]  schedule+0x27/0xf0
[Tue Apr  8 12:19:07 2025]  schedule_timeout+0xe3/0xf0
[Tue Apr  8 12:19:07 2025]  ? __folio_mod_stat+0x2a/0x80
[Tue Apr  8 12:19:07 2025]  ? set_ptes.constprop.0+0x27/0x90
[Tue Apr  8 12:19:07 2025]  __down_common+0x155/0x280
[Tue Apr  8 12:19:07 2025]  down+0x53/0x70
[Tue Apr  8 12:19:07 2025]  read_dummy_semaphore+0x23/0x60
[Tue Apr  8 12:19:07 2025]  full_proxy_read+0x5f/0xa0
[Tue Apr  8 12:19:07 2025]  vfs_read+0xbc/0x350
[Tue Apr  8 12:19:07 2025]  ? __count_memcg_events+0xa5/0x140
[Tue Apr  8 12:19:07 2025]  ? count_memcg_events.constprop.0+0x1a/0x30
[Tue Apr  8 12:19:07 2025]  ? handle_mm_fault+0x180/0x260
[Tue Apr  8 12:19:07 2025]  ksys_read+0x66/0xe0
[Tue Apr  8 12:19:07 2025]  do_syscall_64+0x51/0x120
[Tue Apr  8 12:19:07 2025]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[Tue Apr  8 12:19:07 2025] RIP: 0033:0x7f419478f46e
[Tue Apr  8 12:19:07 2025] RSP: 002b:00007fff1c4d2668 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
[Tue Apr  8 12:19:07 2025] RAX: ffffffffffffffda RBX: 0000000000020000 RCX: 00007f419478f46e
[Tue Apr  8 12:19:07 2025] RDX: 0000000000020000 RSI: 00007f4194683000 RDI: 0000000000000003
[Tue Apr  8 12:19:07 2025] RBP: 00007f4194683000 R08: 00007f4194682010 R09: 0000000000000000
[Tue Apr  8 12:19:07 2025] R10: fffffffffffffbc5 R11: 0000000000000246 R12: 0000000000000000
[Tue Apr  8 12:19:07 2025] R13: 0000000000000003 R14: 0000000000020000 R15: 0000000000020000
[Tue Apr  8 12:19:07 2025]  </TASK>
[Tue Apr  8 12:19:07 2025] INFO: task cat:945 blocked on a semaphore likely last held by task cat:938
[Tue Apr  8 12:19:07 2025] task:cat             state:S stack:0     pid:938   tgid:938   ppid:584    task_flags:0x400000 flags:0x00000000
[Tue Apr  8 12:19:07 2025] Call Trace:
[Tue Apr  8 12:19:07 2025]  <TASK>
[Tue Apr  8 12:19:07 2025]  __schedule+0x491/0xbd0
[Tue Apr  8 12:19:07 2025]  ? _raw_spin_unlock_irqrestore+0xe/0x40
[Tue Apr  8 12:19:07 2025]  schedule+0x27/0xf0
[Tue Apr  8 12:19:07 2025]  schedule_timeout+0x77/0xf0
[Tue Apr  8 12:19:07 2025]  ? __pfx_process_timeout+0x10/0x10
[Tue Apr  8 12:19:07 2025]  msleep_interruptible+0x49/0x60
[Tue Apr  8 12:19:07 2025]  read_dummy_semaphore+0x2d/0x60
[Tue Apr  8 12:19:07 2025]  full_proxy_read+0x5f/0xa0
[Tue Apr  8 12:19:07 2025]  vfs_read+0xbc/0x350
[Tue Apr  8 12:19:07 2025]  ? __count_memcg_events+0xa5/0x140
[Tue Apr  8 12:19:07 2025]  ? count_memcg_events.constprop.0+0x1a/0x30
[Tue Apr  8 12:19:07 2025]  ? handle_mm_fault+0x180/0x260
[Tue Apr  8 12:19:07 2025]  ksys_read+0x66/0xe0
[Tue Apr  8 12:19:07 2025]  do_syscall_64+0x51/0x120
[Tue Apr  8 12:19:07 2025]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[Tue Apr  8 12:19:07 2025] RIP: 0033:0x7f7c584a646e
[Tue Apr  8 12:19:07 2025] RSP: 002b:00007ffdba8ce158 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
[Tue Apr  8 12:19:07 2025] RAX: ffffffffffffffda RBX: 0000000000020000 RCX: 00007f7c584a646e
[Tue Apr  8 12:19:07 2025] RDX: 0000000000020000 RSI: 00007f7c5839a000 RDI: 0000000000000003
[Tue Apr  8 12:19:07 2025] RBP: 00007f7c5839a000 R08: 00007f7c58399010 R09: 0000000000000000
[Tue Apr  8 12:19:07 2025] R10: fffffffffffffbc5 R11: 0000000000000246 R12: 0000000000000000
[Tue Apr  8 12:19:07 2025] R13: 0000000000000003 R14: 0000000000020000 R15: 0000000000020000
[Tue Apr  8 12:19:07 2025]  </TASK>

[1] https://lore.kernel.org/all/174046694331.2194069.15472952050240807469.stgit@mhiramat.tok.corp.google.com

Thanks,
Lance

---
v4 -> v5:
 * #01 Add comments for the blocker field, suggested by Andrew
 * #02 Reduce unnecessary #ifdef directives, suggested by Andrew
 * https://lore.kernel.org/all/20250320064923.24000-1-ioworker0@gmail.com

v3 -> v4:
 * #01 #02 Pick RB from Masami - thanks!
 * #03 Pick AB from Masami - thanks!
 * Extract the type from the blocker and use a switch-case instead of
if-else, suggested by Masami
 * https://lore.kernel.org/all/20250319081138.25133-1-ioworker0@gmail.com

v2 -> v3:
 * Remove the unnecessary WARN_ON_ONCE check for 'current->blocker',
 suggested by Masami
 * Drop the redundant #ifdef for including the hung task header file,
 suggested by Masam
 * Unify the samples into 'hung_task_tests.c', suggested by Masami
 * https://lore.kernel.org/all/20250314144300.32542-1-ioworker0@gmail.com

v1 -> v2:
 * Use one field to store the blocker as only one is active at a time,
 suggested by Masami
 * Leverage the LSB of the blocker field to reduce memory footprint,
 suggested by Masami
 * Add a hung_task detector semaphore blocking test sample code
 * https://lore.kernel.org/all/20250301055102.88746-1-ioworker0@gmail.com

Lance Yang (2):
  hung_task: replace blocker_mutex with encoded blocker
  hung_task: show the blocker task if the task is hung on semaphore

Zi Li (1):
  samples: extend hung_task detector test with semaphore support

 include/linux/hung_task.h           | 99 +++++++++++++++++++++++++++++
 include/linux/sched.h               |  6 +-
 include/linux/semaphore.h           | 15 ++++-
 kernel/hung_task.c                  | 55 ++++++++++++----
 kernel/locking/mutex.c              |  5 +-
 kernel/locking/semaphore.c          | 57 +++++++++++++++--
 samples/Kconfig                     |  9 +--
 samples/hung_task/Makefile          |  2 +-
 samples/hung_task/hung_task_mutex.c | 66 -------------------
 samples/hung_task/hung_task_tests.c | 97 ++++++++++++++++++++++++++++
 10 files changed, 319 insertions(+), 92 deletions(-)
 create mode 100644 include/linux/hung_task.h
 delete mode 100644 samples/hung_task/hung_task_mutex.c
 create mode 100644 samples/hung_task/hung_task_tests.c

-- 
2.49.0


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

* [PATCH v5 1/3] hung_task: replace blocker_mutex with encoded blocker
  2025-04-14 14:59 [PATCH v5 0/3] hung_task: extend blocking task stacktrace dump to semaphore Lance Yang
@ 2025-04-14 14:59 ` Lance Yang
  2025-04-14 21:36   ` Andrew Morton
  2025-04-14 14:59 ` [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore Lance Yang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 27+ messages in thread
From: Lance Yang @ 2025-04-14 14:59 UTC (permalink / raw)
  To: akpm
  Cc: will, peterz, mingo, longman, mhiramat, anna.schumaker,
	boqun.feng, joel.granados, kent.overstreet, leonylgao,
	linux-kernel, rostedt, senozhatsky, tfiga, amaindex, jstultz,
	Lance Yang, Mingzhe Yang

This patch replaces 'struct mutex *blocker_mutex' with 'unsigned long
blocker', as only one blocker is active at a time.

The blocker filed can store both the lock addrees and the lock type, with
LSB used to encode the type as Masami suggested, making it easier to extend
the feature to cover other types of locks.

Also, once the lock type is determined, we can directly extract the address
and cast it to a lock pointer ;)

Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Mingzhe Yang <mingzhe.yang@ly.com>
Signed-off-by: Lance Yang <ioworker0@gmail.com>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 include/linux/hung_task.h | 99 +++++++++++++++++++++++++++++++++++++++
 include/linux/sched.h     |  6 ++-
 kernel/hung_task.c        | 13 +++--
 kernel/locking/mutex.c    |  5 +-
 4 files changed, 115 insertions(+), 8 deletions(-)
 create mode 100644 include/linux/hung_task.h

diff --git a/include/linux/hung_task.h b/include/linux/hung_task.h
new file mode 100644
index 000000000000..1bc2b3244613
--- /dev/null
+++ b/include/linux/hung_task.h
@@ -0,0 +1,99 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Detect Hung Task: detecting tasks stuck in D state
+ *
+ * Copyright (C) 2025 Tongcheng Travel (www.ly.com)
+ * Author: Lance Yang <mingzhe.yang@ly.com>
+ */
+#ifndef __LINUX_HUNG_TASK_H
+#define __LINUX_HUNG_TASK_H
+
+#include <linux/bug.h>
+#include <linux/sched.h>
+#include <linux/compiler.h>
+
+/*
+ * @blocker: Combines lock address and blocking type.
+ *
+ * Since lock pointers are at least 4-byte aligned(32-bit) or 8-byte
+ * aligned(64-bit). This leaves the 2 least bits (LSBs) of the pointer
+ * always zero. So we can use these bits to encode the specific blocking
+ * type.
+ *
+ * Type encoding:
+ * 00 - Blocked on mutex        (BLOCKER_TYPE_MUTEX)
+ * 01 - Blocked on semaphore    (BLOCKER_TYPE_SEM)
+ * 10 - Blocked on rt-mutex     (BLOCKER_TYPE_RTMUTEX)
+ * 11 - Blocked on rw-semaphore (BLOCKER_TYPE_RWSEM)
+ */
+#define BLOCKER_TYPE_MUTEX      0x00UL
+#define BLOCKER_TYPE_SEM        0x01UL
+#define BLOCKER_TYPE_RTMUTEX    0x02UL
+#define BLOCKER_TYPE_RWSEM      0x03UL
+
+#define BLOCKER_TYPE_MASK       0x03UL
+
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+static inline void hung_task_set_blocker(void *lock, unsigned long type)
+{
+	unsigned long lock_ptr = (unsigned long)lock;
+
+	WARN_ON_ONCE(!lock_ptr);
+	WARN_ON_ONCE(READ_ONCE(current->blocker));
+
+	/*
+	 * If the lock pointer matches the BLOCKER_TYPE_MASK, return
+	 * without writing anything.
+	 */
+	if (WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK))
+		return;
+
+	WRITE_ONCE(current->blocker, lock_ptr | type);
+}
+
+static inline void hung_task_clear_blocker(void)
+{
+	WARN_ON_ONCE(!READ_ONCE(current->blocker));
+
+	WRITE_ONCE(current->blocker, 0UL);
+}
+
+/*
+ * hung_task_get_blocker_type - Extracts blocker type from encoded blocker
+ * address.
+ *
+ * @blocker: Blocker pointer with encoded type (via LSB bits)
+ *
+ * Returns: BLOCKER_TYPE_MUTEX, BLOCKER_TYPE_SEM, etc.
+ */
+static inline unsigned long hung_task_get_blocker_type(unsigned long blocker)
+{
+	WARN_ON_ONCE(!blocker);
+
+	return blocker & BLOCKER_TYPE_MASK;
+}
+
+static inline void *hung_task_blocker_to_lock(unsigned long blocker)
+{
+	WARN_ON_ONCE(!blocker);
+
+	return (void *)(blocker & ~BLOCKER_TYPE_MASK);
+}
+#else
+static inline void hung_task_set_blocker(void *lock, unsigned long type)
+{
+}
+static inline void hung_task_clear_blocker(void)
+{
+}
+static inline unsigned long hung_task_get_blocker_type(unsigned long blocker)
+{
+	return 0UL;
+}
+static inline void *hung_task_blocker_to_lock(unsigned long blocker)
+{
+	return NULL;
+}
+#endif
+
+#endif /* __LINUX_HUNG_TASK_H */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 1419d94c8e87..27dad9aa99a0 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1218,7 +1218,11 @@ struct task_struct {
 #endif
 
 #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
-	struct mutex			*blocker_mutex;
+	/*
+	 * Encoded lock address causing task block (lower 2 bits = type from
+	 * <linux/hung_task.h>). Accessed via hung_task_*() helpers.
+	 */
+	unsigned long			blocker;
 #endif
 
 #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index dc898ec93463..79558d76ef06 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -22,6 +22,7 @@
 #include <linux/sched/signal.h>
 #include <linux/sched/debug.h>
 #include <linux/sched/sysctl.h>
+#include <linux/hung_task.h>
 
 #include <trace/events/sched.h>
 
@@ -98,16 +99,18 @@ static struct notifier_block panic_block = {
 static void debug_show_blocker(struct task_struct *task)
 {
 	struct task_struct *g, *t;
-	unsigned long owner;
-	struct mutex *lock;
+	unsigned long owner, blocker;
 
 	RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "No rcu lock held");
 
-	lock = READ_ONCE(task->blocker_mutex);
-	if (!lock)
+	blocker = READ_ONCE(task->blocker);
+	if (!blocker ||
+	    hung_task_get_blocker_type(blocker) != BLOCKER_TYPE_MUTEX)
 		return;
 
-	owner = mutex_get_owner(lock);
+	owner = mutex_get_owner(
+		(struct mutex *)hung_task_blocker_to_lock(blocker));
+
 	if (unlikely(!owner)) {
 		pr_err("INFO: task %s:%d is blocked on a mutex, but the owner is not found.\n",
 			task->comm, task->pid);
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 6a543c204a14..e9ef70a6cb5f 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -29,6 +29,7 @@
 #include <linux/interrupt.h>
 #include <linux/debug_locks.h>
 #include <linux/osq_lock.h>
+#include <linux/hung_task.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/lock.h>
@@ -189,7 +190,7 @@ __mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
 		   struct list_head *list)
 {
 #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
-	WRITE_ONCE(current->blocker_mutex, lock);
+	hung_task_set_blocker(lock, BLOCKER_TYPE_MUTEX);
 #endif
 	debug_mutex_add_waiter(lock, waiter, current);
 
@@ -207,7 +208,7 @@ __mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter)
 
 	debug_mutex_remove_waiter(lock, waiter, current);
 #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
-	WRITE_ONCE(current->blocker_mutex, NULL);
+	hung_task_clear_blocker();
 #endif
 }
 
-- 
2.49.0


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

* [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore
  2025-04-14 14:59 [PATCH v5 0/3] hung_task: extend blocking task stacktrace dump to semaphore Lance Yang
  2025-04-14 14:59 ` [PATCH v5 1/3] hung_task: replace blocker_mutex with encoded blocker Lance Yang
@ 2025-04-14 14:59 ` Lance Yang
  2025-08-22  7:38   ` Geert Uytterhoeven
  2025-04-14 14:59 ` [PATCH v5 3/3] samples: extend hung_task detector test with semaphore support Lance Yang
  2025-04-14 21:38 ` [PATCH v5 0/3] hung_task: extend blocking task stacktrace dump to semaphore Andrew Morton
  3 siblings, 1 reply; 27+ messages in thread
From: Lance Yang @ 2025-04-14 14:59 UTC (permalink / raw)
  To: akpm
  Cc: will, peterz, mingo, longman, mhiramat, anna.schumaker,
	boqun.feng, joel.granados, kent.overstreet, leonylgao,
	linux-kernel, rostedt, senozhatsky, tfiga, amaindex, jstultz,
	Lance Yang, Mingzhe Yang

Inspired by mutex blocker tracking[1], this patch makes a trade-off to
balance the overhead and utility of the hung task detector.

Unlike mutexes, semaphores lack explicit ownership tracking, making it
challenging to identify the root cause of hangs. To address this, we
introduce a last_holder field to the semaphore structure, which is
updated when a task successfully calls down() and cleared during up().

The assumption is that if a task is blocked on a semaphore, the holders
must not have released it. While this does not guarantee that the last
holder is one of the current blockers, it likely provides a practical hint
for diagnosing semaphore-related stalls.

With this change, the hung task detector can now show blocker task's info
like below:

[Tue Apr  8 12:19:07 2025] INFO: task cat:945 blocked for more than 120 seconds.
[Tue Apr  8 12:19:07 2025]       Tainted: G            E      6.14.0-rc6+ #1
[Tue Apr  8 12:19:07 2025] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[Tue Apr  8 12:19:07 2025] task:cat             state:D stack:0     pid:945   tgid:945   ppid:828    task_flags:0x400000 flags:0x00000000
[Tue Apr  8 12:19:07 2025] Call Trace:
[Tue Apr  8 12:19:07 2025]  <TASK>
[Tue Apr  8 12:19:07 2025]  __schedule+0x491/0xbd0
[Tue Apr  8 12:19:07 2025]  schedule+0x27/0xf0
[Tue Apr  8 12:19:07 2025]  schedule_timeout+0xe3/0xf0
[Tue Apr  8 12:19:07 2025]  ? __folio_mod_stat+0x2a/0x80
[Tue Apr  8 12:19:07 2025]  ? set_ptes.constprop.0+0x27/0x90
[Tue Apr  8 12:19:07 2025]  __down_common+0x155/0x280
[Tue Apr  8 12:19:07 2025]  down+0x53/0x70
[Tue Apr  8 12:19:07 2025]  read_dummy_semaphore+0x23/0x60
[Tue Apr  8 12:19:07 2025]  full_proxy_read+0x5f/0xa0
[Tue Apr  8 12:19:07 2025]  vfs_read+0xbc/0x350
[Tue Apr  8 12:19:07 2025]  ? __count_memcg_events+0xa5/0x140
[Tue Apr  8 12:19:07 2025]  ? count_memcg_events.constprop.0+0x1a/0x30
[Tue Apr  8 12:19:07 2025]  ? handle_mm_fault+0x180/0x260
[Tue Apr  8 12:19:07 2025]  ksys_read+0x66/0xe0
[Tue Apr  8 12:19:07 2025]  do_syscall_64+0x51/0x120
[Tue Apr  8 12:19:07 2025]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[Tue Apr  8 12:19:07 2025] RIP: 0033:0x7f419478f46e
[Tue Apr  8 12:19:07 2025] RSP: 002b:00007fff1c4d2668 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
[Tue Apr  8 12:19:07 2025] RAX: ffffffffffffffda RBX: 0000000000020000 RCX: 00007f419478f46e
[Tue Apr  8 12:19:07 2025] RDX: 0000000000020000 RSI: 00007f4194683000 RDI: 0000000000000003
[Tue Apr  8 12:19:07 2025] RBP: 00007f4194683000 R08: 00007f4194682010 R09: 0000000000000000
[Tue Apr  8 12:19:07 2025] R10: fffffffffffffbc5 R11: 0000000000000246 R12: 0000000000000000
[Tue Apr  8 12:19:07 2025] R13: 0000000000000003 R14: 0000000000020000 R15: 0000000000020000
[Tue Apr  8 12:19:07 2025]  </TASK>
[Tue Apr  8 12:19:07 2025] INFO: task cat:945 blocked on a semaphore likely last held by task cat:938
[Tue Apr  8 12:19:07 2025] task:cat             state:S stack:0     pid:938   tgid:938   ppid:584    task_flags:0x400000 flags:0x00000000
[Tue Apr  8 12:19:07 2025] Call Trace:
[Tue Apr  8 12:19:07 2025]  <TASK>
[Tue Apr  8 12:19:07 2025]  __schedule+0x491/0xbd0
[Tue Apr  8 12:19:07 2025]  ? _raw_spin_unlock_irqrestore+0xe/0x40
[Tue Apr  8 12:19:07 2025]  schedule+0x27/0xf0
[Tue Apr  8 12:19:07 2025]  schedule_timeout+0x77/0xf0
[Tue Apr  8 12:19:07 2025]  ? __pfx_process_timeout+0x10/0x10
[Tue Apr  8 12:19:07 2025]  msleep_interruptible+0x49/0x60
[Tue Apr  8 12:19:07 2025]  read_dummy_semaphore+0x2d/0x60
[Tue Apr  8 12:19:07 2025]  full_proxy_read+0x5f/0xa0
[Tue Apr  8 12:19:07 2025]  vfs_read+0xbc/0x350
[Tue Apr  8 12:19:07 2025]  ? __count_memcg_events+0xa5/0x140
[Tue Apr  8 12:19:07 2025]  ? count_memcg_events.constprop.0+0x1a/0x30
[Tue Apr  8 12:19:07 2025]  ? handle_mm_fault+0x180/0x260
[Tue Apr  8 12:19:07 2025]  ksys_read+0x66/0xe0
[Tue Apr  8 12:19:07 2025]  do_syscall_64+0x51/0x120
[Tue Apr  8 12:19:07 2025]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[Tue Apr  8 12:19:07 2025] RIP: 0033:0x7f7c584a646e
[Tue Apr  8 12:19:07 2025] RSP: 002b:00007ffdba8ce158 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
[Tue Apr  8 12:19:07 2025] RAX: ffffffffffffffda RBX: 0000000000020000 RCX: 00007f7c584a646e
[Tue Apr  8 12:19:07 2025] RDX: 0000000000020000 RSI: 00007f7c5839a000 RDI: 0000000000000003
[Tue Apr  8 12:19:07 2025] RBP: 00007f7c5839a000 R08: 00007f7c58399010 R09: 0000000000000000
[Tue Apr  8 12:19:07 2025] R10: fffffffffffffbc5 R11: 0000000000000246 R12: 0000000000000000
[Tue Apr  8 12:19:07 2025] R13: 0000000000000003 R14: 0000000000020000 R15: 0000000000020000
[Tue Apr  8 12:19:07 2025]  </TASK>

[1] https://lore.kernel.org/all/174046694331.2194069.15472952050240807469.stgit@mhiramat.tok.corp.google.com

Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Mingzhe Yang <mingzhe.yang@ly.com>
Signed-off-by: Lance Yang <ioworker0@gmail.com>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 include/linux/semaphore.h  | 15 +++++++++-
 kernel/hung_task.c         | 52 ++++++++++++++++++++++++++--------
 kernel/locking/semaphore.c | 57 ++++++++++++++++++++++++++++++++++----
 3 files changed, 106 insertions(+), 18 deletions(-)

diff --git a/include/linux/semaphore.h b/include/linux/semaphore.h
index 04655faadc2d..89706157e622 100644
--- a/include/linux/semaphore.h
+++ b/include/linux/semaphore.h
@@ -16,13 +16,25 @@ struct semaphore {
 	raw_spinlock_t		lock;
 	unsigned int		count;
 	struct list_head	wait_list;
+
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+	unsigned long		last_holder;
+#endif
 };
 
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+#define __LAST_HOLDER_SEMAPHORE_INITIALIZER				\
+	, .last_holder = 0UL
+#else
+#define __LAST_HOLDER_SEMAPHORE_INITIALIZER
+#endif
+
 #define __SEMAPHORE_INITIALIZER(name, n)				\
 {									\
 	.lock		= __RAW_SPIN_LOCK_UNLOCKED((name).lock),	\
 	.count		= n,						\
-	.wait_list	= LIST_HEAD_INIT((name).wait_list),		\
+	.wait_list	= LIST_HEAD_INIT((name).wait_list)		\
+	__LAST_HOLDER_SEMAPHORE_INITIALIZER				\
 }
 
 /*
@@ -47,5 +59,6 @@ extern int __must_check down_killable(struct semaphore *sem);
 extern int __must_check down_trylock(struct semaphore *sem);
 extern int __must_check down_timeout(struct semaphore *sem, long jiffies);
 extern void up(struct semaphore *sem);
+extern unsigned long sem_last_holder(struct semaphore *sem);
 
 #endif /* __LINUX_SEMAPHORE_H */
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index 79558d76ef06..d2432df2b905 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -99,32 +99,62 @@ static struct notifier_block panic_block = {
 static void debug_show_blocker(struct task_struct *task)
 {
 	struct task_struct *g, *t;
-	unsigned long owner, blocker;
+	unsigned long owner, blocker, blocker_type;
 
 	RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "No rcu lock held");
 
 	blocker = READ_ONCE(task->blocker);
-	if (!blocker ||
-	    hung_task_get_blocker_type(blocker) != BLOCKER_TYPE_MUTEX)
+	if (!blocker)
 		return;
 
-	owner = mutex_get_owner(
-		(struct mutex *)hung_task_blocker_to_lock(blocker));
+	blocker_type = hung_task_get_blocker_type(blocker);
+
+	switch (blocker_type) {
+	case BLOCKER_TYPE_MUTEX:
+		owner = mutex_get_owner(
+			(struct mutex *)hung_task_blocker_to_lock(blocker));
+		break;
+	case BLOCKER_TYPE_SEM:
+		owner = sem_last_holder(
+			(struct semaphore *)hung_task_blocker_to_lock(blocker));
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return;
+	}
+
 
 	if (unlikely(!owner)) {
-		pr_err("INFO: task %s:%d is blocked on a mutex, but the owner is not found.\n",
-			task->comm, task->pid);
+		switch (blocker_type) {
+		case BLOCKER_TYPE_MUTEX:
+			pr_err("INFO: task %s:%d is blocked on a mutex, but the owner is not found.\n",
+			       task->comm, task->pid);
+			break;
+		case BLOCKER_TYPE_SEM:
+			pr_err("INFO: task %s:%d is blocked on a semaphore, but the last holder is not found.\n",
+			       task->comm, task->pid);
+			break;
+		}
 		return;
 	}
 
 	/* Ensure the owner information is correct. */
 	for_each_process_thread(g, t) {
-		if ((unsigned long)t == owner) {
+		if ((unsigned long)t != owner)
+			continue;
+
+		switch (blocker_type) {
+		case BLOCKER_TYPE_MUTEX:
 			pr_err("INFO: task %s:%d is blocked on a mutex likely owned by task %s:%d.\n",
-				task->comm, task->pid, t->comm, t->pid);
-			sched_show_task(t);
-			return;
+			       task->comm, task->pid, t->comm, t->pid);
+			break;
+		case BLOCKER_TYPE_SEM:
+			pr_err("INFO: task %s:%d blocked on a semaphore likely last held by task %s:%d\n",
+			       task->comm, task->pid, t->comm, t->pid);
+			break;
 		}
+		sched_show_task(t);
+		return;
 	}
 }
 #else
diff --git a/kernel/locking/semaphore.c b/kernel/locking/semaphore.c
index 34bfae72f295..db8a8f696f50 100644
--- a/kernel/locking/semaphore.c
+++ b/kernel/locking/semaphore.c
@@ -33,6 +33,7 @@
 #include <linux/spinlock.h>
 #include <linux/ftrace.h>
 #include <trace/events/lock.h>
+#include <linux/hung_task.h>
 
 static noinline void __down(struct semaphore *sem);
 static noinline int __down_interruptible(struct semaphore *sem);
@@ -40,6 +41,41 @@ static noinline int __down_killable(struct semaphore *sem);
 static noinline int __down_timeout(struct semaphore *sem, long timeout);
 static noinline void __up(struct semaphore *sem);
 
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+static inline void hung_task_sem_set_holder(struct semaphore *sem)
+{
+	WRITE_ONCE((sem)->last_holder, (unsigned long)current);
+}
+
+static inline void hung_task_sem_clear_if_holder(struct semaphore *sem)
+{
+	if (READ_ONCE((sem)->last_holder) == (unsigned long)current)
+		WRITE_ONCE((sem)->last_holder, 0UL);
+}
+
+unsigned long sem_last_holder(struct semaphore *sem)
+{
+	return READ_ONCE(sem->last_holder);
+}
+#else
+static inline void hung_task_sem_set_holder(struct semaphore *sem)
+{
+}
+static inline void hung_task_sem_clear_if_holder(struct semaphore *sem)
+{
+}
+unsigned long sem_last_holder(struct semaphore *sem)
+{
+	return 0UL;
+}
+#endif
+
+static inline void __sem_acquire(struct semaphore *sem)
+{
+	sem->count--;
+	hung_task_sem_set_holder(sem);
+}
+
 /**
  * down - acquire the semaphore
  * @sem: the semaphore to be acquired
@@ -58,7 +94,7 @@ void __sched down(struct semaphore *sem)
 	might_sleep();
 	raw_spin_lock_irqsave(&sem->lock, flags);
 	if (likely(sem->count > 0))
-		sem->count--;
+		__sem_acquire(sem);
 	else
 		__down(sem);
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
@@ -82,7 +118,7 @@ int __sched down_interruptible(struct semaphore *sem)
 	might_sleep();
 	raw_spin_lock_irqsave(&sem->lock, flags);
 	if (likely(sem->count > 0))
-		sem->count--;
+		__sem_acquire(sem);
 	else
 		result = __down_interruptible(sem);
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
@@ -109,7 +145,7 @@ int __sched down_killable(struct semaphore *sem)
 	might_sleep();
 	raw_spin_lock_irqsave(&sem->lock, flags);
 	if (likely(sem->count > 0))
-		sem->count--;
+		__sem_acquire(sem);
 	else
 		result = __down_killable(sem);
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
@@ -139,7 +175,7 @@ int __sched down_trylock(struct semaphore *sem)
 	raw_spin_lock_irqsave(&sem->lock, flags);
 	count = sem->count - 1;
 	if (likely(count >= 0))
-		sem->count = count;
+		__sem_acquire(sem);
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
 
 	return (count < 0);
@@ -164,7 +200,7 @@ int __sched down_timeout(struct semaphore *sem, long timeout)
 	might_sleep();
 	raw_spin_lock_irqsave(&sem->lock, flags);
 	if (likely(sem->count > 0))
-		sem->count--;
+		__sem_acquire(sem);
 	else
 		result = __down_timeout(sem, timeout);
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
@@ -185,6 +221,9 @@ void __sched up(struct semaphore *sem)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&sem->lock, flags);
+
+	hung_task_sem_clear_if_holder(sem);
+
 	if (likely(list_empty(&sem->wait_list)))
 		sem->count++;
 	else
@@ -224,8 +263,10 @@ static inline int __sched ___down_common(struct semaphore *sem, long state,
 		raw_spin_unlock_irq(&sem->lock);
 		timeout = schedule_timeout(timeout);
 		raw_spin_lock_irq(&sem->lock);
-		if (waiter.up)
+		if (waiter.up) {
+			hung_task_sem_set_holder(sem);
 			return 0;
+		}
 	}
 
  timed_out:
@@ -242,10 +283,14 @@ static inline int __sched __down_common(struct semaphore *sem, long state,
 {
 	int ret;
 
+	hung_task_set_blocker(sem, BLOCKER_TYPE_SEM);
+
 	trace_contention_begin(sem, 0);
 	ret = ___down_common(sem, state, timeout);
 	trace_contention_end(sem, ret);
 
+	hung_task_clear_blocker();
+
 	return ret;
 }
 
-- 
2.49.0


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

* [PATCH v5 3/3] samples: extend hung_task detector test with semaphore support
  2025-04-14 14:59 [PATCH v5 0/3] hung_task: extend blocking task stacktrace dump to semaphore Lance Yang
  2025-04-14 14:59 ` [PATCH v5 1/3] hung_task: replace blocker_mutex with encoded blocker Lance Yang
  2025-04-14 14:59 ` [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore Lance Yang
@ 2025-04-14 14:59 ` Lance Yang
  2025-04-14 21:38 ` [PATCH v5 0/3] hung_task: extend blocking task stacktrace dump to semaphore Andrew Morton
  3 siblings, 0 replies; 27+ messages in thread
From: Lance Yang @ 2025-04-14 14:59 UTC (permalink / raw)
  To: akpm
  Cc: will, peterz, mingo, longman, mhiramat, anna.schumaker,
	boqun.feng, joel.granados, kent.overstreet, leonylgao,
	linux-kernel, rostedt, senozhatsky, tfiga, amaindex, jstultz,
	Lance Yang

From: Zi Li <amaindex@outlook.com>

Extend the existing hung_task detector test module to support multiple lock
types, including mutex and semaphore, with room for future additions (e.g.,
spinlock, etc.). This module creates dummy files under <debugfs>/hung_task,
such as 'mutex' and 'semaphore'. The read process on any of these files
will sleep for enough long time (256 seconds) while holding the respective
lock. As a result, the second process will wait on the lock for a prolonged
duration and be detected by the hung_task detector.

This change unifies the previous mutex-only sample into a single,
extensible hung_task_tests module, reducing code duplication and improving
maintainability.

Usage is:

	> cd /sys/kernel/debug/hung_task
	> cat mutex & cat mutex          # Test mutex blocking
	> cat semaphore & cat semaphore  # Test semaphore blocking

Update the Kconfig description to reflect multiple debugfs files support.

Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Lance Yang <ioworker0@gmail.com>
Signed-off-by: Zi Li <amaindex@outlook.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 samples/Kconfig                     |  9 +--
 samples/hung_task/Makefile          |  2 +-
 samples/hung_task/hung_task_mutex.c | 66 --------------------
 samples/hung_task/hung_task_tests.c | 97 +++++++++++++++++++++++++++++
 4 files changed, 103 insertions(+), 71 deletions(-)
 delete mode 100644 samples/hung_task/hung_task_mutex.c
 create mode 100644 samples/hung_task/hung_task_tests.c

diff --git a/samples/Kconfig b/samples/Kconfig
index 09011be2391a..753ed1f170b5 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -304,10 +304,11 @@ config SAMPLE_HUNG_TASK
 	tristate "Hung task detector test code"
 	depends on DETECT_HUNG_TASK && DEBUG_FS
 	help
-	  Build a module which provide a simple debugfs file. If user reads
-	  the file, it will sleep long time (256 seconds) with holding a
-	  mutex. Thus if there are 2 or more processes read this file, it
-	  will be detected by the hung_task watchdog.
+	  Build a module that provides debugfs files (e.g., mutex, semaphore,
+	  etc.) under <debugfs>/hung_task. If user reads one of these files,
+	  it will sleep long time (256 seconds) with holding a lock. Thus,
+	  if 2 or more processes read the same file concurrently, it will
+	  be detected by the hung_task watchdog.
 
 source "samples/rust/Kconfig"
 
diff --git a/samples/hung_task/Makefile b/samples/hung_task/Makefile
index f4d6ab563488..86036f1a204d 100644
--- a/samples/hung_task/Makefile
+++ b/samples/hung_task/Makefile
@@ -1,2 +1,2 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_SAMPLE_HUNG_TASK) += hung_task_mutex.o
+obj-$(CONFIG_SAMPLE_HUNG_TASK) += hung_task_tests.o
diff --git a/samples/hung_task/hung_task_mutex.c b/samples/hung_task/hung_task_mutex.c
deleted file mode 100644
index 47ed38239ea3..000000000000
--- a/samples/hung_task/hung_task_mutex.c
+++ /dev/null
@@ -1,66 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * hung_task_mutex.c - Sample code which causes hung task by mutex
- *
- * Usage: load this module and read `<debugfs>/hung_task/mutex`
- *        by 2 or more processes.
- *
- * This is for testing kernel hung_task error message.
- * Note that this will make your system freeze and maybe
- * cause panic. So do not use this except for the test.
- */
-
-#include <linux/debugfs.h>
-#include <linux/delay.h>
-#include <linux/fs.h>
-#include <linux/module.h>
-#include <linux/mutex.h>
-
-#define HUNG_TASK_DIR   "hung_task"
-#define HUNG_TASK_FILE  "mutex"
-#define SLEEP_SECOND 256
-
-static const char dummy_string[] = "This is a dummy string.";
-static DEFINE_MUTEX(dummy_mutex);
-static struct dentry *hung_task_dir;
-
-static ssize_t read_dummy(struct file *file, char __user *user_buf,
-			  size_t count, loff_t *ppos)
-{
-	/* If the second task waits on the lock, it is uninterruptible sleep. */
-	guard(mutex)(&dummy_mutex);
-
-	/* When the first task sleep here, it is interruptible. */
-	msleep_interruptible(SLEEP_SECOND * 1000);
-
-	return simple_read_from_buffer(user_buf, count, ppos,
-				dummy_string, sizeof(dummy_string));
-}
-
-static const struct file_operations hung_task_fops = {
-	.read = read_dummy,
-};
-
-static int __init hung_task_sample_init(void)
-{
-	hung_task_dir = debugfs_create_dir(HUNG_TASK_DIR, NULL);
-	if (IS_ERR(hung_task_dir))
-		return PTR_ERR(hung_task_dir);
-
-	debugfs_create_file(HUNG_TASK_FILE, 0400, hung_task_dir,
-			    NULL, &hung_task_fops);
-
-	return 0;
-}
-
-static void __exit hung_task_sample_exit(void)
-{
-	debugfs_remove_recursive(hung_task_dir);
-}
-
-module_init(hung_task_sample_init);
-module_exit(hung_task_sample_exit);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Masami Hiramatsu");
-MODULE_DESCRIPTION("Simple sleep under mutex file for testing hung task");
diff --git a/samples/hung_task/hung_task_tests.c b/samples/hung_task/hung_task_tests.c
new file mode 100644
index 000000000000..a5c09bd3a47d
--- /dev/null
+++ b/samples/hung_task/hung_task_tests.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * hung_task_tests.c - Sample code for testing hung tasks with mutex,
+ * semaphore, etc.
+ *
+ * Usage: Load this module and read `<debugfs>/hung_task/mutex`,
+ *        `<debugfs>/hung_task/semaphore`, etc., with 2 or more processes.
+ *
+ * This is for testing kernel hung_task error messages with various locking
+ * mechanisms (e.g., mutex, semaphore, etc.). Note that this may freeze
+ * your system or cause a panic. Use only for testing purposes.
+ */
+
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/fs.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/semaphore.h>
+
+#define HUNG_TASK_DIR		"hung_task"
+#define HUNG_TASK_MUTEX_FILE	"mutex"
+#define HUNG_TASK_SEM_FILE	"semaphore"
+#define SLEEP_SECOND		256
+
+static const char dummy_string[] = "This is a dummy string.";
+static DEFINE_MUTEX(dummy_mutex);
+static DEFINE_SEMAPHORE(dummy_sem, 1);
+static struct dentry *hung_task_dir;
+
+/* Mutex-based read function */
+static ssize_t read_dummy_mutex(struct file *file, char __user *user_buf,
+				size_t count, loff_t *ppos)
+{
+	/* Second task waits on mutex, entering uninterruptible sleep */
+	guard(mutex)(&dummy_mutex);
+
+	/* First task sleeps here, interruptible */
+	msleep_interruptible(SLEEP_SECOND * 1000);
+
+	return simple_read_from_buffer(user_buf, count, ppos, dummy_string,
+				       sizeof(dummy_string));
+}
+
+/* Semaphore-based read function */
+static ssize_t read_dummy_semaphore(struct file *file, char __user *user_buf,
+				    size_t count, loff_t *ppos)
+{
+	/* Second task waits on semaphore, entering uninterruptible sleep */
+	down(&dummy_sem);
+
+	/* First task sleeps here, interruptible */
+	msleep_interruptible(SLEEP_SECOND * 1000);
+
+	up(&dummy_sem);
+
+	return simple_read_from_buffer(user_buf, count, ppos, dummy_string,
+				       sizeof(dummy_string));
+}
+
+/* File operations for mutex */
+static const struct file_operations hung_task_mutex_fops = {
+	.read = read_dummy_mutex,
+};
+
+/* File operations for semaphore */
+static const struct file_operations hung_task_sem_fops = {
+	.read = read_dummy_semaphore,
+};
+
+static int __init hung_task_tests_init(void)
+{
+	hung_task_dir = debugfs_create_dir(HUNG_TASK_DIR, NULL);
+	if (IS_ERR(hung_task_dir))
+		return PTR_ERR(hung_task_dir);
+
+	/* Create debugfs files for mutex and semaphore tests */
+	debugfs_create_file(HUNG_TASK_MUTEX_FILE, 0400, hung_task_dir, NULL,
+			    &hung_task_mutex_fops);
+	debugfs_create_file(HUNG_TASK_SEM_FILE, 0400, hung_task_dir, NULL,
+			    &hung_task_sem_fops);
+
+	return 0;
+}
+
+static void __exit hung_task_tests_exit(void)
+{
+	debugfs_remove_recursive(hung_task_dir);
+}
+
+module_init(hung_task_tests_init);
+module_exit(hung_task_tests_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Masami Hiramatsu <mhiramat@kernel.org>");
+MODULE_AUTHOR("Zi Li <amaindex@outlook.com>");
+MODULE_DESCRIPTION("Simple sleep under lock files for testing hung task");
-- 
2.49.0


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

* Re: [PATCH v5 1/3] hung_task: replace blocker_mutex with encoded blocker
  2025-04-14 14:59 ` [PATCH v5 1/3] hung_task: replace blocker_mutex with encoded blocker Lance Yang
@ 2025-04-14 21:36   ` Andrew Morton
  2025-04-15  3:44     ` Lance Yang
  0 siblings, 1 reply; 27+ messages in thread
From: Andrew Morton @ 2025-04-14 21:36 UTC (permalink / raw)
  To: Lance Yang
  Cc: will, peterz, mingo, longman, mhiramat, anna.schumaker,
	boqun.feng, joel.granados, kent.overstreet, leonylgao,
	linux-kernel, rostedt, senozhatsky, tfiga, amaindex, jstultz,
	Mingzhe Yang

On Mon, 14 Apr 2025 22:59:43 +0800 Lance Yang <ioworker0@gmail.com> wrote:

> This patch replaces 'struct mutex *blocker_mutex' with 'unsigned long
> blocker', as only one blocker is active at a time.
> 
> The blocker filed can store both the lock addrees and the lock type, with
> LSB used to encode the type as Masami suggested, making it easier to extend
> the feature to cover other types of locks.
> 
> Also, once the lock type is determined, we can directly extract the address
> and cast it to a lock pointer ;)
> 
> ...
>
>  static void debug_show_blocker(struct task_struct *task)
>  {
>  	struct task_struct *g, *t;
> -	unsigned long owner;
> -	struct mutex *lock;
> +	unsigned long owner, blocker;
>  
>  	RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "No rcu lock held");
>  
> -	lock = READ_ONCE(task->blocker_mutex);
> -	if (!lock)
> +	blocker = READ_ONCE(task->blocker);
> +	if (!blocker ||
> +	    hung_task_get_blocker_type(blocker) != BLOCKER_TYPE_MUTEX)
>  		return;
>  
> -	owner = mutex_get_owner(lock);
> +	owner = mutex_get_owner(
> +		(struct mutex *)hung_task_blocker_to_lock(blocker));

typecast is unneeded?



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

* Re: [PATCH v5 0/3] hung_task: extend blocking task stacktrace dump to semaphore
  2025-04-14 14:59 [PATCH v5 0/3] hung_task: extend blocking task stacktrace dump to semaphore Lance Yang
                   ` (2 preceding siblings ...)
  2025-04-14 14:59 ` [PATCH v5 3/3] samples: extend hung_task detector test with semaphore support Lance Yang
@ 2025-04-14 21:38 ` Andrew Morton
  3 siblings, 0 replies; 27+ messages in thread
From: Andrew Morton @ 2025-04-14 21:38 UTC (permalink / raw)
  To: Lance Yang
  Cc: will, peterz, mingo, longman, mhiramat, anna.schumaker,
	boqun.feng, joel.granados, kent.overstreet, leonylgao,
	linux-kernel, rostedt, senozhatsky, tfiga, amaindex, jstultz

On Mon, 14 Apr 2025 22:59:42 +0800 Lance Yang <ioworker0@gmail.com> wrote:

> Inspired by mutex blocker tracking[1], this patch series extend the
> feature to not only dump the blocker task holding a mutex but also to
> support semaphores. Unlike mutexes, semaphores lack explicit ownership
> tracking, making it challenging to identify the root cause of hangs. To
> address this, we introduce a last_holder field to the semaphore structure,
> which is updated when a task successfully calls down() and cleared during
> up().

Thanks, I'll queue this series in mm.git's mm-nonmm-unstable branch for
some test and exposure.

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

* Re: [PATCH v5 1/3] hung_task: replace blocker_mutex with encoded blocker
  2025-04-14 21:36   ` Andrew Morton
@ 2025-04-15  3:44     ` Lance Yang
  0 siblings, 0 replies; 27+ messages in thread
From: Lance Yang @ 2025-04-15  3:44 UTC (permalink / raw)
  To: akpm
  Cc: amaindex, anna.schumaker, boqun.feng, ioworker0, joel.granados,
	jstultz, kent.overstreet, leonylgao, linux-kernel, longman,
	mhiramat, mingo, mingzhe.yang, peterz, rostedt, senozhatsky,
	tfiga, will

Hi Andrew,

On Tue, Apr 15, 2025 at 5:36 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Mon, 14 Apr 2025 22:59:43 +0800 Lance Yang <ioworker0@gmail.com> wrote:
>
> > This patch replaces 'struct mutex *blocker_mutex' with 'unsigned long
> > blocker', as only one blocker is active at a time.
> >
> > The blocker filed can store both the lock addrees and the lock type, with
> > LSB used to encode the type as Masami suggested, making it easier to extend
> > the feature to cover other types of locks.
> >
> > Also, once the lock type is determined, we can directly extract the address
> > and cast it to a lock pointer ;)
> >
> > ...
> >
> >  static void debug_show_blocker(struct task_struct *task)
> >  {
> >       struct task_struct *g, *t;
> > -     unsigned long owner;
> > -     struct mutex *lock;
> > +     unsigned long owner, blocker;
> > 
> >       RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "No rcu lock held");
> > 
> > -     lock = READ_ONCE(task->blocker_mutex);
> > -     if (!lock)
> > +     blocker = READ_ONCE(task->blocker);
> > +     if (!blocker ||
> > +         hung_task_get_blocker_type(blocker) != BLOCKER_TYPE_MUTEX)
> >               return;
> > 
> > -     owner = mutex_get_owner(lock);
> > +     owner = mutex_get_owner(
> > +             (struct mutex *)hung_task_blocker_to_lock(blocker));
>
> typecast is unneeded?

Good catch! This typecast is redundant here. Since the #02[1] patch already
covers this code section, could you please fold the following change into
that patch?

diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index d2432df2b905..b30b9ab17694 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -111,12 +111,10 @@ static void debug_show_blocker(struct task_struct *task)
 
 	switch (blocker_type) {
 	case BLOCKER_TYPE_MUTEX:
-		owner = mutex_get_owner(
-			(struct mutex *)hung_task_blocker_to_lock(blocker));
+		owner = mutex_get_owner(hung_task_blocker_to_lock(blocker));
 		break;
 	case BLOCKER_TYPE_SEM:
-		owner = sem_last_holder(
-			(struct semaphore *)hung_task_blocker_to_lock(blocker));
+		owner = sem_last_holder(hung_task_blocker_to_lock(blocker));
 		break;
 	default:
 		WARN_ON_ONCE(1);
--

[1] https://lore.kernel.org/all/20250414145945.84916-3-ioworker0@gmail.com

Thanks,
Lance

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

* Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore
  2025-04-14 14:59 ` [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore Lance Yang
@ 2025-08-22  7:38   ` Geert Uytterhoeven
  2025-08-22 15:18     ` Lance Yang
  0 siblings, 1 reply; 27+ messages in thread
From: Geert Uytterhoeven @ 2025-08-22  7:38 UTC (permalink / raw)
  To: Lance Yang
  Cc: akpm, will, peterz, mingo, longman, mhiramat, anna.schumaker,
	boqun.feng, joel.granados, kent.overstreet, leonylgao,
	linux-kernel, rostedt, senozhatsky, tfiga, amaindex, jstultz,
	Mingzhe Yang, Eero Tamminen, linux-m68k

Hi Lance,

(this time the right email thread, I hope ;-)

On Mon, 14 Apr 2025 at 17:23, Lance Yang <ioworker0@gmail.com> wrote:
> Inspired by mutex blocker tracking[1], this patch makes a trade-off to
> balance the overhead and utility of the hung task detector.
>
> Unlike mutexes, semaphores lack explicit ownership tracking, making it
> challenging to identify the root cause of hangs. To address this, we
> introduce a last_holder field to the semaphore structure, which is
> updated when a task successfully calls down() and cleared during up().
>
> The assumption is that if a task is blocked on a semaphore, the holders
> must not have released it. While this does not guarantee that the last
> holder is one of the current blockers, it likely provides a practical hint
> for diagnosing semaphore-related stalls.
>
> With this change, the hung task detector can now show blocker task's info
> like below:
>
> [Tue Apr  8 12:19:07 2025] INFO: task cat:945 blocked for more than 120 seconds.
> [Tue Apr  8 12:19:07 2025]       Tainted: G            E      6.14.0-rc6+ #1
> [Tue Apr  8 12:19:07 2025] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [Tue Apr  8 12:19:07 2025] task:cat             state:D stack:0     pid:945   tgid:945   ppid:828    task_flags:0x400000 flags:0x00000000
> [Tue Apr  8 12:19:07 2025] Call Trace:
> [Tue Apr  8 12:19:07 2025]  <TASK>
> [Tue Apr  8 12:19:07 2025]  __schedule+0x491/0xbd0
> [Tue Apr  8 12:19:07 2025]  schedule+0x27/0xf0
> [Tue Apr  8 12:19:07 2025]  schedule_timeout+0xe3/0xf0
> [Tue Apr  8 12:19:07 2025]  ? __folio_mod_stat+0x2a/0x80
> [Tue Apr  8 12:19:07 2025]  ? set_ptes.constprop.0+0x27/0x90
> [Tue Apr  8 12:19:07 2025]  __down_common+0x155/0x280
> [Tue Apr  8 12:19:07 2025]  down+0x53/0x70
> [Tue Apr  8 12:19:07 2025]  read_dummy_semaphore+0x23/0x60
> [Tue Apr  8 12:19:07 2025]  full_proxy_read+0x5f/0xa0
> [Tue Apr  8 12:19:07 2025]  vfs_read+0xbc/0x350
> [Tue Apr  8 12:19:07 2025]  ? __count_memcg_events+0xa5/0x140
> [Tue Apr  8 12:19:07 2025]  ? count_memcg_events.constprop.0+0x1a/0x30
> [Tue Apr  8 12:19:07 2025]  ? handle_mm_fault+0x180/0x260
> [Tue Apr  8 12:19:07 2025]  ksys_read+0x66/0xe0
> [Tue Apr  8 12:19:07 2025]  do_syscall_64+0x51/0x120
> [Tue Apr  8 12:19:07 2025]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [Tue Apr  8 12:19:07 2025] RIP: 0033:0x7f419478f46e
> [Tue Apr  8 12:19:07 2025] RSP: 002b:00007fff1c4d2668 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> [Tue Apr  8 12:19:07 2025] RAX: ffffffffffffffda RBX: 0000000000020000 RCX: 00007f419478f46e
> [Tue Apr  8 12:19:07 2025] RDX: 0000000000020000 RSI: 00007f4194683000 RDI: 0000000000000003
> [Tue Apr  8 12:19:07 2025] RBP: 00007f4194683000 R08: 00007f4194682010 R09: 0000000000000000
> [Tue Apr  8 12:19:07 2025] R10: fffffffffffffbc5 R11: 0000000000000246 R12: 0000000000000000
> [Tue Apr  8 12:19:07 2025] R13: 0000000000000003 R14: 0000000000020000 R15: 0000000000020000
> [Tue Apr  8 12:19:07 2025]  </TASK>
> [Tue Apr  8 12:19:07 2025] INFO: task cat:945 blocked on a semaphore likely last held by task cat:938
> [Tue Apr  8 12:19:07 2025] task:cat             state:S stack:0     pid:938   tgid:938   ppid:584    task_flags:0x400000 flags:0x00000000
> [Tue Apr  8 12:19:07 2025] Call Trace:
> [Tue Apr  8 12:19:07 2025]  <TASK>
> [Tue Apr  8 12:19:07 2025]  __schedule+0x491/0xbd0
> [Tue Apr  8 12:19:07 2025]  ? _raw_spin_unlock_irqrestore+0xe/0x40
> [Tue Apr  8 12:19:07 2025]  schedule+0x27/0xf0
> [Tue Apr  8 12:19:07 2025]  schedule_timeout+0x77/0xf0
> [Tue Apr  8 12:19:07 2025]  ? __pfx_process_timeout+0x10/0x10
> [Tue Apr  8 12:19:07 2025]  msleep_interruptible+0x49/0x60
> [Tue Apr  8 12:19:07 2025]  read_dummy_semaphore+0x2d/0x60
> [Tue Apr  8 12:19:07 2025]  full_proxy_read+0x5f/0xa0
> [Tue Apr  8 12:19:07 2025]  vfs_read+0xbc/0x350
> [Tue Apr  8 12:19:07 2025]  ? __count_memcg_events+0xa5/0x140
> [Tue Apr  8 12:19:07 2025]  ? count_memcg_events.constprop.0+0x1a/0x30
> [Tue Apr  8 12:19:07 2025]  ? handle_mm_fault+0x180/0x260
> [Tue Apr  8 12:19:07 2025]  ksys_read+0x66/0xe0
> [Tue Apr  8 12:19:07 2025]  do_syscall_64+0x51/0x120
> [Tue Apr  8 12:19:07 2025]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [Tue Apr  8 12:19:07 2025] RIP: 0033:0x7f7c584a646e
> [Tue Apr  8 12:19:07 2025] RSP: 002b:00007ffdba8ce158 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> [Tue Apr  8 12:19:07 2025] RAX: ffffffffffffffda RBX: 0000000000020000 RCX: 00007f7c584a646e
> [Tue Apr  8 12:19:07 2025] RDX: 0000000000020000 RSI: 00007f7c5839a000 RDI: 0000000000000003
> [Tue Apr  8 12:19:07 2025] RBP: 00007f7c5839a000 R08: 00007f7c58399010 R09: 0000000000000000
> [Tue Apr  8 12:19:07 2025] R10: fffffffffffffbc5 R11: 0000000000000246 R12: 0000000000000000
> [Tue Apr  8 12:19:07 2025] R13: 0000000000000003 R14: 0000000000020000 R15: 0000000000020000
> [Tue Apr  8 12:19:07 2025]  </TASK>
>
> [1] https://lore.kernel.org/all/174046694331.2194069.15472952050240807469.stgit@mhiramat.tok.corp.google.com
>
> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
> Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Signed-off-by: Mingzhe Yang <mingzhe.yang@ly.com>
> Signed-off-by: Lance Yang <ioworker0@gmail.com>
> Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks for your patch, which is now commit 194a9b9e843b4077
("hung_task: show the blocker task if the task is hung on
semaphore") in v6.16-rc1.

Eero reported [1] two WARNINGS seen with v6.16 on emulated Atari.
I managed to reproduce it on ARAnyM using the provided config (it does
not happen with atari_defconfig), and bisected it to this commit:

------------[ cut here ]------------
WARNING: CPU: 0 PID: 39 at include/linux/hung_task.h:48
__down_common+0x13a/0x1be
CPU: 0 UID: 0 PID: 39 Comm: getty Not tainted
6.15.0-rc6hatari-00018-g194a9b9e843b #1986 NONE
Stack from 01633d00:
        01633d00 00366e9e 00366e9e 00000000 002c9762 00360cb5 01633d24 0000873e
        00366e9e 01633d40 0002e0d4 00360cb5 00000030 00000009 0039c79a 00061408
        01633d78 000028e0 00360cb5 00000030 002c9762 00000009 00000000 00000000
        7fffffff 00000002 1185d266 01633eb0 01326c58 00000080 01633dc0 002c9762
        00360cb5 00000030 00000009 00000000 00002014 01326c00 1185d266 01633eb0
        002c93ea 00053d60 00061408 01326c58 0038db90 0038db90 01633e32 01633fb8
Call Trace: [<002c9762>] __down_common+0x13a/0x1be
 [<0000873e>] dump_stack+0x10/0x16
 [<0002e0d4>] __warn+0x7a/0xbc
 [<00061408>] msleep+0x0/0x2c
 [<000028e0>] warn_slowpath_fmt+0x42/0x62
 [<002c9762>] __down_common+0x13a/0x1be
 [<002c9762>] __down_common+0x13a/0x1be
 [<00002014>] arch_local_irq_enable+0xe/0x22
 [<002c93ea>] mutex_lock+0x0/0x28
 [<00053d60>] other_cpu_in_panic+0x0/0x26
 [<00061408>] msleep+0x0/0x2c
 [<002c97fc>] __down+0x16/0x1e
 [<002c9832>] down+0x2e/0x30
 [<00053dac>] console_lock+0x26/0x4c
 [<001aae4e>] do_con_write+0x3a/0x16d4
 [<002c93ea>] mutex_lock+0x0/0x28
 [<0004fa70>] __add_wait_queue+0x3a/0x6a
 [<001ac520>] con_write+0x1a/0x30
 [<0019cafa>] n_tty_write+0x2c6/0x35e
 [<00199456>] signal_pending+0x0/0x26
 [<000aba2a>] __kvmalloc_node_noprof+0x3a/0x114
 [<00004cc0>] io_uring_try_cancel_requests+0x98/0x318
 [<0004fb2e>] woken_wake_function+0x0/0x24
 [<0019a180>] file_tty_write.isra.0+0x144/0x1b8
 [<0019a206>] tty_write+0x12/0x16
 [<000b97c2>] vfs_write+0xec/0x148
 [<00028000>] fp_getdest+0x1b8/0x224
 [<00010000>] g_trace+0x16/0x28
 [<000b9916>] ksys_write+0x54/0x8a
 [<000b9962>] sys_write+0x16/0x1a
 [<000093da>] syscall+0x8/0xc
 [<0000c001>] arch_dma_prep_coherent+0x51/0x58

---[ end trace 0000000000000000 ]---
------------[ cut here ]------------
WARNING: CPU: 0 PID: 39 at include/linux/hung_task.h:56
__down_common+0x17a/0x1be
CPU: 0 UID: 0 PID: 39 Comm: getty Tainted: G        W
6.15.0-rc6hatari-00018-g194a9b9e843b #1986 NONE
Tainted: [W]=WARN
Stack from 01633d00:
        01633d00 00366e9e 00366e9e 00000000 002c97a2 00360cb5 01633d24 0000873e
        00366e9e 01633d40 0002e0d4 00360cb5 00000038 00000009 0039c79a 01633db2
        01633d78 000028e0 00360cb5 00000038 002c97a2 00000009 00000000 00000000
        00000000 00000002 00000000 00000000 01326c58 0039c79a 01633dc0 002c97a2
        00360cb5 00000038 00000009 00000000 00002014 01326c00 1185d266 01633eb0
        002c93ea 00053d60 00061408 01326c58 00380000 01000000 01220162 b64001b8
Call Trace: [<002c97a2>] __down_common+0x17a/0x1be
 [<0000873e>] dump_stack+0x10/0x16
 [<0002e0d4>] __warn+0x7a/0xbc
 [<000028e0>] warn_slowpath_fmt+0x42/0x62
 [<002c97a2>] __down_common+0x17a/0x1be
 [<002c97a2>] __down_common+0x17a/0x1be
 [<00002014>] arch_local_irq_enable+0xe/0x22
 [<002c93ea>] mutex_lock+0x0/0x28
 [<00053d60>] other_cpu_in_panic+0x0/0x26
 [<00061408>] msleep+0x0/0x2c
 [<002c97fc>] __down+0x16/0x1e
 [<002c9832>] down+0x2e/0x30
 [<00053dac>] console_lock+0x26/0x4c
 [<001aae4e>] do_con_write+0x3a/0x16d4
 [<002c93ea>] mutex_lock+0x0/0x28
 [<0004fa70>] __add_wait_queue+0x3a/0x6a
 [<001ac520>] con_write+0x1a/0x30
 [<0019cafa>] n_tty_write+0x2c6/0x35e
 [<00199456>] signal_pending+0x0/0x26
 [<000aba2a>] __kvmalloc_node_noprof+0x3a/0x114
 [<00004cc0>] io_uring_try_cancel_requests+0x98/0x318
 [<0004fb2e>] woken_wake_function+0x0/0x24
 [<0019a180>] file_tty_write.isra.0+0x144/0x1b8
 [<0019a206>] tty_write+0x12/0x16
 [<000b97c2>] vfs_write+0xec/0x148
 [<00028000>] fp_getdest+0x1b8/0x224
 [<00010000>] g_trace+0x16/0x28
 [<000b9916>] ksys_write+0x54/0x8a
 [<000b9962>] sys_write+0x16/0x1a
 [<000093da>] syscall+0x8/0xc
 [<0000c001>] arch_dma_prep_coherent+0x51/0x58

---[ end trace 0000000000000000 ]---

It still happens on v6.17-rc2.  Reverting commits 77da18de55ac6417
("hung_task: extend hung task blocker tracking to rwsems") and
194a9b9e843b4077 ("hung_task: show the blocker task if the task is
hung on semaphore") fixes the issue for me.

Thanks!

[1] "v6.16 console issues on Atari Falcon"
    https://lore.kernel.org/all/92518308-c763-4591-96ef-6b38c5d8f434@helsinkinet.fi

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore
  2025-08-22  7:38   ` Geert Uytterhoeven
@ 2025-08-22 15:18     ` Lance Yang
  2025-08-22 15:37       ` Geert Uytterhoeven
  0 siblings, 1 reply; 27+ messages in thread
From: Lance Yang @ 2025-08-22 15:18 UTC (permalink / raw)
  To: Geert Uytterhoeven, senozhatsky, mhiramat
  Cc: akpm, will, peterz, mingo, longman, anna.schumaker, boqun.feng,
	joel.granados, kent.overstreet, leonylgao, linux-kernel, rostedt,
	tfiga, amaindex, jstultz, Mingzhe Yang, Eero Tamminen, linux-m68k,
	Lance Yang

Hi Geert,

Thanks a lot for bisecting and reporting!

On 2025/8/22 15:38, Geert Uytterhoeven wrote:
> Hi Lance,
> 
> (this time the right email thread, I hope ;-)
> 
> On Mon, 14 Apr 2025 at 17:23, Lance Yang <ioworker0@gmail.com> wrote:
>> Inspired by mutex blocker tracking[1], this patch makes a trade-off to
>> balance the overhead and utility of the hung task detector.
>>
>> Unlike mutexes, semaphores lack explicit ownership tracking, making it
>> challenging to identify the root cause of hangs. To address this, we
>> introduce a last_holder field to the semaphore structure, which is
>> updated when a task successfully calls down() and cleared during up().
>>
>> The assumption is that if a task is blocked on a semaphore, the holders
>> must not have released it. While this does not guarantee that the last
>> holder is one of the current blockers, it likely provides a practical hint
>> for diagnosing semaphore-related stalls.
>>
[...]
> 
> Thanks for your patch, which is now commit 194a9b9e843b4077
> ("hung_task: show the blocker task if the task is hung on
> semaphore") in v6.16-rc1.
> 
> Eero reported [1] two WARNINGS seen with v6.16 on emulated Atari.
> I managed to reproduce it on ARAnyM using the provided config (it does
> not happen with atari_defconfig), and bisected it to this commit:

The two warnings are directly related, and the first one
is the root cause, IIUC.

> 
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 39 at include/linux/hung_task.h:48

The first warning at hung_task.h:48 is triggered because
WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK) check fails.

static inline void hung_task_set_blocker(void *lock, unsigned long type)
{
	unsigned long lock_ptr = (unsigned long)lock;

	WARN_ON_ONCE(!lock_ptr);
	WARN_ON_ONCE(READ_ONCE(current->blocker));

	/*
	 * If the lock pointer matches the BLOCKER_TYPE_MASK, return
	 * without writing anything.
	 */
	if (WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK)) <- here
		return;

This logic assumes the lock pointer is sufficiently aligned,
allowing the lower bits to be used for the lock type. But it
appears we are being passed an unaligned lock pointer,
unfortunately.

	WRITE_ONCE(current->blocker, lock_ptr | type);
}

Because the check fails, hung_task_set_blocker() returns
early without setting current->blocker, which directly
leads to the second warning at hung_task.h:56

> __down_common+0x13a/0x1be
> CPU: 0 UID: 0 PID: 39 Comm: getty Not tainted
> 6.15.0-rc6hatari-00018-g194a9b9e843b #1986 NONE
> Stack from 01633d00:
>          01633d00 00366e9e 00366e9e 00000000 002c9762 00360cb5 01633d24 0000873e
>          00366e9e 01633d40 0002e0d4 00360cb5 00000030 00000009 0039c79a 00061408
>          01633d78 000028e0 00360cb5 00000030 002c9762 00000009 00000000 00000000
>          7fffffff 00000002 1185d266 01633eb0 01326c58 00000080 01633dc0 002c9762
>          00360cb5 00000030 00000009 00000000 00002014 01326c00 1185d266 01633eb0
>          002c93ea 00053d60 00061408 01326c58 0038db90 0038db90 01633e32 01633fb8
> Call Trace: [<002c9762>] __down_common+0x13a/0x1be
>   [<0000873e>] dump_stack+0x10/0x16
>   [<0002e0d4>] __warn+0x7a/0xbc
>   [<00061408>] msleep+0x0/0x2c
>   [<000028e0>] warn_slowpath_fmt+0x42/0x62
>   [<002c9762>] __down_common+0x13a/0x1be
>   [<002c9762>] __down_common+0x13a/0x1be
>   [<00002014>] arch_local_irq_enable+0xe/0x22
>   [<002c93ea>] mutex_lock+0x0/0x28
>   [<00053d60>] other_cpu_in_panic+0x0/0x26
>   [<00061408>] msleep+0x0/0x2c
>   [<002c97fc>] __down+0x16/0x1e
>   [<002c9832>] down+0x2e/0x30
>   [<00053dac>] console_lock+0x26/0x4c
>   [<001aae4e>] do_con_write+0x3a/0x16d4
>   [<002c93ea>] mutex_lock+0x0/0x28
>   [<0004fa70>] __add_wait_queue+0x3a/0x6a
>   [<001ac520>] con_write+0x1a/0x30
>   [<0019cafa>] n_tty_write+0x2c6/0x35e
>   [<00199456>] signal_pending+0x0/0x26
>   [<000aba2a>] __kvmalloc_node_noprof+0x3a/0x114
>   [<00004cc0>] io_uring_try_cancel_requests+0x98/0x318
>   [<0004fb2e>] woken_wake_function+0x0/0x24
>   [<0019a180>] file_tty_write.isra.0+0x144/0x1b8
>   [<0019a206>] tty_write+0x12/0x16
>   [<000b97c2>] vfs_write+0xec/0x148
>   [<00028000>] fp_getdest+0x1b8/0x224
>   [<00010000>] g_trace+0x16/0x28
>   [<000b9916>] ksys_write+0x54/0x8a
>   [<000b9962>] sys_write+0x16/0x1a
>   [<000093da>] syscall+0x8/0xc
>   [<0000c001>] arch_dma_prep_coherent+0x51/0x58
> 
> ---[ end trace 0000000000000000 ]---
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 39 at include/linux/hung_task.h:56

Later, when hung_task_clear_blocker() is called, its
WARN_ON_ONCE triggers because it finds current->blocker
is still zero from the earlier failure.

static inline void hung_task_clear_blocker(void)
{
	WARN_ON_ONCE(!READ_ONCE(current->blocker)); <- here

	WRITE_ONCE(current->blocker, 0UL);
}

So, the unaligned lock pointer appears to be the root cause,
breaking the assumptions of the blocker tracking mechanism.

> __down_common+0x17a/0x1be
> CPU: 0 UID: 0 PID: 39 Comm: getty Tainted: G        W
> 6.15.0-rc6hatari-00018-g194a9b9e843b #1986 NONE
> Tainted: [W]=WARN
> Stack from 01633d00:
>          01633d00 00366e9e 00366e9e 00000000 002c97a2 00360cb5 01633d24 0000873e
>          00366e9e 01633d40 0002e0d4 00360cb5 00000038 00000009 0039c79a 01633db2
>          01633d78 000028e0 00360cb5 00000038 002c97a2 00000009 00000000 00000000
>          00000000 00000002 00000000 00000000 01326c58 0039c79a 01633dc0 002c97a2
>          00360cb5 00000038 00000009 00000000 00002014 01326c00 1185d266 01633eb0
>          002c93ea 00053d60 00061408 01326c58 00380000 01000000 01220162 b64001b8
> Call Trace: [<002c97a2>] __down_common+0x17a/0x1be
>   [<0000873e>] dump_stack+0x10/0x16
>   [<0002e0d4>] __warn+0x7a/0xbc
>   [<000028e0>] warn_slowpath_fmt+0x42/0x62
>   [<002c97a2>] __down_common+0x17a/0x1be
>   [<002c97a2>] __down_common+0x17a/0x1be
>   [<00002014>] arch_local_irq_enable+0xe/0x22
>   [<002c93ea>] mutex_lock+0x0/0x28
>   [<00053d60>] other_cpu_in_panic+0x0/0x26
>   [<00061408>] msleep+0x0/0x2c
>   [<002c97fc>] __down+0x16/0x1e
>   [<002c9832>] down+0x2e/0x30
>   [<00053dac>] console_lock+0x26/0x4c
>   [<001aae4e>] do_con_write+0x3a/0x16d4
>   [<002c93ea>] mutex_lock+0x0/0x28
>   [<0004fa70>] __add_wait_queue+0x3a/0x6a
>   [<001ac520>] con_write+0x1a/0x30
>   [<0019cafa>] n_tty_write+0x2c6/0x35e
>   [<00199456>] signal_pending+0x0/0x26
>   [<000aba2a>] __kvmalloc_node_noprof+0x3a/0x114
>   [<00004cc0>] io_uring_try_cancel_requests+0x98/0x318
>   [<0004fb2e>] woken_wake_function+0x0/0x24
>   [<0019a180>] file_tty_write.isra.0+0x144/0x1b8
>   [<0019a206>] tty_write+0x12/0x16
>   [<000b97c2>] vfs_write+0xec/0x148
>   [<00028000>] fp_getdest+0x1b8/0x224
>   [<00010000>] g_trace+0x16/0x28
>   [<000b9916>] ksys_write+0x54/0x8a
>   [<000b9962>] sys_write+0x16/0x1a
>   [<000093da>] syscall+0x8/0xc
>   [<0000c001>] arch_dma_prep_coherent+0x51/0x58
> 
> ---[ end trace 0000000000000000 ]---
> 
> It still happens on v6.17-rc2.  Reverting commits 77da18de55ac6417
> ("hung_task: extend hung task blocker tracking to rwsems") and
> 194a9b9e843b4077 ("hung_task: show the blocker task if the task is
> hung on semaphore") fixes the issue for me.

Thanks! I'm looking into it.

Lance

> 
> Thanks!
> 
> [1] "v6.16 console issues on Atari Falcon"
>      https://lore.kernel.org/all/92518308-c763-4591-96ef-6b38c5d8f434@helsinkinet.fi
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 


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

* Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore
  2025-08-22 15:18     ` Lance Yang
@ 2025-08-22 15:37       ` Geert Uytterhoeven
  2025-08-22 16:42         ` Lance Yang
  0 siblings, 1 reply; 27+ messages in thread
From: Geert Uytterhoeven @ 2025-08-22 15:37 UTC (permalink / raw)
  To: Lance Yang
  Cc: senozhatsky, mhiramat, akpm, will, peterz, mingo, longman,
	anna.schumaker, boqun.feng, joel.granados, kent.overstreet,
	leonylgao, linux-kernel, rostedt, tfiga, amaindex, jstultz,
	Mingzhe Yang, Eero Tamminen, linux-m68k, Lance Yang

Hi Lance,

On Fri, 22 Aug 2025 at 17:18, Lance Yang <lance.yang@linux.dev> wrote:
> On 2025/8/22 15:38, Geert Uytterhoeven wrote:
> > (this time the right email thread, I hope ;-)
> >
> > On Mon, 14 Apr 2025 at 17:23, Lance Yang <ioworker0@gmail.com> wrote:
> >> Inspired by mutex blocker tracking[1], this patch makes a trade-off to
> >> balance the overhead and utility of the hung task detector.
> >>
> >> Unlike mutexes, semaphores lack explicit ownership tracking, making it
> >> challenging to identify the root cause of hangs. To address this, we
> >> introduce a last_holder field to the semaphore structure, which is
> >> updated when a task successfully calls down() and cleared during up().
> >>
> >> The assumption is that if a task is blocked on a semaphore, the holders
> >> must not have released it. While this does not guarantee that the last
> >> holder is one of the current blockers, it likely provides a practical hint
> >> for diagnosing semaphore-related stalls.
> >>
> [...]
> >
> > Thanks for your patch, which is now commit 194a9b9e843b4077
> > ("hung_task: show the blocker task if the task is hung on
> > semaphore") in v6.16-rc1.
> >
> > Eero reported [1] two WARNINGS seen with v6.16 on emulated Atari.
> > I managed to reproduce it on ARAnyM using the provided config (it does
> > not happen with atari_defconfig), and bisected it to this commit:
>
> The two warnings are directly related, and the first one
> is the root cause, IIUC.
>
> >
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 39 at include/linux/hung_task.h:48
>
> The first warning at hung_task.h:48 is triggered because
> WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK) check fails.
>
> static inline void hung_task_set_blocker(void *lock, unsigned long type)
> {
>         unsigned long lock_ptr = (unsigned long)lock;
>
>         WARN_ON_ONCE(!lock_ptr);
>         WARN_ON_ONCE(READ_ONCE(current->blocker));
>
>         /*
>          * If the lock pointer matches the BLOCKER_TYPE_MASK, return
>          * without writing anything.
>          */
>         if (WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK)) <- here
>                 return;
>
> This logic assumes the lock pointer is sufficiently aligned,
> allowing the lower bits to be used for the lock type. But it
> appears we are being passed an unaligned lock pointer,
> unfortunately.

Thanks, that gives me a clue...

include/linux/hung_task.h-/*
include/linux/hung_task.h- * @blocker: Combines lock address and blocking type.
include/linux/hung_task.h- *
include/linux/hung_task.h- * Since lock pointers are at least 4-byte
aligned(32-bit) or 8-byte
include/linux/hung_task.h- * aligned(64-bit). This leaves the 2 least
bits (LSBs) of the pointer
include/linux/hung_task.h- * always zero. So we can use these bits to
encode the specific blocking
include/linux/hung_task.h- * type.
include/linux/hung_task.h- *
include/linux/hung_task.h- * Type encoding:
include/linux/hung_task.h- * 00 - Blocked on mutex
 (BLOCKER_TYPE_MUTEX)
include/linux/hung_task.h- * 01 - Blocked on semaphore
 (BLOCKER_TYPE_SEM)
include/linux/hung_task.h- * 10 - Blocked on rw-semaphore as READER
 (BLOCKER_TYPE_RWSEM_READER)
include/linux/hung_task.h- * 11 - Blocked on rw-semaphore as WRITER
 (BLOCKER_TYPE_RWSEM_WRITER)
include/linux/hung_task.h- */
include/linux/hung_task.h-#define BLOCKER_TYPE_MUTEX            0x00UL
include/linux/hung_task.h-#define BLOCKER_TYPE_SEM              0x01UL
include/linux/hung_task.h-#define BLOCKER_TYPE_RWSEM_READER     0x02UL
include/linux/hung_task.h-#define BLOCKER_TYPE_RWSEM_WRITER     0x03UL
include/linux/hung_task.h-
include/linux/hung_task.h:#define BLOCKER_TYPE_MASK             0x03UL

On m68k, the minimum alignment of int and larger is 2 bytes.
If you want to use the lowest 2 bits of a pointer for your own use,
you must make sure data is sufficiently aligned.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore
  2025-08-22 15:37       ` Geert Uytterhoeven
@ 2025-08-22 16:42         ` Lance Yang
  2025-08-23  0:27           ` Finn Thain
  0 siblings, 1 reply; 27+ messages in thread
From: Lance Yang @ 2025-08-22 16:42 UTC (permalink / raw)
  To: Geert Uytterhoeven, mhiramat
  Cc: akpm, will, peterz, mingo, longman, anna.schumaker, boqun.feng,
	joel.granados, kent.overstreet, leonylgao, linux-kernel, rostedt,
	tfiga, amaindex, jstultz, Mingzhe Yang, Eero Tamminen, linux-m68k,
	Lance Yang, senozhatsky

@Masami

On 2025/8/22 23:37, Geert Uytterhoeven wrote:
> Hi Lance,
> 
> On Fri, 22 Aug 2025 at 17:18, Lance Yang <lance.yang@linux.dev> wrote:
>> On 2025/8/22 15:38, Geert Uytterhoeven wrote:
>>> (this time the right email thread, I hope ;-)
>>>
>>> On Mon, 14 Apr 2025 at 17:23, Lance Yang <ioworker0@gmail.com> wrote:
>>>> Inspired by mutex blocker tracking[1], this patch makes a trade-off to
>>>> balance the overhead and utility of the hung task detector.
>>>>
>>>> Unlike mutexes, semaphores lack explicit ownership tracking, making it
>>>> challenging to identify the root cause of hangs. To address this, we
>>>> introduce a last_holder field to the semaphore structure, which is
>>>> updated when a task successfully calls down() and cleared during up().
>>>>
>>>> The assumption is that if a task is blocked on a semaphore, the holders
>>>> must not have released it. While this does not guarantee that the last
>>>> holder is one of the current blockers, it likely provides a practical hint
>>>> for diagnosing semaphore-related stalls.
>>>>
>> [...]
>>>
>>> Thanks for your patch, which is now commit 194a9b9e843b4077
>>> ("hung_task: show the blocker task if the task is hung on
>>> semaphore") in v6.16-rc1.
>>>
>>> Eero reported [1] two WARNINGS seen with v6.16 on emulated Atari.
>>> I managed to reproduce it on ARAnyM using the provided config (it does
>>> not happen with atari_defconfig), and bisected it to this commit:
>>
>> The two warnings are directly related, and the first one
>> is the root cause, IIUC.
>>
>>>
>>> ------------[ cut here ]------------
>>> WARNING: CPU: 0 PID: 39 at include/linux/hung_task.h:48
>>
>> The first warning at hung_task.h:48 is triggered because
>> WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK) check fails.
>>
>> static inline void hung_task_set_blocker(void *lock, unsigned long type)
>> {
>>          unsigned long lock_ptr = (unsigned long)lock;
>>
>>          WARN_ON_ONCE(!lock_ptr);
>>          WARN_ON_ONCE(READ_ONCE(current->blocker));
>>
>>          /*
>>           * If the lock pointer matches the BLOCKER_TYPE_MASK, return
>>           * without writing anything.
>>           */
>>          if (WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK)) <- here
>>                  return;
>>
>> This logic assumes the lock pointer is sufficiently aligned,
>> allowing the lower bits to be used for the lock type. But it
>> appears we are being passed an unaligned lock pointer,
>> unfortunately.
> 
> Thanks, that gives me a clue...
> 
> include/linux/hung_task.h-/*
> include/linux/hung_task.h- * @blocker: Combines lock address and blocking type.
> include/linux/hung_task.h- *
> include/linux/hung_task.h- * Since lock pointers are at least 4-byte
> aligned(32-bit) or 8-byte
> include/linux/hung_task.h- * aligned(64-bit). This leaves the 2 least
> bits (LSBs) of the pointer
> include/linux/hung_task.h- * always zero. So we can use these bits to
> encode the specific blocking
> include/linux/hung_task.h- * type.
> include/linux/hung_task.h- *
> include/linux/hung_task.h- * Type encoding:
> include/linux/hung_task.h- * 00 - Blocked on mutex
>   (BLOCKER_TYPE_MUTEX)
> include/linux/hung_task.h- * 01 - Blocked on semaphore
>   (BLOCKER_TYPE_SEM)
> include/linux/hung_task.h- * 10 - Blocked on rw-semaphore as READER
>   (BLOCKER_TYPE_RWSEM_READER)
> include/linux/hung_task.h- * 11 - Blocked on rw-semaphore as WRITER
>   (BLOCKER_TYPE_RWSEM_WRITER)
> include/linux/hung_task.h- */
> include/linux/hung_task.h-#define BLOCKER_TYPE_MUTEX            0x00UL
> include/linux/hung_task.h-#define BLOCKER_TYPE_SEM              0x01UL
> include/linux/hung_task.h-#define BLOCKER_TYPE_RWSEM_READER     0x02UL
> include/linux/hung_task.h-#define BLOCKER_TYPE_RWSEM_WRITER     0x03UL
> include/linux/hung_task.h-
> include/linux/hung_task.h:#define BLOCKER_TYPE_MASK             0x03UL
> 
> On m68k, the minimum alignment of int and larger is 2 bytes.

Ah, thanks, that's good to know! It clearly explains why the
WARN_ON_ONCE() is triggering.

> If you want to use the lowest 2 bits of a pointer for your own use,
> you must make sure data is sufficiently aligned.

You're right. Apparently I missed that :(

I'm wondering if there's a way to check an architecture's minimum
alignment at compile-time. If so, we could disable this feature on
architectures that don't guarantee 4-byte alignment.

If not, the fallback is to adjust the runtime checks. We could change
the first WARN_ON_ONCE() to a simple if that returns silently for
unaligned pointers. Then we can just remove the second WARN_ON_ONCE()
in hung_task_clear_blocker() altogether.

static inline void hung_task_set_blocker(void *lock, unsigned long type)
{
	unsigned long lock_ptr = (unsigned long)lock;

	WARN_ON_ONCE(!lock_ptr);
	WARN_ON_ONCE(READ_ONCE(current->blocker));

	/*
	 * If the lock pointer matches the BLOCKER_TYPE_MASK, return
	 * without writing anything.
	 */
	if (lock_ptr & BLOCKER_TYPE_MASK)
		return;

	WRITE_ONCE(current->blocker, lock_ptr | type);
}

static inline void hung_task_clear_blocker(void)
{
	WRITE_ONCE(current->blocker, 0UL);
}

This would fix both warnings and let the feature gracefully do nothing
on architectures like m68k.

Thanks,
Lance

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

* Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore
  2025-08-22 16:42         ` Lance Yang
@ 2025-08-23  0:27           ` Finn Thain
  2025-08-23  4:47             ` Lance Yang
  0 siblings, 1 reply; 27+ messages in thread
From: Finn Thain @ 2025-08-23  0:27 UTC (permalink / raw)
  To: Lance Yang
  Cc: Geert Uytterhoeven, mhiramat, akpm, will, peterz, mingo, longman,
	anna.schumaker, boqun.feng, joel.granados, kent.overstreet,
	leonylgao, linux-kernel, rostedt, tfiga, amaindex, jstultz,
	Mingzhe Yang, Eero Tamminen, linux-m68k, Lance Yang, senozhatsky


On Sat, 23 Aug 2025, Lance Yang wrote:

> > 
> > include/linux/hung_task.h-/*
> > include/linux/hung_task.h- * @blocker: Combines lock address and blocking type.
> > include/linux/hung_task.h- *
> > include/linux/hung_task.h- * Since lock pointers are at least 4-byte aligned(32-bit) or 8-byte
> > include/linux/hung_task.h- * aligned(64-bit). This leaves the 2 least bits (LSBs) of the pointer
> > include/linux/hung_task.h- * always zero. So we can use these bits to encode the specific blocking
> > include/linux/hung_task.h- * type.
> > include/linux/hung_task.h- *

That comment was introduced in commit e711faaafbe5 ("hung_task: replace 
blocker_mutex with encoded blocker"). It's wrong and should be fixed.

> > include/linux/hung_task.h- * Type encoding:
> > include/linux/hung_task.h- * 00 - Blocked on mutex
> >   (BLOCKER_TYPE_MUTEX)
> > include/linux/hung_task.h- * 01 - Blocked on semaphore
> >   (BLOCKER_TYPE_SEM)
> > include/linux/hung_task.h- * 10 - Blocked on rw-semaphore as READER
> >   (BLOCKER_TYPE_RWSEM_READER)
> > include/linux/hung_task.h- * 11 - Blocked on rw-semaphore as WRITER
> >   (BLOCKER_TYPE_RWSEM_WRITER)
> > include/linux/hung_task.h- */
> > include/linux/hung_task.h-#define BLOCKER_TYPE_MUTEX            0x00UL
> > include/linux/hung_task.h-#define BLOCKER_TYPE_SEM              0x01UL
> > include/linux/hung_task.h-#define BLOCKER_TYPE_RWSEM_READER     0x02UL
> > include/linux/hung_task.h-#define BLOCKER_TYPE_RWSEM_WRITER     0x03UL
> > include/linux/hung_task.h-
> > include/linux/hung_task.h:#define BLOCKER_TYPE_MASK             0x03UL
> > 
> > On m68k, the minimum alignment of int and larger is 2 bytes.
> 
> Ah, thanks, that's good to know! It clearly explains why the
> WARN_ON_ONCE() is triggering.
> 
> > If you want to use the lowest 2 bits of a pointer for your own use,
> > you must make sure data is sufficiently aligned.
> 
> You're right. Apparently I missed that :(
> 
> I'm wondering if there's a way to check an architecture's minimum 
> alignment at compile-time. If so, we could disable this feature on 
> architectures that don't guarantee 4-byte alignment.
> 

As Geert says, the compiler can give you all the bits you need, so you 
won't have to contort your algorithm to fit whatever free bits happen to 
be available. Please see for example, commit 258a980d1ec2 ("net: dst: 
Force 4-byte alignment of dst_metrics").

> If not, the fallback is to adjust the runtime checks.
> 

That would be a solution to a different problem.

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

* Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore
  2025-08-23  0:27           ` Finn Thain
@ 2025-08-23  4:47             ` Lance Yang
  2025-08-23  5:00               ` [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers Lance Yang
                                 ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Lance Yang @ 2025-08-23  4:47 UTC (permalink / raw)
  To: Finn Thain, Geert Uytterhoeven, mhiramat
  Cc: akpm, will, peterz, mingo, longman, anna.schumaker, boqun.feng,
	joel.granados, kent.overstreet, leonylgao, linux-kernel, rostedt,
	tfiga, amaindex, jstultz, Mingzhe Yang, Eero Tamminen, linux-m68k,
	Lance Yang, senozhatsky

Hi Finn,

On 2025/8/23 08:27, Finn Thain wrote:
> 
> On Sat, 23 Aug 2025, Lance Yang wrote:
> 
>>>
>>> include/linux/hung_task.h-/*
>>> include/linux/hung_task.h- * @blocker: Combines lock address and blocking type.
>>> include/linux/hung_task.h- *
>>> include/linux/hung_task.h- * Since lock pointers are at least 4-byte aligned(32-bit) or 8-byte
>>> include/linux/hung_task.h- * aligned(64-bit). This leaves the 2 least bits (LSBs) of the pointer
>>> include/linux/hung_task.h- * always zero. So we can use these bits to encode the specific blocking
>>> include/linux/hung_task.h- * type.
>>> include/linux/hung_task.h- *
> 
> That comment was introduced in commit e711faaafbe5 ("hung_task: replace
> blocker_mutex with encoded blocker"). It's wrong and should be fixed.

Right, the problematic assumption was introduced in that commit ;)

> 
>>> include/linux/hung_task.h- * Type encoding:
>>> include/linux/hung_task.h- * 00 - Blocked on mutex
>>>    (BLOCKER_TYPE_MUTEX)
>>> include/linux/hung_task.h- * 01 - Blocked on semaphore
>>>    (BLOCKER_TYPE_SEM)
>>> include/linux/hung_task.h- * 10 - Blocked on rw-semaphore as READER
>>>    (BLOCKER_TYPE_RWSEM_READER)
>>> include/linux/hung_task.h- * 11 - Blocked on rw-semaphore as WRITER
>>>    (BLOCKER_TYPE_RWSEM_WRITER)
>>> include/linux/hung_task.h- */
>>> include/linux/hung_task.h-#define BLOCKER_TYPE_MUTEX            0x00UL
>>> include/linux/hung_task.h-#define BLOCKER_TYPE_SEM              0x01UL
>>> include/linux/hung_task.h-#define BLOCKER_TYPE_RWSEM_READER     0x02UL
>>> include/linux/hung_task.h-#define BLOCKER_TYPE_RWSEM_WRITER     0x03UL
>>> include/linux/hung_task.h-
>>> include/linux/hung_task.h:#define BLOCKER_TYPE_MASK             0x03UL
>>>
>>> On m68k, the minimum alignment of int and larger is 2 bytes.
>>
>> Ah, thanks, that's good to know! It clearly explains why the
>> WARN_ON_ONCE() is triggering.
>>
>>> If you want to use the lowest 2 bits of a pointer for your own use,
>>> you must make sure data is sufficiently aligned.
>>
>> You're right. Apparently I missed that :(
>>
>> I'm wondering if there's a way to check an architecture's minimum
>> alignment at compile-time. If so, we could disable this feature on
>> architectures that don't guarantee 4-byte alignment.
>>
> 
> As Geert says, the compiler can give you all the bits you need, so you
> won't have to contort your algorithm to fit whatever free bits happen to
> be available. Please see for example, commit 258a980d1ec2 ("net: dst:
> Force 4-byte alignment of dst_metrics").

Yes, thanks, it's a helpful example!

I see your point that explicitly enforcing alignment is a very clean
solution for the lock structures supported by the blocker tracking
mechanism.

However, I'm thinking about the "principle of minimal impact" here.
Forcing alignment on the core lock types themselves — like struct
semaphore — feels like a broad change to fix an issue that's local to the
hung task detector :)

> 
>> If not, the fallback is to adjust the runtime checks.
>>
> 
> That would be a solution to a different problem.

For that reason, I would prefer to simply adjust the runtime checks within
the hung task detector. It feels like a more generic and self-contained
solution. It works out-of-the-box for the majority of architectures and
provides a safe fallback for those that aren't.

Happy to hear what you and others think about this trade-off. Perhaps
there's a perspective I'm missing ;)

Thanks,
Lance

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

* [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers
  2025-08-23  4:47             ` Lance Yang
@ 2025-08-23  5:00               ` Lance Yang
  2025-08-26  4:49                 ` Masami Hiramatsu
  2025-08-23  7:40               ` [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures Lance Yang
  2025-08-23  7:49               ` [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore Lance Yang
  2 siblings, 1 reply; 27+ messages in thread
From: Lance Yang @ 2025-08-23  5:00 UTC (permalink / raw)
  To: akpm, fthain, geert, mhiramat, senozhatsky
  Cc: lance.yang, amaindex, anna.schumaker, boqun.feng, ioworker0,
	joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
	linux-m68k, longman, mingo, mingzhe.yang, oak, peterz, rostedt,
	tfiga, will, stable

From: Lance Yang <lance.yang@linux.dev>

The blocker tracking mechanism assumes that lock pointers are at least
4-byte aligned to use their lower bits for type encoding.

However, as reported by Geert Uytterhoeven, some architectures like m68k
only guarantee 2-byte alignment of 32-bit values. This breaks the
assumption and causes two related WARN_ON_ONCE checks to trigger.

To fix this, the runtime checks are adjusted. The first WARN_ON_ONCE in
hung_task_set_blocker() is changed to a simple 'if' that returns silently
for unaligned pointers. The second, now-invalid WARN_ON_ONCE in
hung_task_clear_blocker() is then removed.

Thanks to Geert for bisecting!

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Closes: https://lore.kernel.org/lkml/CAMuHMdW7Ab13DdGs2acMQcix5ObJK0O2dG_Fxzr8_g58Rc1_0g@mail.gmail.com
Fixes: e711faaafbe5 ("hung_task: replace blocker_mutex with encoded blocker")
Cc: <stable@vger.kernel.org>
Signed-off-by: Lance Yang <lance.yang@linux.dev>
---
 include/linux/hung_task.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/hung_task.h b/include/linux/hung_task.h
index 34e615c76ca5..69640f266a69 100644
--- a/include/linux/hung_task.h
+++ b/include/linux/hung_task.h
@@ -20,6 +20,10 @@
  * always zero. So we can use these bits to encode the specific blocking
  * type.
  *
+ * Note that on architectures like m68k with only 2-byte alignment, the
+ * blocker tracking mechanism gracefully does nothing for any lock that is
+ * not 4-byte aligned.
+ *
  * Type encoding:
  * 00 - Blocked on mutex			(BLOCKER_TYPE_MUTEX)
  * 01 - Blocked on semaphore			(BLOCKER_TYPE_SEM)
@@ -45,7 +49,7 @@ static inline void hung_task_set_blocker(void *lock, unsigned long type)
 	 * If the lock pointer matches the BLOCKER_TYPE_MASK, return
 	 * without writing anything.
 	 */
-	if (WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK))
+	if (lock_ptr & BLOCKER_TYPE_MASK)
 		return;
 
 	WRITE_ONCE(current->blocker, lock_ptr | type);
@@ -53,8 +57,6 @@ static inline void hung_task_set_blocker(void *lock, unsigned long type)
 
 static inline void hung_task_clear_blocker(void)
 {
-	WARN_ON_ONCE(!READ_ONCE(current->blocker));
-
 	WRITE_ONCE(current->blocker, 0UL);
 }
 
-- 
2.49.0


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

* [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
  2025-08-23  4:47             ` Lance Yang
  2025-08-23  5:00               ` [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers Lance Yang
@ 2025-08-23  7:40               ` Lance Yang
  2025-08-23 21:53                 ` kernel test robot
  2025-08-26  5:02                 ` Masami Hiramatsu
  2025-08-23  7:49               ` [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore Lance Yang
  2 siblings, 2 replies; 27+ messages in thread
From: Lance Yang @ 2025-08-23  7:40 UTC (permalink / raw)
  To: akpm, fthain, geert, mhiramat, senozhatsky
  Cc: lance.yang, amaindex, anna.schumaker, boqun.feng, ioworker0,
	joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
	linux-m68k, longman, mingo, mingzhe.yang, oak, peterz, rostedt,
	tfiga, will, stable

From: Lance Yang <lance.yang@linux.dev>

The blocker tracking mechanism assumes that lock pointers are at least
4-byte aligned to use their lower bits for type encoding.

However, as reported by Geert Uytterhoeven, some architectures like m68k
only guarantee 2-byte alignment of 32-bit values. This breaks the
assumption and causes two related WARN_ON_ONCE checks to trigger.

To fix this, enforce a minimum of 4-byte alignment on the core lock
structures supported by the blocker tracking mechanism. This ensures the
algorithm's alignment assumption now holds true on all architectures.

This patch adds __aligned(4) to the definitions of "struct mutex",
"struct semaphore", and "struct rw_semaphore", resolving the warnings.

Thanks to Geert for bisecting!

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Closes: https://lore.kernel.org/lkml/CAMuHMdW7Ab13DdGs2acMQcix5ObJK0O2dG_Fxzr8_g58Rc1_0g@mail.gmail.com
Fixes: e711faaafbe5 ("hung_task: replace blocker_mutex with encoded blocker")
Cc: <stable@vger.kernel.org>
Signed-off-by: Lance Yang <lance.yang@linux.dev>
---
 include/linux/mutex_types.h | 2 +-
 include/linux/rwsem.h       | 2 +-
 include/linux/semaphore.h   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/mutex_types.h b/include/linux/mutex_types.h
index fdf7f515fde8..de798bfbc4c7 100644
--- a/include/linux/mutex_types.h
+++ b/include/linux/mutex_types.h
@@ -51,7 +51,7 @@ struct mutex {
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 	struct lockdep_map	dep_map;
 #endif
-};
+} __aligned(4); /* For hung_task blocker tracking, which encodes type in LSBs */
 
 #else /* !CONFIG_PREEMPT_RT */
 /*
diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h
index f1aaf676a874..f6ecf4a4710d 100644
--- a/include/linux/rwsem.h
+++ b/include/linux/rwsem.h
@@ -64,7 +64,7 @@ struct rw_semaphore {
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 	struct lockdep_map	dep_map;
 #endif
-};
+} __aligned(4); /* For hung_task blocker tracking, which encodes type in LSBs */
 
 #define RWSEM_UNLOCKED_VALUE		0UL
 #define RWSEM_WRITER_LOCKED		(1UL << 0)
diff --git a/include/linux/semaphore.h b/include/linux/semaphore.h
index 89706157e622..ac9b9c87bfb7 100644
--- a/include/linux/semaphore.h
+++ b/include/linux/semaphore.h
@@ -20,7 +20,7 @@ struct semaphore {
 #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
 	unsigned long		last_holder;
 #endif
-};
+} __aligned(4); /* For hung_task blocker tracking, which encodes type in LSBs */
 
 #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
 #define __LAST_HOLDER_SEMAPHORE_INITIALIZER				\
-- 
2.49.0


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

* Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore
  2025-08-23  4:47             ` Lance Yang
  2025-08-23  5:00               ` [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers Lance Yang
  2025-08-23  7:40               ` [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures Lance Yang
@ 2025-08-23  7:49               ` Lance Yang
  2 siblings, 0 replies; 27+ messages in thread
From: Lance Yang @ 2025-08-23  7:49 UTC (permalink / raw)
  To: akpm, mhiramat, Finn Thain, Geert Uytterhoeven, senozhatsky
  Cc: will, peterz, mingo, longman, anna.schumaker, boqun.feng,
	joel.granados, kent.overstreet, leonylgao, linux-kernel, rostedt,
	tfiga, amaindex, jstultz, Mingzhe Yang, Eero Tamminen, linux-m68k,
	Lance Yang



On 2025/8/23 12:47, Lance Yang wrote:
> Hi Finn,
> 
> On 2025/8/23 08:27, Finn Thain wrote:
>>
>> On Sat, 23 Aug 2025, Lance Yang wrote:
>>
>>>>
>>>> include/linux/hung_task.h-/*
>>>> include/linux/hung_task.h- * @blocker: Combines lock address and 
>>>> blocking type.
>>>> include/linux/hung_task.h- *
>>>> include/linux/hung_task.h- * Since lock pointers are at least 4-byte 
>>>> aligned(32-bit) or 8-byte
>>>> include/linux/hung_task.h- * aligned(64-bit). This leaves the 2 
>>>> least bits (LSBs) of the pointer
>>>> include/linux/hung_task.h- * always zero. So we can use these bits 
>>>> to encode the specific blocking
>>>> include/linux/hung_task.h- * type.
>>>> include/linux/hung_task.h- *
>>
>> That comment was introduced in commit e711faaafbe5 ("hung_task: replace
>> blocker_mutex with encoded blocker"). It's wrong and should be fixed.
> 
> Right, the problematic assumption was introduced in that commit ;)
> 
>>
>>>> include/linux/hung_task.h- * Type encoding:
>>>> include/linux/hung_task.h- * 00 - Blocked on mutex
>>>>    (BLOCKER_TYPE_MUTEX)
>>>> include/linux/hung_task.h- * 01 - Blocked on semaphore
>>>>    (BLOCKER_TYPE_SEM)
>>>> include/linux/hung_task.h- * 10 - Blocked on rw-semaphore as READER
>>>>    (BLOCKER_TYPE_RWSEM_READER)
>>>> include/linux/hung_task.h- * 11 - Blocked on rw-semaphore as WRITER
>>>>    (BLOCKER_TYPE_RWSEM_WRITER)
>>>> include/linux/hung_task.h- */
>>>> include/linux/hung_task.h-#define BLOCKER_TYPE_MUTEX            0x00UL
>>>> include/linux/hung_task.h-#define BLOCKER_TYPE_SEM              0x01UL
>>>> include/linux/hung_task.h-#define BLOCKER_TYPE_RWSEM_READER     0x02UL
>>>> include/linux/hung_task.h-#define BLOCKER_TYPE_RWSEM_WRITER     0x03UL
>>>> include/linux/hung_task.h-
>>>> include/linux/hung_task.h:#define BLOCKER_TYPE_MASK             0x03UL
>>>>
>>>> On m68k, the minimum alignment of int and larger is 2 bytes.
>>>
>>> Ah, thanks, that's good to know! It clearly explains why the
>>> WARN_ON_ONCE() is triggering.
>>>
>>>> If you want to use the lowest 2 bits of a pointer for your own use,
>>>> you must make sure data is sufficiently aligned.
>>>
>>> You're right. Apparently I missed that :(
>>>
>>> I'm wondering if there's a way to check an architecture's minimum
>>> alignment at compile-time. If so, we could disable this feature on
>>> architectures that don't guarantee 4-byte alignment.
>>>
>>
>> As Geert says, the compiler can give you all the bits you need, so you
>> won't have to contort your algorithm to fit whatever free bits happen to
>> be available. Please see for example, commit 258a980d1ec2 ("net: dst:
>> Force 4-byte alignment of dst_metrics").
> 
> Yes, thanks, it's a helpful example!
> 
> I see your point that explicitly enforcing alignment is a very clean
> solution for the lock structures supported by the blocker tracking
> mechanism.
> 
> However, I'm thinking about the "principle of minimal impact" here.
> Forcing alignment on the core lock types themselves — like struct
> semaphore — feels like a broad change to fix an issue that's local to the
> hung task detector :)
> 
>>
>>> If not, the fallback is to adjust the runtime checks.
>>>
>>
>> That would be a solution to a different problem.
> 
> For that reason, I would prefer to simply adjust the runtime checks within
> the hung task detector. It feels like a more generic and self-contained
> solution. It works out-of-the-box for the majority of architectures and
> provides a safe fallback for those that aren't.
> 
> Happy to hear what you and others think about this trade-off. Perhaps
> there's a perspective I'm missing ;)

Anyway, I've prepared two patches for discussion, either of which should
fix the alignment issue :)

Patch A[1] adjusts the runtime checks to handle unaligned pointers.
Patch B[2] enforces 4-byte alignment on the core lock structures.

Both tested on x86-64.

[1] https://lore.kernel.org/lkml/20250823050036.7748-1-lance.yang@linux.dev
[2] https://lore.kernel.org/lkml/20250823074048.92498-1-lance.yang@linux.dev

Thanks,
Lance

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

* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
  2025-08-23  7:40               ` [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures Lance Yang
@ 2025-08-23 21:53                 ` kernel test robot
  2025-08-24  0:47                   ` Finn Thain
  2025-08-26  5:02                 ` Masami Hiramatsu
  1 sibling, 1 reply; 27+ messages in thread
From: kernel test robot @ 2025-08-23 21:53 UTC (permalink / raw)
  To: Lance Yang, akpm, fthain, geert, mhiramat, senozhatsky
  Cc: oe-kbuild-all, lance.yang, amaindex, anna.schumaker, boqun.feng,
	ioworker0, joel.granados, jstultz, kent.overstreet, leonylgao,
	linux-kernel, linux-m68k, longman, mingo, mingzhe.yang, oak,
	rostedt, tfiga, will, stable

Hi Lance,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/locking/core]
[also build test WARNING on akpm-mm/mm-everything sysctl/sysctl-next linus/master v6.17-rc2 next-20250822]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lance-Yang/hung_task-fix-warnings-by-enforcing-alignment-on-lock-structures/20250823-164122
base:   tip/locking/core
patch link:    https://lore.kernel.org/r/20250823074048.92498-1-lance.yang%40linux.dev
patch subject: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
config: x86_64-buildonly-randconfig-001-20250823 (https://download.01.org/0day-ci/archive/20250824/202508240539.ARmC1Umu-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250824/202508240539.ARmC1Umu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508240539.ARmC1Umu-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from sound/soc/codecs/mt6660.c:15:
>> sound/soc/codecs/mt6660.h:28:1: warning: alignment 1 of 'struct mt6660_chip' is less than 8 [-Wpacked-not-aligned]
      28 | };
         | ^
>> sound/soc/codecs/mt6660.h:25:22: warning: 'io_lock' offset 49 in 'struct mt6660_chip' isn't aligned to 8 [-Wpacked-not-aligned]
      25 |         struct mutex io_lock;
         |                      ^~~~~~~


vim +28 sound/soc/codecs/mt6660.h

f289e55c6eeb43 Jeff Chang 2020-01-16  19  
f289e55c6eeb43 Jeff Chang 2020-01-16  20  struct mt6660_chip {
f289e55c6eeb43 Jeff Chang 2020-01-16  21  	struct i2c_client *i2c;
f289e55c6eeb43 Jeff Chang 2020-01-16  22  	struct device *dev;
f289e55c6eeb43 Jeff Chang 2020-01-16  23  	struct platform_device *param_dev;
f289e55c6eeb43 Jeff Chang 2020-01-16  24  	struct mt6660_platform_data plat_data;
f289e55c6eeb43 Jeff Chang 2020-01-16 @25  	struct mutex io_lock;
f289e55c6eeb43 Jeff Chang 2020-01-16  26  	struct regmap *regmap;
f289e55c6eeb43 Jeff Chang 2020-01-16  27  	u16 chip_rev;
f289e55c6eeb43 Jeff Chang 2020-01-16 @28  };
f289e55c6eeb43 Jeff Chang 2020-01-16  29  #pragma pack(pop)
f289e55c6eeb43 Jeff Chang 2020-01-16  30  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
  2025-08-23 21:53                 ` kernel test robot
@ 2025-08-24  0:47                   ` Finn Thain
  2025-08-24  3:03                     ` Lance Yang
  0 siblings, 1 reply; 27+ messages in thread
From: Finn Thain @ 2025-08-24  0:47 UTC (permalink / raw)
  To: Lance Yang
  Cc: kernel test robot, akpm, geert, mhiramat, senozhatsky,
	oe-kbuild-all, amaindex, anna.schumaker, boqun.feng, ioworker0,
	joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
	linux-m68k, longman, mingo, mingzhe.yang, oak, rostedt, tfiga,
	will, stable


On Sun, 24 Aug 2025, kernel test robot wrote:

> 
> All warnings (new ones prefixed by >>):
> 
>    In file included from sound/soc/codecs/mt6660.c:15:
> >> sound/soc/codecs/mt6660.h:28:1: warning: alignment 1 of 'struct mt6660_chip' is less than 8 [-Wpacked-not-aligned]
>       28 | };
>          | ^
> >> sound/soc/codecs/mt6660.h:25:22: warning: 'io_lock' offset 49 in 'struct mt6660_chip' isn't aligned to 8 [-Wpacked-not-aligned]
>       25 |         struct mutex io_lock;
>          |                      ^~~~~~~
> 

Misalignment warnings like this one won't work if you just pick an 
alignment arbitrarily i.e. to suit whatever bitfield you happen to need.

Instead, I think I would naturally align the actual locks, that is, 
arch_spinlock_t and arch_rwlock_t in include/linux/spinlock_types*.h.

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

* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
  2025-08-24  0:47                   ` Finn Thain
@ 2025-08-24  3:03                     ` Lance Yang
  2025-08-24  4:18                       ` Finn Thain
  0 siblings, 1 reply; 27+ messages in thread
From: Lance Yang @ 2025-08-24  3:03 UTC (permalink / raw)
  To: Finn Thain, akpm, mhiramat
  Cc: kernel test robot, geert, senozhatsky, oe-kbuild-all, amaindex,
	anna.schumaker, boqun.feng, ioworker0, joel.granados, jstultz,
	kent.overstreet, leonylgao, linux-kernel, linux-m68k, longman,
	mingo, mingzhe.yang, oak, rostedt, tfiga, will, stable

Hi Finn, Hi all,

Thanks to the kernel test robot for finding this issue, and thank you,
Finn, for the explanation!

On 2025/8/24 08:47, Finn Thain wrote:
> 
> On Sun, 24 Aug 2025, kernel test robot wrote:
> 
>>
>> All warnings (new ones prefixed by >>):
>>
>>     In file included from sound/soc/codecs/mt6660.c:15:
>>>> sound/soc/codecs/mt6660.h:28:1: warning: alignment 1 of 'struct mt6660_chip' is less than 8 [-Wpacked-not-aligned]
>>        28 | };
>>           | ^
>>>> sound/soc/codecs/mt6660.h:25:22: warning: 'io_lock' offset 49 in 'struct mt6660_chip' isn't aligned to 8 [-Wpacked-not-aligned]
>>        25 |         struct mutex io_lock;
>>           |                      ^~~~~~~
>>
> 
> Misalignment warnings like this one won't work if you just pick an
> alignment arbitrarily i.e. to suit whatever bitfield you happen to need.

Yes.

The build warnings reported by the test robot are exactly the kind of
unintended side effect I was concerned about. It confirms that forcing
alignment on a core structure like struct mutex breaks other parts of
the kernel that rely on packed structures ;)

> 
> Instead, I think I would naturally align the actual locks, that is,
> arch_spinlock_t and arch_rwlock_t in include/linux/spinlock_types*.h.

That's an interesting point. The blocker tracking mechanism currently
operates on higher-level structures like struct mutex. Moving the type
encoding down to the lowest-level locks would be a more complex and
invasive change, likely beyond the scope of fixing this particular issue.

Looking further ahead, a better long-term solution might be to stop
repurposing pointer bits altogether. We could add an explicit blocker_type
field to task_struct to be used alongside the blocker field. That would be
a much cleaner design. TODO +1 for that idea :)

So, let's drop the patch[1] that enforces alignment and go back to my
initial proposal[2], which adjusts the runtime checks to gracefully handle
unaligned pointers. That one is self-contained, has minimal impact, and is
clearly the safer solution for now.

[1] https://lore.kernel.org/lkml/20250823074048.92498-1-lance.yang@linux.dev
[2] https://lore.kernel.org/lkml/20250823050036.7748-1-lance.yang@linux.dev

Thanks,
Lance

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

* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
  2025-08-24  3:03                     ` Lance Yang
@ 2025-08-24  4:18                       ` Finn Thain
  2025-08-24  5:02                         ` Lance Yang
  0 siblings, 1 reply; 27+ messages in thread
From: Finn Thain @ 2025-08-24  4:18 UTC (permalink / raw)
  To: Lance Yang
  Cc: akpm, mhiramat, kernel test robot, geert, senozhatsky,
	oe-kbuild-all, amaindex, anna.schumaker, boqun.feng, ioworker0,
	joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
	linux-m68k, longman, mingo, mingzhe.yang, oak, rostedt, tfiga,
	will, stable


On Sun, 24 Aug 2025, Lance Yang wrote:

> On 2025/8/24 08:47, Finn Thain wrote:
> > 
> > On Sun, 24 Aug 2025, kernel test robot wrote:
> > 
> >> All warnings (new ones prefixed by >>):
> >>
> >>     In file included from sound/soc/codecs/mt6660.c:15:
> >>>> sound/soc/codecs/mt6660.h:28:1: warning: alignment 1 of 'struct
> >>>> mt6660_chip' is less than 8 [-Wpacked-not-aligned]
> >>        28 | };
> >>           | ^
> >>>> sound/soc/codecs/mt6660.h:25:22: warning: 'io_lock' offset 49 in 'struct
> >>>> mt6660_chip' isn't aligned to 8 [-Wpacked-not-aligned]
> >>        25 |         struct mutex io_lock;
> >>           |                      ^~~~~~~
> >>
> > 
> > Misalignment warnings like this one won't work if you just pick an
> > alignment arbitrarily i.e. to suit whatever bitfield you happen to need.
> 
> Yes.
> 
> The build warnings reported by the test robot are exactly the kind of
> unintended side effect I was concerned about. It confirms that forcing
> alignment on a core structure like struct mutex breaks other parts of
> the kernel that rely on packed structures ;)
> 

Sure, your patch broke the build. So why not write a better patch? You 
don't need to align the struct, you need to align the lock, like I said 
already.

> > 
> > Instead, I think I would naturally align the actual locks, that is, 
> > arch_spinlock_t and arch_rwlock_t in include/linux/spinlock_types*.h.
> 
> That's an interesting point. The blocker tracking mechanism currently 
> operates on higher-level structures like struct mutex. Moving the type 
> encoding down to the lowest-level locks would be a more complex and 
> invasive change, likely beyond the scope of fixing this particular 
> issue.
> 

I don't see why changing kernel struct layouts on m68k is particularly 
invasive. Perhaps I'm missing something (?)

> Looking further ahead, a better long-term solution might be to stop 
> repurposing pointer bits altogether. We could add an explicit 
> blocker_type field to task_struct to be used alongside the blocker 
> field. That would be a much cleaner design. TODO +1 for that idea :)
> 
> So, let's drop the patch[1] that enforces alignment and go back to my 
> initial proposal[2], which adjusts the runtime checks to gracefully 
> handle unaligned pointers. That one is self-contained, has minimal 
> impact, and is clearly the safer solution for now.
> 
> [1] https://lore.kernel.org/lkml/20250823074048.92498-1-lance.yang@linux.dev
> [2] https://lore.kernel.org/lkml/20250823050036.7748-1-lance.yang@linux.dev
> 

I am willing to send a patch if it serves correctness and portability. So 
you may wish to refrain from crippling your blocker tracking algorithm for 
now.

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

* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
  2025-08-24  4:18                       ` Finn Thain
@ 2025-08-24  5:02                         ` Lance Yang
  2025-08-24  5:57                           ` Finn Thain
  0 siblings, 1 reply; 27+ messages in thread
From: Lance Yang @ 2025-08-24  5:02 UTC (permalink / raw)
  To: Finn Thain
  Cc: akpm, mhiramat, kernel test robot, geert, senozhatsky,
	oe-kbuild-all, amaindex, anna.schumaker, boqun.feng, ioworker0,
	joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
	linux-m68k, longman, mingo, mingzhe.yang, oak, rostedt, tfiga,
	will, stable



On 2025/8/24 12:18, Finn Thain wrote:
> 
> On Sun, 24 Aug 2025, Lance Yang wrote:
> 
>> On 2025/8/24 08:47, Finn Thain wrote:
>>>
>>> On Sun, 24 Aug 2025, kernel test robot wrote:
>>>
>>>> All warnings (new ones prefixed by >>):
>>>>
>>>>      In file included from sound/soc/codecs/mt6660.c:15:
>>>>>> sound/soc/codecs/mt6660.h:28:1: warning: alignment 1 of 'struct
>>>>>> mt6660_chip' is less than 8 [-Wpacked-not-aligned]
>>>>         28 | };
>>>>            | ^
>>>>>> sound/soc/codecs/mt6660.h:25:22: warning: 'io_lock' offset 49 in 'struct
>>>>>> mt6660_chip' isn't aligned to 8 [-Wpacked-not-aligned]
>>>>         25 |         struct mutex io_lock;
>>>>            |                      ^~~~~~~
>>>>
>>>
>>> Misalignment warnings like this one won't work if you just pick an
>>> alignment arbitrarily i.e. to suit whatever bitfield you happen to need.
>>
>> Yes.
>>
>> The build warnings reported by the test robot are exactly the kind of
>> unintended side effect I was concerned about. It confirms that forcing
>> alignment on a core structure like struct mutex breaks other parts of
>> the kernel that rely on packed structures ;)
>>
> 
> Sure, your patch broke the build. So why not write a better patch? You
> don't need to align the struct, you need to align the lock, like I said
> already.

I think there might be a misunderstanding about the level of abstraction
at which the blocker tracking operates.

The blocker tracking mechanism operates on pointers to higher-level
locks (like struct mutex), as that is what is stored in the
task_struct->blocker field. It does not operate on the lower-level
arch_spinlock_t inside it.

While we could track the internal arch_spinlock_t, that would break
encapsulation. The hung task detector should remain generic and not
depend on lock-specific implementation details ;)

> 
>>>
>>> Instead, I think I would naturally align the actual locks, that is,
>>> arch_spinlock_t and arch_rwlock_t in include/linux/spinlock_types*.h.
>>
>> That's an interesting point. The blocker tracking mechanism currently
>> operates on higher-level structures like struct mutex. Moving the type
>> encoding down to the lowest-level locks would be a more complex and
>> invasive change, likely beyond the scope of fixing this particular
>> issue.
>>
> 
> I don't see why changing kernel struct layouts on m68k is particularly
> invasive. Perhaps I'm missing something (?)
> 
>> Looking further ahead, a better long-term solution might be to stop
>> repurposing pointer bits altogether. We could add an explicit
>> blocker_type field to task_struct to be used alongside the blocker
>> field. That would be a much cleaner design. TODO +1 for that idea :)
>>
>> So, let's drop the patch[1] that enforces alignment and go back to my
>> initial proposal[2], which adjusts the runtime checks to gracefully
>> handle unaligned pointers. That one is self-contained, has minimal
>> impact, and is clearly the safer solution for now.
>>
>> [1] https://lore.kernel.org/lkml/20250823074048.92498-1-lance.yang@linux.dev
>> [2] https://lore.kernel.org/lkml/20250823050036.7748-1-lance.yang@linux.dev
>>
> 
> I am willing to send a patch if it serves correctness and portability. So
> you may wish to refrain from crippling your blocker tracking algorithm for
> now.


Completely agreed that correctness and portability are the goals.

Please, feel free to send a patch.

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

* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
  2025-08-24  5:02                         ` Lance Yang
@ 2025-08-24  5:57                           ` Finn Thain
  2025-08-24  6:18                             ` Lance Yang
  0 siblings, 1 reply; 27+ messages in thread
From: Finn Thain @ 2025-08-24  5:57 UTC (permalink / raw)
  To: Lance Yang
  Cc: akpm, mhiramat, kernel test robot, geert, senozhatsky,
	oe-kbuild-all, amaindex, anna.schumaker, boqun.feng, ioworker0,
	joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
	linux-m68k, longman, mingo, mingzhe.yang, oak, rostedt, tfiga,
	will, stable


On Sun, 24 Aug 2025, Lance Yang wrote:

> 
> The blocker tracking mechanism operates on pointers to higher-level 
> locks (like struct mutex), as that is what is stored in the 
> task_struct->blocker field. It does not operate on the lower-level 
> arch_spinlock_t inside it.
> 

Perhaps you are aware that the minimum alignment of the struct is at least 
the minimum alignment of the first member. I believe that the reason why 
the lock is always the first member is that misaligned accesses would harm 
performance.

I really don't know why you want to argue about fixing this.

> While we could track the internal arch_spinlock_t, that would break 
> encapsulation.
>

Would it.

> The hung task detector should remain generic and not depend on 
> lock-specific implementation details ;)
> 

OK, like a new class derived from bitfield and pointer? Is that what you 
mean by "generic" and "encapsulated"?

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

* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
  2025-08-24  5:57                           ` Finn Thain
@ 2025-08-24  6:18                             ` Lance Yang
  0 siblings, 0 replies; 27+ messages in thread
From: Lance Yang @ 2025-08-24  6:18 UTC (permalink / raw)
  To: Finn Thain
  Cc: akpm, mhiramat, kernel test robot, geert, senozhatsky,
	oe-kbuild-all, amaindex, anna.schumaker, boqun.feng, ioworker0,
	joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
	linux-m68k, longman, mingo, mingzhe.yang, oak, rostedt, tfiga,
	will, stable



On 2025/8/24 13:57, Finn Thain wrote:
> 
> On Sun, 24 Aug 2025, Lance Yang wrote:
> 
>>
>> The blocker tracking mechanism operates on pointers to higher-level
>> locks (like struct mutex), as that is what is stored in the
>> task_struct->blocker field. It does not operate on the lower-level
>> arch_spinlock_t inside it.
>>
> 
> Perhaps you are aware that the minimum alignment of the struct is at least
> the minimum alignment of the first member. I believe that the reason why

Yes, that's how it should work in theory.

> the lock is always the first member is that misaligned accesses would harm
> performance.
> 
> I really don't know why you want to argue about fixing this.

Okay, arguing further isn't productive. Looking forward to seeing
your patch ;)

Thanks,
Lance


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

* Re: [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers
  2025-08-23  5:00               ` [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers Lance Yang
@ 2025-08-26  4:49                 ` Masami Hiramatsu
  2025-08-26  5:11                   ` Lance Yang
  0 siblings, 1 reply; 27+ messages in thread
From: Masami Hiramatsu @ 2025-08-26  4:49 UTC (permalink / raw)
  To: Lance Yang
  Cc: akpm, fthain, geert, senozhatsky, amaindex, anna.schumaker,
	boqun.feng, ioworker0, joel.granados, jstultz, kent.overstreet,
	leonylgao, linux-kernel, linux-m68k, longman, mingo, mingzhe.yang,
	oak, peterz, rostedt, tfiga, will, stable

On Sat, 23 Aug 2025 13:00:36 +0800
Lance Yang <lance.yang@linux.dev> wrote:

> From: Lance Yang <lance.yang@linux.dev>
> 
> The blocker tracking mechanism assumes that lock pointers are at least
> 4-byte aligned to use their lower bits for type encoding.
> 
> However, as reported by Geert Uytterhoeven, some architectures like m68k
> only guarantee 2-byte alignment of 32-bit values. This breaks the
> assumption and causes two related WARN_ON_ONCE checks to trigger.
> 
> To fix this, the runtime checks are adjusted. The first WARN_ON_ONCE in
> hung_task_set_blocker() is changed to a simple 'if' that returns silently
> for unaligned pointers. The second, now-invalid WARN_ON_ONCE in
> hung_task_clear_blocker() is then removed.
> 
> Thanks to Geert for bisecting!
> 
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Closes: https://lore.kernel.org/lkml/CAMuHMdW7Ab13DdGs2acMQcix5ObJK0O2dG_Fxzr8_g58Rc1_0g@mail.gmail.com
> Fixes: e711faaafbe5 ("hung_task: replace blocker_mutex with encoded blocker")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Lance Yang <lance.yang@linux.dev>

Looks good to me. I think we can just ignore it for
this debugging option.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you,

> ---
>  include/linux/hung_task.h | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/hung_task.h b/include/linux/hung_task.h
> index 34e615c76ca5..69640f266a69 100644
> --- a/include/linux/hung_task.h
> +++ b/include/linux/hung_task.h
> @@ -20,6 +20,10 @@
>   * always zero. So we can use these bits to encode the specific blocking
>   * type.
>   *
> + * Note that on architectures like m68k with only 2-byte alignment, the
> + * blocker tracking mechanism gracefully does nothing for any lock that is
> + * not 4-byte aligned.
> + *
>   * Type encoding:
>   * 00 - Blocked on mutex			(BLOCKER_TYPE_MUTEX)
>   * 01 - Blocked on semaphore			(BLOCKER_TYPE_SEM)
> @@ -45,7 +49,7 @@ static inline void hung_task_set_blocker(void *lock, unsigned long type)
>  	 * If the lock pointer matches the BLOCKER_TYPE_MASK, return
>  	 * without writing anything.
>  	 */
> -	if (WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK))
> +	if (lock_ptr & BLOCKER_TYPE_MASK)
>  		return;
>  
>  	WRITE_ONCE(current->blocker, lock_ptr | type);
> @@ -53,8 +57,6 @@ static inline void hung_task_set_blocker(void *lock, unsigned long type)
>  
>  static inline void hung_task_clear_blocker(void)
>  {
> -	WARN_ON_ONCE(!READ_ONCE(current->blocker));
> -
>  	WRITE_ONCE(current->blocker, 0UL);
>  }
>  
> -- 
> 2.49.0
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
  2025-08-23  7:40               ` [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures Lance Yang
  2025-08-23 21:53                 ` kernel test robot
@ 2025-08-26  5:02                 ` Masami Hiramatsu
  2025-08-26  5:16                   ` Lance Yang
  1 sibling, 1 reply; 27+ messages in thread
From: Masami Hiramatsu @ 2025-08-26  5:02 UTC (permalink / raw)
  To: Lance Yang
  Cc: akpm, fthain, geert, senozhatsky, amaindex, anna.schumaker,
	boqun.feng, ioworker0, joel.granados, jstultz, kent.overstreet,
	leonylgao, linux-kernel, linux-m68k, longman, mingo, mingzhe.yang,
	oak, peterz, rostedt, tfiga, will, stable

Hi Lence,

On Sat, 23 Aug 2025 15:40:48 +0800
Lance Yang <lance.yang@linux.dev> wrote:

> From: Lance Yang <lance.yang@linux.dev>
> 
> The blocker tracking mechanism assumes that lock pointers are at least
> 4-byte aligned to use their lower bits for type encoding.
> 
> However, as reported by Geert Uytterhoeven, some architectures like m68k
> only guarantee 2-byte alignment of 32-bit values. This breaks the
> assumption and causes two related WARN_ON_ONCE checks to trigger.
> 
> To fix this, enforce a minimum of 4-byte alignment on the core lock
> structures supported by the blocker tracking mechanism. This ensures the
> algorithm's alignment assumption now holds true on all architectures.
> 
> This patch adds __aligned(4) to the definitions of "struct mutex",
> "struct semaphore", and "struct rw_semaphore", resolving the warnings.

Instead of putting the type flags in the blocker address (pointer),
can't we record the type information outside? It is hard to enforce
the alignment to the locks, because it is embedded in the data
structure. Instead, it is better to record the type as blocker_type
in current task_struct.

Thank you,

> 
> Thanks to Geert for bisecting!
> 
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Closes: https://lore.kernel.org/lkml/CAMuHMdW7Ab13DdGs2acMQcix5ObJK0O2dG_Fxzr8_g58Rc1_0g@mail.gmail.com
> Fixes: e711faaafbe5 ("hung_task: replace blocker_mutex with encoded blocker")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Lance Yang <lance.yang@linux.dev>
> ---
>  include/linux/mutex_types.h | 2 +-
>  include/linux/rwsem.h       | 2 +-
>  include/linux/semaphore.h   | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/mutex_types.h b/include/linux/mutex_types.h
> index fdf7f515fde8..de798bfbc4c7 100644
> --- a/include/linux/mutex_types.h
> +++ b/include/linux/mutex_types.h
> @@ -51,7 +51,7 @@ struct mutex {
>  #ifdef CONFIG_DEBUG_LOCK_ALLOC
>  	struct lockdep_map	dep_map;
>  #endif
> -};
> +} __aligned(4); /* For hung_task blocker tracking, which encodes type in LSBs */
>  
>  #else /* !CONFIG_PREEMPT_RT */
>  /*
> diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h
> index f1aaf676a874..f6ecf4a4710d 100644
> --- a/include/linux/rwsem.h
> +++ b/include/linux/rwsem.h
> @@ -64,7 +64,7 @@ struct rw_semaphore {
>  #ifdef CONFIG_DEBUG_LOCK_ALLOC
>  	struct lockdep_map	dep_map;
>  #endif
> -};
> +} __aligned(4); /* For hung_task blocker tracking, which encodes type in LSBs */
>  
>  #define RWSEM_UNLOCKED_VALUE		0UL
>  #define RWSEM_WRITER_LOCKED		(1UL << 0)
> diff --git a/include/linux/semaphore.h b/include/linux/semaphore.h
> index 89706157e622..ac9b9c87bfb7 100644
> --- a/include/linux/semaphore.h
> +++ b/include/linux/semaphore.h
> @@ -20,7 +20,7 @@ struct semaphore {
>  #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
>  	unsigned long		last_holder;
>  #endif
> -};
> +} __aligned(4); /* For hung_task blocker tracking, which encodes type in LSBs */
>  
>  #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
>  #define __LAST_HOLDER_SEMAPHORE_INITIALIZER				\
> -- 
> 2.49.0
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

* Re: [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers
  2025-08-26  4:49                 ` Masami Hiramatsu
@ 2025-08-26  5:11                   ` Lance Yang
  0 siblings, 0 replies; 27+ messages in thread
From: Lance Yang @ 2025-08-26  5:11 UTC (permalink / raw)
  To: Masami Hiramatsu (Google), akpm
  Cc: fthain, geert, senozhatsky, amaindex, anna.schumaker, boqun.feng,
	ioworker0, joel.granados, jstultz, kent.overstreet, leonylgao,
	linux-kernel, linux-m68k, longman, mingo, mingzhe.yang, oak,
	peterz, rostedt, tfiga, will, stable

Thanks for the review!

On 2025/8/26 12:49, Masami Hiramatsu (Google) wrote:
> On Sat, 23 Aug 2025 13:00:36 +0800
> Lance Yang <lance.yang@linux.dev> wrote:
> 
>> From: Lance Yang <lance.yang@linux.dev>
>>
>> The blocker tracking mechanism assumes that lock pointers are at least
>> 4-byte aligned to use their lower bits for type encoding.
>>
>> However, as reported by Geert Uytterhoeven, some architectures like m68k
>> only guarantee 2-byte alignment of 32-bit values. This breaks the
>> assumption and causes two related WARN_ON_ONCE checks to trigger.
>>
>> To fix this, the runtime checks are adjusted. The first WARN_ON_ONCE in
>> hung_task_set_blocker() is changed to a simple 'if' that returns silently
>> for unaligned pointers. The second, now-invalid WARN_ON_ONCE in
>> hung_task_clear_blocker() is then removed.
>>
>> Thanks to Geert for bisecting!
>>
>> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> Closes: https://lore.kernel.org/lkml/CAMuHMdW7Ab13DdGs2acMQcix5ObJK0O2dG_Fxzr8_g58Rc1_0g@mail.gmail.com
>> Fixes: e711faaafbe5 ("hung_task: replace blocker_mutex with encoded blocker")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Lance Yang <lance.yang@linux.dev>
> 
> Looks good to me. I think we can just ignore it for
> this debugging option.

Exactly. As Peter pointed out, most architectures would trap on the
unaligned atomic access long before this check is ever reached.

So this patch only affects the few architectures that don't trap,
gracefully silencing the warning there. This makes it a clean and safe
fix for backporting.

Cheers,
Lance

> 
> Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Thank you,
> 
>> ---
>>   include/linux/hung_task.h | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/hung_task.h b/include/linux/hung_task.h
>> index 34e615c76ca5..69640f266a69 100644
>> --- a/include/linux/hung_task.h
>> +++ b/include/linux/hung_task.h
>> @@ -20,6 +20,10 @@
>>    * always zero. So we can use these bits to encode the specific blocking
>>    * type.
>>    *
>> + * Note that on architectures like m68k with only 2-byte alignment, the
>> + * blocker tracking mechanism gracefully does nothing for any lock that is
>> + * not 4-byte aligned.
>> + *
>>    * Type encoding:
>>    * 00 - Blocked on mutex			(BLOCKER_TYPE_MUTEX)
>>    * 01 - Blocked on semaphore			(BLOCKER_TYPE_SEM)
>> @@ -45,7 +49,7 @@ static inline void hung_task_set_blocker(void *lock, unsigned long type)
>>   	 * If the lock pointer matches the BLOCKER_TYPE_MASK, return
>>   	 * without writing anything.
>>   	 */
>> -	if (WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK))
>> +	if (lock_ptr & BLOCKER_TYPE_MASK)
>>   		return;
>>   
>>   	WRITE_ONCE(current->blocker, lock_ptr | type);
>> @@ -53,8 +57,6 @@ static inline void hung_task_set_blocker(void *lock, unsigned long type)
>>   
>>   static inline void hung_task_clear_blocker(void)
>>   {
>> -	WARN_ON_ONCE(!READ_ONCE(current->blocker));
>> -
>>   	WRITE_ONCE(current->blocker, 0UL);
>>   }
>>   
>> -- 
>> 2.49.0
>>
> 
> 


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

* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
  2025-08-26  5:02                 ` Masami Hiramatsu
@ 2025-08-26  5:16                   ` Lance Yang
  0 siblings, 0 replies; 27+ messages in thread
From: Lance Yang @ 2025-08-26  5:16 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: akpm, fthain, geert, senozhatsky, amaindex, anna.schumaker,
	boqun.feng, ioworker0, joel.granados, jstultz, kent.overstreet,
	leonylgao, linux-kernel, linux-m68k, longman, mingo, mingzhe.yang,
	oak, peterz, rostedt, tfiga, will, stable



On 2025/8/26 13:02, Masami Hiramatsu (Google) wrote:
> Hi Lence,
> 
> On Sat, 23 Aug 2025 15:40:48 +0800
> Lance Yang <lance.yang@linux.dev> wrote:
> 
>> From: Lance Yang <lance.yang@linux.dev>
>>
>> The blocker tracking mechanism assumes that lock pointers are at least
>> 4-byte aligned to use their lower bits for type encoding.
>>
>> However, as reported by Geert Uytterhoeven, some architectures like m68k
>> only guarantee 2-byte alignment of 32-bit values. This breaks the
>> assumption and causes two related WARN_ON_ONCE checks to trigger.
>>
>> To fix this, enforce a minimum of 4-byte alignment on the core lock
>> structures supported by the blocker tracking mechanism. This ensures the
>> algorithm's alignment assumption now holds true on all architectures.
>>
>> This patch adds __aligned(4) to the definitions of "struct mutex",
>> "struct semaphore", and "struct rw_semaphore", resolving the warnings.
> 
> Instead of putting the type flags in the blocker address (pointer),
> can't we record the type information outside? It is hard to enforce

Yes. Of course. The current pointer-encoding is a tricky trade-off ...

> the alignment to the locks, because it is embedded in the data
> structure. Instead, it is better to record the type as blocker_type
> in current task_struct.

TODO +1. Separating the type into its own field in task_struct is the
right long-term solution ;)

Cheers,
Lance

> 
> Thank you,
> 
>>
>> Thanks to Geert for bisecting!
>>
>> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> Closes: https://lore.kernel.org/lkml/CAMuHMdW7Ab13DdGs2acMQcix5ObJK0O2dG_Fxzr8_g58Rc1_0g@mail.gmail.com
>> Fixes: e711faaafbe5 ("hung_task: replace blocker_mutex with encoded blocker")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Lance Yang <lance.yang@linux.dev>
>> ---
>>   include/linux/mutex_types.h | 2 +-
>>   include/linux/rwsem.h       | 2 +-
>>   include/linux/semaphore.h   | 2 +-
>>   3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/mutex_types.h b/include/linux/mutex_types.h
>> index fdf7f515fde8..de798bfbc4c7 100644
>> --- a/include/linux/mutex_types.h
>> +++ b/include/linux/mutex_types.h
>> @@ -51,7 +51,7 @@ struct mutex {
>>   #ifdef CONFIG_DEBUG_LOCK_ALLOC
>>   	struct lockdep_map	dep_map;
>>   #endif
>> -};
>> +} __aligned(4); /* For hung_task blocker tracking, which encodes type in LSBs */
>>   
>>   #else /* !CONFIG_PREEMPT_RT */
>>   /*
>> diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h
>> index f1aaf676a874..f6ecf4a4710d 100644
>> --- a/include/linux/rwsem.h
>> +++ b/include/linux/rwsem.h
>> @@ -64,7 +64,7 @@ struct rw_semaphore {
>>   #ifdef CONFIG_DEBUG_LOCK_ALLOC
>>   	struct lockdep_map	dep_map;
>>   #endif
>> -};
>> +} __aligned(4); /* For hung_task blocker tracking, which encodes type in LSBs */
>>   
>>   #define RWSEM_UNLOCKED_VALUE		0UL
>>   #define RWSEM_WRITER_LOCKED		(1UL << 0)
>> diff --git a/include/linux/semaphore.h b/include/linux/semaphore.h
>> index 89706157e622..ac9b9c87bfb7 100644
>> --- a/include/linux/semaphore.h
>> +++ b/include/linux/semaphore.h
>> @@ -20,7 +20,7 @@ struct semaphore {
>>   #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
>>   	unsigned long		last_holder;
>>   #endif
>> -};
>> +} __aligned(4); /* For hung_task blocker tracking, which encodes type in LSBs */
>>   
>>   #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
>>   #define __LAST_HOLDER_SEMAPHORE_INITIALIZER				\
>> -- 
>> 2.49.0
>>
> 
> 


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

end of thread, other threads:[~2025-08-26  5:16 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-14 14:59 [PATCH v5 0/3] hung_task: extend blocking task stacktrace dump to semaphore Lance Yang
2025-04-14 14:59 ` [PATCH v5 1/3] hung_task: replace blocker_mutex with encoded blocker Lance Yang
2025-04-14 21:36   ` Andrew Morton
2025-04-15  3:44     ` Lance Yang
2025-04-14 14:59 ` [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore Lance Yang
2025-08-22  7:38   ` Geert Uytterhoeven
2025-08-22 15:18     ` Lance Yang
2025-08-22 15:37       ` Geert Uytterhoeven
2025-08-22 16:42         ` Lance Yang
2025-08-23  0:27           ` Finn Thain
2025-08-23  4:47             ` Lance Yang
2025-08-23  5:00               ` [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers Lance Yang
2025-08-26  4:49                 ` Masami Hiramatsu
2025-08-26  5:11                   ` Lance Yang
2025-08-23  7:40               ` [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures Lance Yang
2025-08-23 21:53                 ` kernel test robot
2025-08-24  0:47                   ` Finn Thain
2025-08-24  3:03                     ` Lance Yang
2025-08-24  4:18                       ` Finn Thain
2025-08-24  5:02                         ` Lance Yang
2025-08-24  5:57                           ` Finn Thain
2025-08-24  6:18                             ` Lance Yang
2025-08-26  5:02                 ` Masami Hiramatsu
2025-08-26  5:16                   ` Lance Yang
2025-08-23  7:49               ` [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore Lance Yang
2025-04-14 14:59 ` [PATCH v5 3/3] samples: extend hung_task detector test with semaphore support Lance Yang
2025-04-14 21:38 ` [PATCH v5 0/3] hung_task: extend blocking task stacktrace dump to semaphore Andrew Morton

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).