Re: [PATCH 1/5] x86/mm/pat: introcude cpa_lock() and cpa_unlock()

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Jul 28, 2026 at 04:55:28PM +0200, Peter Zijlstra wrote:
> On Tue, Jul 28, 2026 at 05:46:30PM +0300, Mike Rapoport wrote:
> > On Tue, Jul 28, 2026 at 04:31:35PM +0200, Peter Zijlstra wrote:
> > > On Tue, Jul 28, 2026 at 07:30:27AM -0700, Dave Hansen wrote:
> > > > On 7/28/26 07:21, Peter Zijlstra wrote:
> > > > > There was already a patch merged that removed the shole debug_pagealloc
> > > > > exception. Is that not better?
> > > > 
> > > > As I'm scanning through email this morning, there's another issue that
> > > > popped up with that patch. It's causing hangs on boot.
> > > > 
> > > > It's looking like debug pagealloc not taking the lock is actually
> > > > functional, not an optimization. Although, I hesitate to say
> > > > "functional" and would prefer to use much less nice words to describe it.
> > > 
> > > Yeah, lets figure out why that is before we retain this wart ;-)
> > 
> > As Lorenzo said:
> > 
> >   __kernel_map_pages() can be called from irq context:
> > 
> >   < GFP_ATOMIC context >
> >   kfree() or whatever
> >   -> ...
> >   -> __free_pages_prepare()
> >   -> debug_pagealloc_unmap_pages()
> >   -> __kernel_map_pages()
> >   -> __change_page_attr_set_clr()
> >   -> cpa_lock
> > 
> 
> The TLBI hack in __kernel_map_pages() makes me wonder how any of this is
> correct to begin with. That comment isn't helping.
> 

So in particular the from IRQ-disabled context thing seems to be
clearing the 'present' bit. Not doing a full TLBI on that would allow
other CPUs to still access the page, resulting a lack of faults.

I suppose that is a non-fatal case. It makes the debug code less robust
than it wants to be, but no malfunction.

But debug_page_alloc_map_pages() as used from post_alloc_hook() for a
GFP_ATOMIC alloc would result in not propagating the present bit,
resulting in concurrent faults. And I'm not immediately seeing a fixup
for that in the fault handler.

Hmm, perhaps spurious_kernel_fault() handles it, after all, it would not
actually observe !present anymore when reading the PTE, its just a stale
TLB entry at that point.

Hmmm...