[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9efaadc9-7f96-435e-9711-7f2ce96a820a@linux.dev>
Date: Sun, 24 Aug 2025 11:03:35 +0800
From: Lance Yang <lance.yang@...ux.dev>
To: Finn Thain <fthain@...ux-m68k.org>, akpm@...ux-foundation.org,
mhiramat@...nel.org
Cc: kernel test robot <lkp@...el.com>, geert@...ux-m68k.org,
senozhatsky@...omium.org, oe-kbuild-all@...ts.linux.dev,
amaindex@...look.com, anna.schumaker@...cle.com, boqun.feng@...il.com,
ioworker0@...il.com, joel.granados@...nel.org, jstultz@...gle.com,
kent.overstreet@...ux.dev, leonylgao@...cent.com,
linux-kernel@...r.kernel.org, linux-m68k@...ts.linux-m68k.org,
longman@...hat.com, mingo@...hat.com, mingzhe.yang@...com,
oak@...sinkinet.fi, rostedt@...dmis.org, tfiga@...omium.org,
will@...nel.org, stable@...r.kernel.org
Subject: Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on
lock structures
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
Powered by blists - more mailing lists