Re: [PATCH 1/1] x86/mm/pae: Align up pteval_t, pmdval_t and pudval_t to avoid split locks

[Ingo Molnar <mingo@kernel.org>]

* Dave Hansen <dave.hansen@intel.com> wrote:

> On 4/2/24 10:23, Javier Pello wrote:
> > On Mon, 1 Apr 2024 10:56:14 -0700 Dave Hansen wrote:
> >> First of all, how is it that you're running a PAE kernel on new,
> >> 64-bit hardware?  That's rather odd.
> > 
> > I got this motherboard and cpu fairly recently to replace old
> > hardware, and I just plugged my old hard disk and went along with
> > it, because I did not feel like bootstrapping a 64-bit system.
> 
> Fair enough.  I can totally understand wanting the convenience.  But
> you're leaving _so_ much performance on the floor that split locks are
> the least of your problems.
> 
> >> The case that you're hitting is actually an on-stack pmd_t.  The
> >> fun part is that it's not shared and doesn't even _need_ atomics.
> >> I think it's just using pmd_populate() because it's convenient.
> > 
> > I see. So just annotating the variable on the stack with
> > __aligned(8) should do it? But the code is under mm/, so it should
> > be arch-agnostic, right? What would the correct fix be, then? I take
> > from your message that using atomics through pmd_populate() here is
> > not needed, but what accessors should be used instead? I am not
> > familiar at all with this part of the kernel.
> 
> I don't think there's a better accessor.
> 
> >> I'd honestly much rather just disable split lock support in 32-bit
> >> builds than mess with this stuff.  You really shouldn't be running
> >> 32-but kernels on this hardware.
> > 
> > Why? Is it unsupported?
> 
> Yes, it's effectively unsupported.  We're not adding new hardware 
> features to 32-bit.  The fact that split lock detection got enabled 
> was an accident.

We do accept well-tested fixes and minor enablement patches though, 
within reason - but indeed this page table entry alignment quirk added 
for the sake of a split-lock debugging false positive doesn't seem to 
be worth it.

> It's not a technical reason.  It's a practical one: I don't want to 
> spend time reviewing the fixes and dealing with the fallout and 
> regressions that the fixes might cause.

Yeah, so it's an indirect technical argument: fixes *with tradeoffs* 
like this one have a future maintenance & robustness cost. Fixes 
without tradeoffs are fine of course.

Thanks,

	Ingo