Hi Finn,
Nice work, thanks for your patch!
On 2025/8/25 10:03, Finn Thain wrote:
Some recent commits incorrectly assumed the natural alignment of locks. That assumption fails on Linux/m68k (and, interestingly, would have failed on Linux/cris also). This leads to spurious warnings from the hang check code. Fix this bug by adding the necessary 'aligned' attribute.
Cc: Andrew Morton akpm@linux-foundation.org Cc: Geert Uytterhoeven geert@linux-m68k.org Cc: Lance Yang lance.yang@linux.dev Cc: Masami Hiramatsu mhiramat@kernel.org Cc: Eero Tamminen oak@helsinkinet.fi Cc: Peter Zijlstra peterz@infradead.org Cc: Will Deacon will@kernel.org Cc: stable@vger.kernel.org Reported-by: Eero Tamminen oak@helsinkinet.fi Closes: https://lore.kernel.org/lkml/CAMuHMdW7Ab13DdGs2acMQcix5ObJK0O2dG_Fxzr8_g58Rc... Fixes: e711faaafbe5 ("hung_task: replace blocker_mutex with encoded blocker") Signed-off-by: Finn Thain fthain@linux-m68k.org
I tested this on m68k using GCC and it fixed the problem for me. AFAIK, the other architectures naturally align ints already so I'm expecting to see no effect there.
Yeah, it is the correct approach for the spurious warnings on architectures like m68k, where the natural alignment of types can be less than 4 bytes.
include/linux/types.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/types.h b/include/linux/types.h index 6dfdb8e8e4c3..cd5b2b0f4b02 100644 --- a/include/linux/types.h +++ b/include/linux/types.h @@ -179,7 +179,7 @@ typedef phys_addr_t resource_size_t; typedef unsigned long irq_hw_number_t; typedef struct {
- int counter;
- int counter __aligned(sizeof(int)); } atomic_t;
#define ATOMIC_INIT(i) { (i) }
However, as we've seen from the kernel test robot's report on mt6660_chip, this won't solve the cases where a lock is forced to be unaligned by #pragma pack(1). That will still trigger warnings, IIUC.
Perhaps we should also apply the follwoing?
diff --git a/include/linux/hung_task.h b/include/linux/hung_task.h index 34e615c76ca5..940f8f3558f6 100644 --- a/include/linux/hung_task.h +++ b/include/linux/hung_task.h @@ -45,7 +45,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 +53,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); }
Let the feature gracefully do nothing on that ;)