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@xxxxxxxxx> wrote:
>
> > From: Lance Yang <lance.yang@xxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxx>
> > Closes: https://lore.kernel.org/lkml/CAMuHMdW7Ab13DdGs2acMQcix5ObJK0O2dG_Fxzr8_g58Rc1_0g@xxxxxxxxxxxxxx
> > Fixes: e711faaafbe5 ("hung_task: replace blocker_mutex with encoded blocker")
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > Signed-off-by: Lance Yang <lance.yang@xxxxxxxxx>
>
> 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@xxxxxxxxxx>
>
> 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