Re: [PATCH 02/12] mm: pgtable: introduce generic p4d_alloc_one() and p4d_free()

Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx> · Wed, 18 Dec 2024 22:42:58 +0800

On 2024/12/18 22:26, Klara Modin wrote:
Hi,

On 2024-12-14 10:02, Qi Zheng wrote:
Several architectures (arm64, riscv, x86) define p4d_alloc_one() as a
wrapper for get_zeroed_page() and p4d_free() as a wrapper for 
free_page().

For these architectures, provide a generic implementation in
asm-generic/pgalloc.h and convert them to use it. And like other levels
of page tables, add statistics for P4D level page table.

For s390, it also defines p4d_alloc_one() and p4d_free(), but it uses its
own logic, so skip it.

Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
---
  arch/arm64/include/asm/pgalloc.h | 15 ++++-----
  arch/riscv/include/asm/pgalloc.h | 25 ++++++---------
  arch/x86/include/asm/pgalloc.h   | 16 ++++------
  arch/x86/mm/pgtable.c            |  3 ++
  include/asm-generic/pgalloc.h    | 55 ++++++++++++++++++++++++++++++++
  include/linux/mm.h               | 16 ++++++++++
  6 files changed, 98 insertions(+), 32 deletions(-)

diff --git a/arch/arm64/include/asm/pgalloc.h 
b/arch/arm64/include/asm/pgalloc.h
index e75422864d1bd..679c530549327 100644
--- a/arch/arm64/include/asm/pgalloc.h
+++ b/arch/arm64/include/asm/pgalloc.h
@@ -15,6 +15,8 @@
  #define __HAVE_ARCH_PGD_FREE
  #define __HAVE_ARCH_PUD_FREE
+#define __HAVE_ARCH_P4D_ALLOC_ONE
+#define __HAVE_ARCH_P4D_FREE
  #include <asm-generic/pgalloc.h>
  #define PGD_SIZE    (PTRS_PER_PGD * sizeof(pgd_t))
@@ -87,19 +89,16 @@ static inline void pgd_populate(struct mm_struct 
*mm, pgd_t *pgdp, p4d_t *p4dp)
  static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned 
long addr)
  {
-    gfp_t gfp = GFP_PGTABLE_USER;
+    if (!pgtable_l5_enabled())
+        return NULL;
-    if (mm == &init_mm)
-        gfp = GFP_PGTABLE_KERNEL;
-    return (p4d_t *)get_zeroed_page(gfp);
+    return __p4d_alloc_one(mm, addr);
  }
  static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
  {
-    if (!pgtable_l5_enabled())
-        return;
-    BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
-    free_page((unsigned long)p4d);
+    if (pgtable_l5_enabled())
+        __p4d_free(mm, p4d);
  }
  #define __p4d_free_tlb(tlb, p4d, addr)  p4d_free((tlb)->mm, p4d)
diff --git a/arch/riscv/include/asm/pgalloc.h 
b/arch/riscv/include/asm/pgalloc.h
index f52264304f772..bb6e1c5f1fb19 100644
--- a/arch/riscv/include/asm/pgalloc.h
+++ b/arch/riscv/include/asm/pgalloc.h
@@ -14,6 +14,8 @@
  #ifdef CONFIG_MMU
  #define __HAVE_ARCH_PUD_ALLOC_ONE
  #define __HAVE_ARCH_PUD_FREE
+#define __HAVE_ARCH_P4D_ALLOC_ONE
+#define __HAVE_ARCH_P4D_FREE
  #include <asm-generic/pgalloc.h>
  static inline void riscv_tlb_remove_ptdesc(struct mmu_gather *tlb, 
void *pt)
@@ -118,21 +120,10 @@ static inline void __pud_free_tlb(struct 
mmu_gather *tlb, pud_t *pud,
  #define p4d_alloc_one p4d_alloc_one
  static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned 
long addr)
  {
-    if (pgtable_l5_enabled) {
-        gfp_t gfp = GFP_PGTABLE_USER;
-
-        if (mm == &init_mm)
-            gfp = GFP_PGTABLE_KERNEL;
-        return (p4d_t *)get_zeroed_page(gfp);
-    }
+    if (!pgtable_l5_enabled)
+        return NULL;
-    return NULL;
-}
-
-static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
-{
-    BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
-    free_page((unsigned long)p4d);
+    return __p4d_alloc_one(mm, addr);
  }
  #define p4d_free p4d_free
