On Tue, 2021-08-31 at 11:40 +0300, Mike Rapoport wrote: > On Mon, Aug 30, 2021 at 04:59:13PM -0700, Rick Edgecombe wrote: <trim> > > > > diff --git a/arch/x86/include/asm/pgalloc.h > > b/arch/x86/include/asm/pgalloc.h > > index c7ec5bb88334..1ff308ea76cd 100644 > > --- a/arch/x86/include/asm/pgalloc.h > > +++ b/arch/x86/include/asm/pgalloc.h > > @@ -7,6 +7,10 @@ > > #include <linux/pagemap.h> > > > > #define __HAVE_ARCH_PTE_ALLOC_ONE > > +#ifdef CONFIG_PKS_PG_TABLES > > +#define __HAVE_ARCH_FREE_TABLE > > +#define __HAVE_ARCH_ALLOC_TABLE > > I think one define would suffice. If we'd ever have an architecture > that > can implement only one of those, we update the ifdefery in > asm-generic/pgalloc.h > Good point, I'll change it. > > +#endif > > #define __HAVE_ARCH_PGD_FREE > > #include <asm-generic/pgalloc.h> > > > > @@ -162,7 +166,7 @@ static inline void p4d_free(struct mm_struct > > *mm, p4d_t *p4d) > > return; > > > > BUG_ON((unsigned long)p4d & (PAGE_SIZE-1)); > > - free_page((unsigned long)p4d); > > + free_table(virt_to_page(p4d)); > > } > > > > extern void ___p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d); > > ... > > > diff --git a/include/asm-generic/pgalloc.h b/include/asm- > > generic/pgalloc.h > > index 02932efad3ab..e576c19abc8c 100644 > > --- a/include/asm-generic/pgalloc.h > > +++ b/include/asm-generic/pgalloc.h > > @@ -2,11 +2,26 @@ > > #ifndef __ASM_GENERIC_PGALLOC_H > > #define __ASM_GENERIC_PGALLOC_H > > > > +#include <linux/mm.h> > > + > > Why is this required? > > > #ifdef CONFIG_MMU > > > > #define GFP_PGTABLE_KERNEL (GFP_KERNEL | __GFP_ZERO) > > #define GFP_PGTABLE_USER (GFP_PGTABLE_KERNEL | __GFP_ACCOUNT) > > > > +#ifndef __HAVE_ARCH_ALLOC_TABLE > > +static inline struct page *alloc_table(gfp_t gfp) > > +{ > > + return alloc_page(gfp); > > +} > > +#else /* __HAVE_ARCH_ALLOC_TABLE */ > > +extern struct page *alloc_table(gfp_t gfp); > > +#endif /* __HAVE_ARCH_ALLOC_TABLE */ > > + > > +#ifdef __HAVE_ARCH_FREE_TABLE > > +extern void free_table(struct page *); > > +#endif /* __HAVE_ARCH_FREE_TABLE */ > > + > > /** > > * __pte_alloc_one_kernel - allocate a page for PTE-level kernel > > page table > > * @mm: the mm_struct of the current context > > ... > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index c13c7af7cad3..ab63d5a201cb 100644 > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > @@ -2327,6 +2327,13 @@ static inline bool ptlock_init(struct page > > *page) { return true; } > > static inline void ptlock_free(struct page *page) {} > > #endif /* USE_SPLIT_PTE_PTLOCKS */ > > > > +#ifndef CONFIG_PKS_PG_TABLES > > +static inline void free_table(struct page *table_page) > > +{ > > + __free_pages(table_page, 0); > > +} > > +#endif /* CONFIG_PKS_PG_TABLES */ > > + > > Can't this live in asm-generic/pgalloc.h? > Then you won't need to include linux/mm.h there. Some architectures couldn't find it in asm-generic/pgalloc.h. There is some other page table stuff in linux/mm.h so didn't seem too strange. > > > static inline void pgtable_init(void) > > { > > ptlock_cache_init(); > > @@ -2337,7 +2344,6 @@ static inline bool > > pgtable_pte_page_ctor(struct page *page) > > { > > if (!ptlock_init(page)) > > return false; > > - __SetPageTable(page); > > This change is only valid when __HAVE_ARCH_ALLOC_TABLE is set. Argh, thanks. Overall solution should still work I think. I'll rework it to support both. > > > inc_lruvec_page_state(page, NR_PAGETABLE); > > return true; > > } > > @@ -2345,7 +2351,6 @@ static inline bool > > pgtable_pte_page_ctor(struct page *page) > > static inline void pgtable_pte_page_dtor(struct page *page) > > { > > ptlock_free(page); > > - __ClearPageTable(page); > > dec_lruvec_page_state(page, NR_PAGETABLE); > > } > > > >