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

[Lance Yang <lance.yang@linux.dev>]

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