Messages in this thread | | | Date | Fri, 6 Jan 2023 11:52:22 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH] x86/mm/cpa: get rid of the cpa lock |
| |
* Jacky Li <jackyli@google.com> wrote:
> It’s true that with such old code, the cpa_lock might protect more > race conditions than those that it was introduced to protect in 2008, > or some old hardware may depend on the cpa_lock for undocumented > behavior. So removing the lock directly might not be a good idea, but > it probably should not mean that we need to keep the inefficient code > forever. I would appreciate any suggestion to navigate this lock > removal from the folks on the to and cc list.
> -/* > - * Serialize cpa() (for !DEBUG_PAGEALLOC which uses large identity mappings) > - * using cpa_lock. So that we don't allow any other cpu, with stale large tlb > - * entries change the page attribute in parallel to some other cpu > - * splitting a large page entry along with changing the attribute. > - */ > -static DEFINE_SPINLOCK(cpa_lock);
Yeah, so I'm *really* tempted to just remove cpa_lock if there's no in-code documented uses of it - your patch provides *exhaustive* background.
The thing is, even in the worst-case if it breaks anything, it will get investigated, documented better and maybe reverted - which would *still* be an improvement over today, because we turn undocumented code into documented code.
We cannot indefinitely keep a global lock just because we fear it might have some undocumented dependencies...
But no strong feelings either way - I've added a few more Cc:s to discuss this more widely.
Unless there's objections I'd be inclined to give this patch a try, and keep an eye open for regressions, it's not difficult to revert either.
Thanks,
Ingo
| |