@@ -145,8 +136,12 @@ static inline void p4d_free(struct mm_struct *mm, 
p4d_t *p4d)
  static inline void __p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d,
                    unsigned long addr)
  {
-    if (pgtable_l5_enabled)
+    if (pgtable_l5_enabled) {
+        struct ptdesc *ptdesc = virt_to_ptdesc(p4d);
+
+        pagetable_p4d_dtor(ptdesc);
          riscv_tlb_remove_ptdesc(tlb, virt_to_ptdesc(p4d));
+    }
  }
  #endif /* __PAGETABLE_PMD_FOLDED */
diff --git a/arch/x86/include/asm/pgalloc.h 
b/arch/x86/include/asm/pgalloc.h
index dcd836b59bebd..d9bc6cae77c9e 100644
--- a/arch/x86/include/asm/pgalloc.h
+++ b/arch/x86/include/asm/pgalloc.h
@@ -8,6 +8,8 @@
  #define __HAVE_ARCH_PTE_ALLOC_ONE
  #define __HAVE_ARCH_PGD_FREE
+#define __HAVE_ARCH_P4D_ALLOC_ONE
+#define __HAVE_ARCH_P4D_FREE
  #include <asm-generic/pgalloc.h>
  static inline int  __paravirt_pgd_alloc(struct mm_struct *mm) { 
return 0; }
@@ -149,20 +151,16 @@ static inline void pgd_populate_safe(struct 
mm_struct *mm, pgd_t *pgd, p4d_t *p4
  static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned 
long addr)
  {
-    gfp_t gfp = GFP_KERNEL_ACCOUNT;
+    if (!pgtable_l5_enabled())
+        return NULL;
-    if (mm == &init_mm)
-        gfp &= ~__GFP_ACCOUNT;
-    return (p4d_t *)get_zeroed_page(gfp);
+    return __p4d_alloc_one(mm, addr);
  }
  static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
  {
-    if (!pgtable_l5_enabled())
-        return;
-
-    BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
-    free_page((unsigned long)p4d);
+    if (pgtable_l5_enabled())
+        return __p4d_free(mm, p4d);
  }
  extern void ___p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d);
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index 69a357b15974a..3d6e84da45b24 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -94,6 +94,9 @@ void ___pud_free_tlb(struct mmu_gather *tlb, pud_t 
*pud)
  #if CONFIG_PGTABLE_LEVELS > 4
  void ___p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d)
  {
+    struct ptdesc *ptdesc = virt_to_ptdesc(p4d);
+
+    pagetable_p4d_dtor(ptdesc);
      paravirt_release_p4d(__pa(p4d) >> PAGE_SHIFT);
      paravirt_tlb_remove_table(tlb, virt_to_page(p4d));
  }
diff --git a/include/asm-generic/pgalloc.h 
b/include/asm-generic/pgalloc.h
index 7c48f5fbf8aa7..dbf61819b3581 100644
--- a/include/asm-generic/pgalloc.h
+++ b/include/asm-generic/pgalloc.h
@@ -215,6 +215,61 @@ static inline void pud_free(struct mm_struct *mm, 
pud_t *pud)
  #endif /* CONFIG_PGTABLE_LEVELS > 3 */
