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

[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

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

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