Re: [PATCH] x86/mm/pat: don't gate cpa_lock on debug_pagealloc_enabled()

[Mike Rapoport]

On Tue, Jul 21, 2026 at 04:49:13PM +0100, Lorenzo Stoakes (ARM) wrote:
> On Wed, Jul 15, 2026 at 05:45:19PM +0300, Mike Rapoport wrote:
> > From: "Mike Rapoport (Microsoft)" <rppt@xxxxxxxxxx>
> >
> > Dave Hansen says:
> >   My only question is *why*!?!? Why add extra locking complexity and rules
> >   to optimize debug_pagealloc, which is already horrendously slow.
> >
> > Stop gating cpa_lock on debug_pagealloc_enabled() to simplify the code.
> >
> > Suggested-by: Dave Hansen <dave.hansen@xxxxxxxxx>
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> > ---
> >  arch/x86/mm/pat/set_memory.c | 19 +++++++------------
> >  1 file changed, 7 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> > index d023a40a1e03..e8316f5ffa8a 100644
> > --- a/arch/x86/mm/pat/set_memory.c
> > +++ b/arch/x86/mm/pat/set_memory.c
> > @@ -62,10 +62,9 @@ enum cpa_warn {
> >  static const int cpa_warn_level = CPA_PROTECT;
> >
> >  /*
> > - * 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.
> > + * Serialize cpa() using cpa_lock so that we don't allow any other cpu, with
> > + * stale large tlb entries, to 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);
> >
> > @@ -1235,11 +1234,9 @@ static int split_large_page(struct cpa_data *cpa, pte_t *kpte,
> >  {
> >  	struct ptdesc *ptdesc;
> >
> > -	if (!debug_pagealloc_enabled())
> > -		spin_unlock(&cpa_lock);
> > +	spin_unlock(&cpa_lock);
> 
> -> _irqsave() is needed I think :) see below

I'm rather thinking that it will be simpler overall to restore the if, but
wrap it in cpa_lock()/cpa_unlock() helpers with a better comment than what
we have now above DEFINE_SPINLOCK(cpa_lock).

> >  	ptdesc = pagetable_alloc(GFP_KERNEL, 0);
> > -	if (!debug_pagealloc_enabled())
> > -		spin_lock(&cpa_lock);
> > +	spin_lock(&cpa_lock);
> 
> -> _irqrestore() as below

-- 
Sincerely yours,
Mike.