Messages in this thread | | | Date | Wed, 3 Apr 2024 09:57:48 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH 1/1] x86/mm/pae: Align up pteval_t, pmdval_t and pudval_t to avoid split locks |
| |
* 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
| |