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.