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

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

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