Linux filesystem development
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@redhat.com>
To: Mateusz Guzik <mjguzik@gmail.com>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-kernel@vger.kernel.org,
	Christian Brauner <brauner@kernel.org>,
	David Howells <dhowells@redhat.com>,
	lkp@intel.com, oe-lkp@lists.linux.dev,
	Alexander Viro <aviro@redhat.com>
Subject: Re: [PATCH RFC] fs: cache-align lock_class_keys in struct file_system_type
Date: Tue, 30 Dec 2025 16:45:26 -0600	[thread overview]
Message-ID: <28f144f8-431d-4c3a-a362-56083bb77541@redhat.com> (raw)
In-Reply-To: <o6cnjqy4ivjqaj4n5xphstfnk5jznufaygwmfkm2gyixqgfump@7fc6c6h6d5if>

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@intel.com>
>> Closes: https://lore.kernel.org/oe-lkp/202512230315.1717476b-lkp@intel.com
>> Fixes: 51a146e05 ("fs: Remove internal old mount API code")
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> 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


  reply	other threads:[~2025-12-30 22:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-30 21:07 [PATCH RFC] fs: cache-align lock_class_keys in struct file_system_type Eric Sandeen
2025-12-30 22:04 ` Mateusz Guzik
2025-12-30 22:45   ` Eric Sandeen [this message]
2026-01-03 12:23     ` Mateusz Guzik
2025-12-31  6:07 ` Oliver Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=28f144f8-431d-4c3a-a362-56083bb77541@redhat.com \
    --to=sandeen@redhat.com \
    --cc=aviro@redhat.com \
    --cc=brauner@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=mjguzik@gmail.com \
    --cc=oe-lkp@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox