Re: [PATCH RFC] fs: cache-align lock_class_keys in struct file_system_type

[Eric Sandeen]

hohum, well I spent a bunch of tokens in claude/opus and basically got

¯\_(ツ)_/¯ 

I could be wrong but I'm not sure sashiko would do a deep dive across
all the involved code.

I was going to paste the summary of the AI analysis which I generally
dislike but tbh there's just nothing to go on :(

It did point out that s_lock_key seems dead/unused and thought removing
it might magically fix the problem as well.

-Eric

On 9/8/26 3:07 AM, Mateusz Guzik wrote:
> While I'm not convinced spending time on the resulting bug is
> worthwhile, with the arrival of sashiko I think you should rebase &
> resend -- it very well might just point out the problem with 0 human
> effort.
> 
> On Tue, Dec 30, 2025 at 11:45 PM Eric Sandeen <sandeen@xxxxxxxxxx> wrote:
>>
>> On 12/30/25 4:04 PM, Mateusz Guzik wrote:
>>> On Tue, Dec 30, 2025 at 03:07:10PM -0600, Eric Sandeen wrote:
>>>> LKP reported that one of their tests was failing to even boot with my
>>>> "old mount API code" removal patch. The test was booting an i386 kernel
>>>> under QEMU, with lockdep enabled. Rather than a functional failure, it
>>>> seemed to have been slowed to a crawl and eventually timed out.
>>>>
>>>> I narrowed the problem down to the removal of the ->mount op from
>>>> file_system_type, which changed structure alignment and seems to have
>>>> caused cacheline issues with this structure. Annotating the alignment
>>>> fixes the problem for me.
>>>>
>>>> Reported-by: kernel test robot <oliver.sang@xxxxxxxxx>
>>>> Closes: https://lore.kernel.org/oe-lkp/202512230315.1717476b-lkp@xxxxxxxxx
>>>> Fixes: 51a146e05 ("fs: Remove internal old mount API code")
>>>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>>>> ---
>>>>
>>>> RFC because I honestly don't understand why this should be so critical,
>>>> especially the structure was not explicitly (or even very well) aligned
>>>> before. I would welcome insights from folks who are smarter than me!
>>>>
>>>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>>>> index 9949d253e5aa..b3d8cad15de1 100644
>>>> --- a/include/linux/fs.h
>>>> +++ b/include/linux/fs.h
>>>> @@ -2279,7 +2279,7 @@ struct file_system_type {
>>>>      struct file_system_type * next;
>>>>      struct hlist_head fs_supers;
>>>>
>>>> -    struct lock_class_key s_lock_key;
>>>> +    struct lock_class_key s_lock_key ____cacheline_aligned;
>>>>      struct lock_class_key s_umount_key;
>>>>      struct lock_class_key s_vfs_rename_key;
>>>>      struct lock_class_key s_writers_key[SB_FREEZE_LEVELS];
>>>>
>>>
>>> There is no way is about cacheline bouncing. According to the linked
>>> thread the test vm has only 2 vcpus:
>>>> test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 4G
>>>
>>> Even if the vcpu count was in hundreds and the ping pong was a problem it
>>> still would not have prevented bootup.
>>
>> Fair enough, it really didn't make sense to me.
>>
>>> Instead something depends on the old layout for correctness.
>>
>> If I add ->mount back to the /end/ of the structure, it boots fine again,
>> so I guess nothing is expecting specific offsets within the structure
>> at least.
>>
>> (If I turn off lockdep in the kernel config, it boots fine again too,
>> FWIW.)
>>
>>> By any chance is this type-punned somewhere?
>>
>> I don't think so...
>>
>>> While I can't be bothered to investigate, I *suspect* the way to catch
>>> this would patch out all of the lock_class_key vars & uses and boot with
>>> KMSAN (or was it KASAN?). Or whatever mechanism which can tell the
>>> access is oob.
>>
>> Can't do KASAN or KMSAN on i386, AFAIK. :(
>>
>> I suppose I could try such things on x86_64 just in case something shows
>> up...
>>
>> Thanks!
>>
>> -Eric
>>
>