Re: [PATCH] x86/mm/pat: allocate split page tables as kernel page tables

[Mike Rapoport]

On Tue, Jul 21, 2026 at 01:09:27PM +0100, Lorenzo Stoakes (ARM) wrote:
> On Tue, Jul 21, 2026 at 01:00:53PM +0100, Lorenzo Stoakes (ARM) wrote:
> > On Tue, Jul 21, 2026 at 01:32:44PM +0300, Mike Rapoport wrote:
> > > On Tue, Jul 21, 2026 at 10:58:50AM +0100, Lorenzo Stoakes (ARM) wrote:
> > > > On Tue, Jul 21, 2026 at 02:45:43AM -0700, Vishal Moola wrote:
> > > > > >
> > > > > > Well some kernel page tables are still allocated without ctor (early allocated
> > > > > > direct map for isntance), and if you did pagetable_dtor_free() it
> > > > > > unconditionally calls pagetable_dtor().
> > >
> > > TBH, I cannot think of a scenario when page tables allocated at boot would
> > > be collapsed. But surely, checking the page type is safer just in case.
> >
> > Yeah nor can to be honest, anything that could be made large in the direct map
> > would already be large right?
> >
> > But it's 'just in case' somebody did something dumb :) Later can maybe make it a
> > WARN_ON(). But just to fix the proximate issue for now.
> >
> > >
> > > > > > The ptlock_free() and __folio_clear_pgtable() there would be harmelss (no locks
> > > > > > assigned for kernel page table, and if PG_table never set clearing it is a noop)
> > > > > > but the lruvec_stat_sub_folio() would cause an unbalanced decrement of
> > > > > > nr_page_table_pages.
> > > > >
> > > > > Gotcha, thanks for the explanation :)
> > > >
> > > > No worries, this is subtle stuff with lots of weird gotchas and stuff we need to
> > > > improve... I seem to have fallen down an unexpected rabbit hole with these fixes
> > > > :)
> > > >
> > > > >
> > > > > > It sucks, but until everything is updated to call the ctor we have to do it this
> > > > > > way :>)
> > > > >
> > > > > Yeah that makes sense. Although I'd rather see the condition as:
> > > > > if(PageTable(ptdesc_page(...)))
> > > > >
> > > > > We really shouldn't be calling ptdesc_folio() anywhere anymore.
> > > >
> > > > I think better for a follow up since the code already uses ptdesc all over the
> > > > place (fundamental to the approach really, keeping a list of page tables etc.)
> > > > and this is a fix that needs backporting.
> > >
> > > I agree with Vishal that it's better to use page type rather than folio
> > > type. And it's the same for backporting ;-)
> >
> > Ah sorry misunderstood, you mean straight up PageTable(ptdesc_page()), I thought
> > Vishal was saying we shouldn't be directly referencing ptdesc's at all (which
> > would be the rework).
> >
> > I guess definitionally page tables are never folios. I lazily went with what I
> > saw elsewhere, my bad :)
> 
> Ah yeah I remember now, i saw __pagetable_ctor() dealt with folios:
> 
> static inline void __pagetable_ctor(struct ptdesc *ptdesc)
> {
> 	struct folio *folio = ptdesc_folio(ptdesc);
> 
> 	__folio_set_pgtable(folio);
> 	lruvec_stat_add_folio(folio, NR_PAGETABLE);
> }
> 
> 
> And was like 'huh?' (surely definitionally they're _not_ folios) but went with
> that on that basis.

I wonder why setting the type is even in ctor rather than in allocation.
 
> Another place to clean up I guess? (that one _definitely_ is a follow up though
> ;)

Yep :)
 
> Cheers, Lorenzo

-- 
Sincerely yours,
Mike.