+#if CONFIG_PGTABLE_LEVELS > 4
+
+static inline p4d_t *__p4d_alloc_one_noprof(struct mm_struct *mm, 
unsigned long addr)
+{
+    gfp_t gfp = GFP_PGTABLE_USER;
+    struct ptdesc *ptdesc;
+
+    if (mm == &init_mm)
+        gfp = GFP_PGTABLE_KERNEL;
+    gfp &= ~__GFP_HIGHMEM;
+
+    ptdesc = pagetable_alloc_noprof(gfp, 0);
+    if (!ptdesc)
+        return NULL;
+
+    pagetable_p4d_ctor(ptdesc);
+    return ptdesc_address(ptdesc);
+}
+#define __p4d_alloc_one(...)    
alloc_hooks(__p4d_alloc_one_noprof(__VA_ARGS__))
+
+#ifndef __HAVE_ARCH_P4D_ALLOC_ONE
+/**
+ * p4d_alloc_one - allocate memory for a P4D-level page table
+ * @mm: the mm_struct of the current context
+ *
+ * Allocate memory for a page table using %GFP_PGTABLE_USER for user 
context
+ * and %GFP_PGTABLE_KERNEL for kernel context.
+ *
+ * Return: pointer to the allocated memory or %NULL on error
+ */
+static inline p4d_t *p4d_alloc_one_noprof(struct mm_struct *mm, 
unsigned long addr)
+{
+    return __p4d_alloc_one_noprof(mm, addr);
+}
+#define p4d_alloc_one(...)    
alloc_hooks(p4d_alloc_one_noprof(__VA_ARGS__))
+#endif
+
+static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
+{
+    struct ptdesc *ptdesc = virt_to_ptdesc(p4d);
+
+    BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
+    pagetable_p4d_dtor(ptdesc);
+    pagetable_free(ptdesc);
+}
+


+#ifndef __HAVE_ARCH_P4D_FREE
+static inline void p4d_free(struct mm_struct *mm, pud_t *p4d)

Should this perhaps be p4d_t *p4d rather than pud_t *p4d?

Yes, my bad. Will fix it.

Thanks!


Otherwise I get this build error:

In file included from 
/home/klara/git/linux/arch/riscv/include/asm/kfence.h:8,
                  from /home/klara/git/linux/mm/kfence/core.c:34:
/home/klara/git/linux/include/asm-generic/pgalloc.h: In function 
‘p4d_free’:
/home/klara/git/linux/include/asm-generic/pgalloc.h:252:24: error: 
passing argument 2 of ‘__p4d_free’ from incompatible pointer type 
[-Wincompatible-pointer-types]
   252 |         __p4d_free(mm, p4d);
       |                        ^~~
       |                        |
       |                        pud_t *
/home/klara/git/linux/include/asm-generic/pgalloc.h:244:60: note: 
expected ‘p4d_t *’ but argument is of type ‘pud_t *’
   244 | static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
       |                                                     ~~~~~~~^~~
In file included from 
/home/klara/git/linux/arch/riscv/include/asm/kfence.h:8,
                  from /home/klara/git/linux/mm/kfence/report.c:22:
/home/klara/git/linux/include/asm-generic/pgalloc.h: In function 
‘p4d_free’:
/home/klara/git/linux/include/asm-generic/pgalloc.h:252:24: error: 
passing argument 2 of ‘__p4d_free’ from incompatible pointer type 
[-Wincompatible-pointer-types]
   252 |         __p4d_free(mm, p4d);
       |                        ^~~
       |                        |
       |                        pud_t *
/home/klara/git/linux/include/asm-generic/pgalloc.h:244:60: note: 
expected ‘p4d_t *’ but argument is of type ‘pud_t *’
   244 | static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
       |                                                     ~~~~~~~^~~

Regards,
Klara Modin

+{
+    __p4d_free(mm, p4d);
+}
+#endif
+
+#endif
+
  #ifndef __HAVE_ARCH_PGD_FREE
  static inline void pgd_free(struct mm_struct *mm, pgd_t *pgd)
  {
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 5e73e53c34e9e..807a12ed8ec96 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3237,6 +3237,22 @@ static inline void pagetable_pud_dtor(struct 
ptdesc *ptdesc)
      lruvec_stat_sub_folio(folio, NR_PAGETABLE);
  }
+static inline void pagetable_p4d_ctor(struct ptdesc *ptdesc)
+{
+    struct folio *folio = ptdesc_folio(ptdesc);
+
+    __folio_set_pgtable(folio);
+    lruvec_stat_add_folio(folio, NR_PAGETABLE);
+}
+
+static inline void pagetable_p4d_dtor(struct ptdesc *ptdesc)
+{
+    struct folio *folio = ptdesc_folio(ptdesc);
+
+    __folio_clear_pgtable(folio);
+    lruvec_stat_sub_folio(folio, NR_PAGETABLE);
+}
+
  extern void __init pagecache_init(void);
  extern void free_initmem(void);








