* [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers
2025-08-23 4:47 ` Lance Yang
@ 2025-08-23 5:00 ` Lance Yang
2025-08-26 4:49 ` Masami Hiramatsu
2025-08-23 7:40 ` [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures Lance Yang
2025-08-23 7:49 ` [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore Lance Yang
2 siblings, 1 reply; 21+ messages in thread
From: Lance Yang @ 2025-08-23 5:00 UTC (permalink / raw)
To: akpm, fthain, geert, mhiramat, senozhatsky
Cc: lance.yang, amaindex, anna.schumaker, boqun.feng, ioworker0,
joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
linux-m68k, longman, mingo, mingzhe.yang, oak, peterz, rostedt,
tfiga, will, stable
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers
2025-08-23 5:00 ` [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers Lance Yang
@ 2025-08-26 4:49 ` Masami Hiramatsu
2025-08-26 5:11 ` Lance Yang
0 siblings, 1 reply; 21+ messages in thread
From: Masami Hiramatsu @ 2025-08-26 4:49 UTC (permalink / raw)
To: Lance Yang
Cc: akpm, fthain, geert, senozhatsky, amaindex, anna.schumaker,
boqun.feng, ioworker0, joel.granados, jstultz, kent.overstreet,
leonylgao, linux-kernel, linux-m68k, longman, mingo, mingzhe.yang,
oak, peterz, rostedt, tfiga, will, stable
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>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers
2025-08-26 4:49 ` Masami Hiramatsu
@ 2025-08-26 5:11 ` Lance Yang
0 siblings, 0 replies; 21+ messages in thread
From: Lance Yang @ 2025-08-26 5:11 UTC (permalink / raw)
To: Masami Hiramatsu (Google), akpm
Cc: fthain, geert, senozhatsky, amaindex, anna.schumaker, boqun.feng,
ioworker0, joel.granados, jstultz, kent.overstreet, leonylgao,
linux-kernel, linux-m68k, longman, mingo, mingzhe.yang, oak,
peterz, rostedt, tfiga, will, stable
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
>>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
2025-08-23 4:47 ` Lance Yang
2025-08-23 5:00 ` [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers Lance Yang
@ 2025-08-23 7:40 ` Lance Yang
2025-08-23 11:06 ` John Paul Adrian Glaubitz
` (2 more replies)
2025-08-23 7:49 ` [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore Lance Yang
2 siblings, 3 replies; 21+ messages in thread
From: Lance Yang @ 2025-08-23 7:40 UTC (permalink / raw)
To: akpm, fthain, geert, mhiramat, senozhatsky
Cc: lance.yang, amaindex, anna.schumaker, boqun.feng, ioworker0,
joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
linux-m68k, longman, mingo, mingzhe.yang, oak, peterz, rostedt,
tfiga, will, stable
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
2025-08-23 7:40 ` [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures Lance Yang
@ 2025-08-23 11:06 ` John Paul Adrian Glaubitz
2025-08-23 21:53 ` kernel test robot
2025-08-26 5:02 ` Masami Hiramatsu
2 siblings, 0 replies; 21+ messages in thread
From: John Paul Adrian Glaubitz @ 2025-08-23 11:06 UTC (permalink / raw)
To: debian-68k; +Cc: linux-m68k
On Sat, 2025-08-23 at 15:40 +0800, Lance Yang 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.
Another case for switching Debian m68k to 32-bit alignment.
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
2025-08-23 7:40 ` [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures Lance Yang
2025-08-23 11:06 ` John Paul Adrian Glaubitz
@ 2025-08-23 21:53 ` kernel test robot
2025-08-24 0:47 ` Finn Thain
2025-08-26 5:02 ` Masami Hiramatsu
2 siblings, 1 reply; 21+ messages in thread
From: kernel test robot @ 2025-08-23 21:53 UTC (permalink / raw)
To: Lance Yang, akpm, fthain, geert, mhiramat, senozhatsky
Cc: oe-kbuild-all, lance.yang, amaindex, anna.schumaker, boqun.feng,
ioworker0, joel.granados, jstultz, kent.overstreet, leonylgao,
linux-kernel, linux-m68k, longman, mingo, mingzhe.yang, oak,
rostedt, tfiga, will, stable
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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
2025-08-23 21:53 ` kernel test robot
@ 2025-08-24 0:47 ` Finn Thain
2025-08-24 3:03 ` Lance Yang
0 siblings, 1 reply; 21+ messages in thread
From: Finn Thain @ 2025-08-24 0:47 UTC (permalink / raw)
To: Lance Yang
Cc: kernel test robot, akpm, geert, mhiramat, senozhatsky,
oe-kbuild-all, amaindex, anna.schumaker, boqun.feng, ioworker0,
joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
linux-m68k, longman, mingo, mingzhe.yang, oak, rostedt, tfiga,
will, stable
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
2025-08-24 0:47 ` Finn Thain
@ 2025-08-24 3:03 ` Lance Yang
2025-08-24 4:18 ` Finn Thain
0 siblings, 1 reply; 21+ messages in thread
From: Lance Yang @ 2025-08-24 3:03 UTC (permalink / raw)
To: Finn Thain, akpm, mhiramat
Cc: kernel test robot, geert, senozhatsky, oe-kbuild-all, amaindex,
anna.schumaker, boqun.feng, ioworker0, joel.granados, jstultz,
kent.overstreet, leonylgao, linux-kernel, linux-m68k, longman,
mingo, mingzhe.yang, oak, rostedt, tfiga, will, stable
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
2025-08-24 3:03 ` Lance Yang
@ 2025-08-24 4:18 ` Finn Thain
2025-08-24 5:02 ` Lance Yang
0 siblings, 1 reply; 21+ messages in thread
From: Finn Thain @ 2025-08-24 4:18 UTC (permalink / raw)
To: Lance Yang
Cc: akpm, mhiramat, kernel test robot, geert, senozhatsky,
oe-kbuild-all, amaindex, anna.schumaker, boqun.feng, ioworker0,
joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
linux-m68k, longman, mingo, mingzhe.yang, oak, rostedt, tfiga,
will, stable
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
2025-08-24 4:18 ` Finn Thain
@ 2025-08-24 5:02 ` Lance Yang
2025-08-24 5:57 ` Finn Thain
0 siblings, 1 reply; 21+ messages in thread
From: Lance Yang @ 2025-08-24 5:02 UTC (permalink / raw)
To: Finn Thain
Cc: akpm, mhiramat, kernel test robot, geert, senozhatsky,
oe-kbuild-all, amaindex, anna.schumaker, boqun.feng, ioworker0,
joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
linux-m68k, longman, mingo, mingzhe.yang, oak, rostedt, tfiga,
will, stable
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
2025-08-24 5:02 ` Lance Yang
@ 2025-08-24 5:57 ` Finn Thain
2025-08-24 6:18 ` Lance Yang
0 siblings, 1 reply; 21+ messages in thread
From: Finn Thain @ 2025-08-24 5:57 UTC (permalink / raw)
To: Lance Yang
Cc: akpm, mhiramat, kernel test robot, geert, senozhatsky,
oe-kbuild-all, amaindex, anna.schumaker, boqun.feng, ioworker0,
joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
linux-m68k, longman, mingo, mingzhe.yang, oak, rostedt, tfiga,
will, stable
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"?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
2025-08-24 5:57 ` Finn Thain
@ 2025-08-24 6:18 ` Lance Yang
0 siblings, 0 replies; 21+ messages in thread
From: Lance Yang @ 2025-08-24 6:18 UTC (permalink / raw)
To: Finn Thain
Cc: akpm, mhiramat, kernel test robot, geert, senozhatsky,
oe-kbuild-all, amaindex, anna.schumaker, boqun.feng, ioworker0,
joel.granados, jstultz, kent.overstreet, leonylgao, linux-kernel,
linux-m68k, longman, mingo, mingzhe.yang, oak, rostedt, tfiga,
will, stable
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
2025-08-23 7:40 ` [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures Lance Yang
2025-08-23 11:06 ` John Paul Adrian Glaubitz
2025-08-23 21:53 ` kernel test robot
@ 2025-08-26 5:02 ` Masami Hiramatsu
2025-08-26 5:16 ` Lance Yang
2 siblings, 1 reply; 21+ messages in thread
From: Masami Hiramatsu @ 2025-08-26 5:02 UTC (permalink / raw)
To: Lance Yang
Cc: akpm, fthain, geert, senozhatsky, amaindex, anna.schumaker,
boqun.feng, ioworker0, joel.granados, jstultz, kent.overstreet,
leonylgao, linux-kernel, linux-m68k, longman, mingo, mingzhe.yang,
oak, peterz, rostedt, tfiga, will, stable
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>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures
2025-08-26 5:02 ` Masami Hiramatsu
@ 2025-08-26 5:16 ` Lance Yang
0 siblings, 0 replies; 21+ messages in thread
From: Lance Yang @ 2025-08-26 5:16 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: akpm, fthain, geert, senozhatsky, amaindex, anna.schumaker,
boqun.feng, ioworker0, joel.granados, jstultz, kent.overstreet,
leonylgao, linux-kernel, linux-m68k, longman, mingo, mingzhe.yang,
oak, peterz, rostedt, tfiga, will, stable
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
>>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 2/3] hung_task: show the blocker task if the task is hung on semaphore
2025-08-23 4:47 ` Lance Yang
2025-08-23 5:00 ` [PATCH 1/1] hung_task: fix warnings caused by unaligned lock pointers Lance Yang
2025-08-23 7:40 ` [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures Lance Yang
@ 2025-08-23 7:49 ` Lance Yang
2 siblings, 0 replies; 21+ messages in thread
From: Lance Yang @ 2025-08-23 7:49 UTC (permalink / raw)
To: akpm, mhiramat, Finn Thain, Geert Uytterhoeven, senozhatsky
Cc: will, peterz, mingo, longman, anna.schumaker, boqun.feng,
joel.granados, kent.overstreet, leonylgao, linux-kernel, rostedt,
tfiga, amaindex, jstultz, Mingzhe Yang, Eero Tamminen, linux-m68k,
Lance Yang
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
^ permalink raw reply [flat|nested] 21+ messages in thread