Message-ID: <d0fe3163-32d9-4d81-81bb-d964f2f43f17@linux.dev>
Date: Sat, 23 Aug 2025 00:42:48 +0800
From: Lance Yang <lance.yang@...ux.dev>
To: Geert Uytterhoeven <geert@...ux-m68k.org>, mhiramat@...nel.org
Cc: akpm@...ux-foundation.org, will@...nel.org, peterz@...radead.org,
 mingo@...hat.com, longman@...hat.com, anna.schumaker@...cle.com,
 boqun.feng@...il.com, joel.granados@...nel.org, kent.overstreet@...ux.dev,
 leonylgao@...cent.com, linux-kernel@...r.kernel.org, rostedt@...dmis.org,
 tfiga@...omium.org, amaindex@...look.com, jstultz@...gle.com,
 Mingzhe Yang <mingzhe.yang@...com>, Eero Tamminen <oak@...sinkinet.fi>,
 linux-m68k <linux-m68k@...ts.linux-m68k.org>,
 Lance Yang <ioworker0@...il.com>, senozhatsky@...omium.org
Subject: Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is
 hung on semaphore

@Masami

On 2025/8/22 23:37, Geert Uytterhoeven wrote:
> Hi Lance,
> 
> On Fri, 22 Aug 2025 at 17:18, Lance Yang <lance.yang@...ux.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@...il.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

Powered by blists - more mailing lists