Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore

Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore

[Geert Uytterhoeven]

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

Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore

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

Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore

[Geert Uytterhoeven]

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

Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore

[Lance Yang]

@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

Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore

[Finn Thain]

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.

Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore

[Lance Yang]

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

[PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers

[Lance Yang]

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

[PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures

[Lance Yang]

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

Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore

[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

Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures

[John Paul Adrian Glaubitz]

On Sat, 2025-08-23 at 15:40 +0800, Lance Yang 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.

Another case for switching Debian m68k to 32-bit alignment.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures

[kernel test robot]

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

Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures

[Finn Thain]

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.

Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures

[Lance Yang]

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

Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures

[Finn Thain]

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.

Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures

[Lance Yang]

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.

Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures

[Finn Thain]

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"?

Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures

[Lance Yang]

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

Re: [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers

[Masami Hiramatsu]

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>

Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures

[Masami Hiramatsu]

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>

Re: [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers

[Lance Yang]

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

Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures

[Lance Yang]

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