Several architectures (arm, arm64, riscv, s390 and x86) define exactly the
same __tlb_remove_table(), just introduce generic __tlb_remove_table() to
eliminate these duplications.

Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
---
 arch/arm/include/asm/tlb.h      |  9 ---------
 arch/arm64/include/asm/tlb.h    |  7 -------
 arch/powerpc/include/asm/tlb.h  |  1 +
 arch/riscv/include/asm/tlb.h    | 12 ------------
 arch/s390/include/asm/tlb.h     |  1 -
 arch/s390/mm/pgalloc.c          |  7 -------
 arch/sparc/include/asm/tlb_32.h |  1 +
 arch/sparc/include/asm/tlb_64.h |  1 +
 arch/x86/include/asm/tlb.h      | 17 -----------------
 include/asm-generic/tlb.h       | 15 +++++++++++++--
 10 files changed, 16 insertions(+), 55 deletions(-)

diff --git a/arch/arm/include/asm/tlb.h b/arch/arm/include/asm/tlb.h
index 264ab635e807a..ea4fbe7b17f6f 100644
--- a/arch/arm/include/asm/tlb.h
+++ b/arch/arm/include/asm/tlb.h
@@ -27,15 +27,6 @@
 #else /* !CONFIG_MMU */
 
 #include <asm/tlbflush.h>
-
-static inline void __tlb_remove_table(void *_table)
-{
-	struct ptdesc *ptdesc = (struct ptdesc *)_table;
-
-	pagetable_dtor(ptdesc);
-	pagetable_free(ptdesc);
-}
-
 #include <asm-generic/tlb.h>
 
 static inline void
diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h
index 93591a80b5bfb..8d762607285cc 100644
--- a/arch/arm64/include/asm/tlb.h
+++ b/arch/arm64/include/asm/tlb.h
@@ -10,13 +10,6 @@
 
 #include <linux/pagemap.h>
 
-static inline void __tlb_remove_table(void *_table)
-{
-	struct ptdesc *ptdesc = (struct ptdesc *)_table;
-
-	pagetable_dtor(ptdesc);
-	pagetable_free(ptdesc);
-}
 
 #define tlb_flush tlb_flush
 static void tlb_flush(struct mmu_gather *tlb);
diff --git a/arch/powerpc/include/asm/tlb.h b/arch/powerpc/include/asm/tlb.h
index 1ca7d4c4b90db..2058e8d3e0138 100644
--- a/arch/powerpc/include/asm/tlb.h
+++ b/arch/powerpc/include/asm/tlb.h
@@ -37,6 +37,7 @@ extern void tlb_flush(struct mmu_gather *tlb);
  */
 #define tlb_needs_table_invalidate()	radix_enabled()
 
+#define __HAVE_ARCH_TLB_REMOVE_TABLE
 /* Get the generic bits... */
 #include <asm-generic/tlb.h>
 
diff --git a/arch/riscv/include/asm/tlb.h b/arch/riscv/include/asm/tlb.h
index ded8724b3c4f7..50b63b5c15bd8 100644
--- a/arch/riscv/include/asm/tlb.h
+++ b/arch/riscv/include/asm/tlb.h
@@ -10,18 +10,6 @@ struct mmu_gather;
 
 static void tlb_flush(struct mmu_gather *tlb);
 
-#ifdef CONFIG_MMU
-
-static inline void __tlb_remove_table(void *table)
-{
-	struct ptdesc *ptdesc = (struct ptdesc *)table;
-
-	pagetable_dtor(ptdesc);
-	pagetable_free(ptdesc);
-}
-
-#endif /* CONFIG_MMU */
-
 #define tlb_flush tlb_flush
 #include <asm-generic/tlb.h>
 
diff --git a/arch/s390/include/asm/tlb.h b/arch/s390/include/asm/tlb.h
index 79df7c0932c56..7052780740349 100644
--- a/arch/s390/include/asm/tlb.h
+++ b/arch/s390/include/asm/tlb.h
@@ -22,7 +22,6 @@
  * Pages used for the page tables is a different story. FIXME: more
  */
 
-void __tlb_remove_table(void *_table);
 static inline void tlb_flush(struct mmu_gather *tlb);
 static inline bool __tlb_remove_page_size(struct mmu_gather *tlb,
 		struct page *page, bool delay_rmap, int page_size);
diff --git a/arch/s390/mm/pgalloc.c b/arch/s390/mm/pgalloc.c
index c73b89811a264..3e002dea6278f 100644
--- a/arch/s390/mm/pgalloc.c
+++ b/arch/s390/mm/pgalloc.c
@@ -193,13 +193,6 @@ void page_table_free(struct mm_struct *mm, unsigned long *table)
 	pagetable_dtor_free(ptdesc);
 }
 
-void __tlb_remove_table(void *table)
-{
-	struct ptdesc *ptdesc = virt_to_ptdesc(table);
-
-	pagetable_dtor_free(ptdesc);
-}
-
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 static void pte_free_now(struct rcu_head *head)
 {
diff --git a/arch/sparc/include/asm/tlb_32.h b/arch/sparc/include/asm/tlb_32.h
index 5cd28a8793e39..910254867dfbd 100644
--- a/arch/sparc/include/asm/tlb_32.h
+++ b/arch/sparc/include/asm/tlb_32.h
@@ -2,6 +2,7 @@
 #ifndef _SPARC_TLB_H
 #define _SPARC_TLB_H
 
+#define __HAVE_ARCH_TLB_REMOVE_TABLE
 #include <asm-generic/tlb.h>
 
 #endif /* _SPARC_TLB_H */
diff --git a/arch/sparc/include/asm/tlb_64.h b/arch/sparc/include/asm/tlb_64.h
index 3037187482db7..1a6e694418e39 100644
--- a/arch/sparc/include/asm/tlb_64.h
+++ b/arch/sparc/include/asm/tlb_64.h
@@ -33,6 +33,7 @@ void flush_tlb_pending(void);
 #define tlb_needs_table_invalidate()	(false)
 #endif
 
+#define __HAVE_ARCH_TLB_REMOVE_TABLE
 #include <asm-generic/tlb.h>
 
 #endif /* _SPARC64_TLB_H */
diff --git a/arch/x86/include/asm/tlb.h b/arch/x86/include/asm/tlb.h
index f64730be5ad67..3858dbf75880e 100644
--- a/arch/x86/include/asm/tlb.h
+++ b/arch/x86/include/asm/tlb.h
@@ -20,23 +20,6 @@ static inline void tlb_flush(struct mmu_gather *tlb)
 	flush_tlb_mm_range(tlb->mm, start, end, stride_shift, tlb->freed_tables);
 }
 
-/*
- * While x86 architecture in general requires an IPI to perform TLB
- * shootdown, enablement code for several hypervisors overrides
- * .flush_tlb_others hook in pv_mmu_ops and implements it by issuing
- * a hypercall. To keep software pagetable walkers safe in this case we
- * switch to RCU based table free (MMU_GATHER_RCU_TABLE_FREE). See the comment
- * below 'ifdef CONFIG_MMU_GATHER_RCU_TABLE_FREE' in include/asm-generic/tlb.h
- * for more details.
- */
-static inline void __tlb_remove_table(void *table)
-{
-	struct ptdesc *ptdesc = (struct ptdesc *)table;
-
-	pagetable_dtor(ptdesc);
-	pagetable_free(ptdesc);
-}
-
 #ifdef CONFIG_PT_RECLAIM
 static inline void __tlb_remove_table_one_rcu(struct rcu_head *head)
 {
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index 709830274b756..939a813023d7e 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -153,8 +153,9 @@
  *
  *  Useful if your architecture has non-page page directories.
  *
- *  When used, an architecture is expected to provide __tlb_remove_table()
- *  which does the actual freeing of these pages.
+ *  When used, an architecture is expected to provide __tlb_remove_table() or
+ *  use the generic __tlb_remove_table(), which does the actual freeing of these
+ *  pages.
  *
  *  MMU_GATHER_RCU_TABLE_FREE
  *
@@ -207,6 +208,16 @@ struct mmu_table_batch {
 #define MAX_TABLE_BATCH		\
 	((PAGE_SIZE - sizeof(struct mmu_table_batch)) / sizeof(void *))
 
+#ifndef __HAVE_ARCH_TLB_REMOVE_TABLE
+static inline void __tlb_remove_table(void *_table)
+{
+	struct ptdesc *ptdesc = (struct ptdesc *)_table;
+
+	pagetable_dtor(ptdesc);
+	pagetable_free(ptdesc);
+}
+#endif
+
 extern void tlb_remove_table(struct mmu_gather *tlb, void *table);
 
 #else /* !CONFIG_MMU_GATHER_HAVE_TABLE_FREE */
-- 
2.20.1

On Sat, Dec 14, 2024 at 05:02:57PM +0800, Qi Zheng wrote:

> diff --git a/arch/s390/mm/pgalloc.c b/arch/s390/mm/pgalloc.c
> index c73b89811a264..3e002dea6278f 100644
> --- a/arch/s390/mm/pgalloc.c
> +++ b/arch/s390/mm/pgalloc.c
> @@ -193,13 +193,6 @@ void page_table_free(struct mm_struct *mm, unsigned long *table)
>  	pagetable_dtor_free(ptdesc);
>  }
>  
> -void __tlb_remove_table(void *table)
> -{
> -	struct ptdesc *ptdesc = virt_to_ptdesc(table);
> -
> -	pagetable_dtor_free(ptdesc);
> -}
> -
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  static void pte_free_now(struct rcu_head *head)
>  {

> diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
> index 709830274b756..939a813023d7e 100644
> --- a/include/asm-generic/tlb.h
> +++ b/include/asm-generic/tlb.h

>  #define MAX_TABLE_BATCH		\
>  	((PAGE_SIZE - sizeof(struct mmu_table_batch)) / sizeof(void *))
>  
> +#ifndef __HAVE_ARCH_TLB_REMOVE_TABLE
> +static inline void __tlb_remove_table(void *_table)
> +{
> +	struct ptdesc *ptdesc = (struct ptdesc *)_table;
> +
> +	pagetable_dtor(ptdesc);
> +	pagetable_free(ptdesc);
> +}
> +#endif


Spot the fail...

That said, all this ptdesc stuff is another giant trainwreck. Let me
clean that up for you.

---
diff --git a/arch/arm/include/asm/tlb.h b/arch/arm/include/asm/tlb.h
index ea4fbe7b17f6..ac3881ec342f 100644
--- a/arch/arm/include/asm/tlb.h
+++ b/arch/arm/include/asm/tlb.h
@@ -32,8 +32,6 @@
 static inline void
 __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte, unsigned long addr)
 {
-	struct ptdesc *ptdesc = page_ptdesc(pte);
-
 #ifndef CONFIG_ARM_LPAE
 	/*
 	 * With the classic ARM MMU, a pte page has two corresponding pmd
@@ -43,16 +41,14 @@ __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte, unsigned long addr)
 	__tlb_adjust_range(tlb, addr - PAGE_SIZE, 2 * PAGE_SIZE);
 #endif
 
-	tlb_remove_ptdesc(tlb, ptdesc);
+	tlb_remove_table(tlb, pte);
 }
 
 static inline void
 __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmdp, unsigned long addr)
 {
 #ifdef CONFIG_ARM_LPAE
-	struct ptdesc *ptdesc = virt_to_ptdesc(pmdp);
-
-	tlb_remove_ptdesc(tlb, ptdesc);
+	tlb_remove_table(tlb, virt_to_page(pmdp));
 #endif
 }
 
diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h
index 8d762607285c..4a60569fed69 100644
--- a/arch/arm64/include/asm/tlb.h
+++ b/arch/arm64/include/asm/tlb.h
@@ -75,18 +75,14 @@ static inline void tlb_flush(struct mmu_gather *tlb)
 static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte,
 				  unsigned long addr)
 {
-	struct ptdesc *ptdesc = page_ptdesc(pte);
-
-	tlb_remove_ptdesc(tlb, ptdesc);
+	tlb_remove_table(tlb, pte);
 }
 
 #if CONFIG_PGTABLE_LEVELS > 2
 static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmdp,
 				  unsigned long addr)
 {
-	struct ptdesc *ptdesc = virt_to_ptdesc(pmdp);
-
-	tlb_remove_ptdesc(tlb, ptdesc);
+	tlb_remove_table(tlb, virt_to_page(pmdp));
 }
 #endif
 
@@ -94,12 +90,10 @@ static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmdp,
 static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pudp,
 				  unsigned long addr)
 {
-	struct ptdesc *ptdesc = virt_to_ptdesc(pudp);
-
 	if (!pgtable_l4_enabled())
 		return;
 
-	tlb_remove_ptdesc(tlb, ptdesc);
+	tlb_remove_table(tlb, virt_to_page(pudp));
 }
 #endif
 
@@ -107,12 +101,10 @@ static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pudp,
 static inline void __p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4dp,
 				  unsigned long addr)
 {
-	struct ptdesc *ptdesc = virt_to_ptdesc(p4dp);
-
 	if (!pgtable_l5_enabled())
 		return;
 
-	tlb_remove_ptdesc(tlb, ptdesc);
+	tlb_remove_table(tlb, virt_to_page(p4dp));
 }
 #endif
 
diff --git a/arch/csky/include/asm/pgalloc.h b/arch/csky/include/asm/pgalloc.h
index f1ce5b7b28f2..2c0897624699 100644
--- a/arch/csky/include/asm/pgalloc.h
+++ b/arch/csky/include/asm/pgalloc.h
@@ -64,7 +64,7 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm)
 #define __pte_free_tlb(tlb, pte, address)		\
 do {							\
 	pagetable_dtor(page_ptdesc(pte));		\
-	tlb_remove_page_ptdesc(tlb, page_ptdesc(pte));	\
+	tlb_remove_page(tlb, pte);			\
 } while (0)
 
 extern void pagetable_init(void);
diff --git a/arch/hexagon/include/asm/pgalloc.h b/arch/hexagon/include/asm/pgalloc.h
index 40e42a0e7167..8b1550498f1b 100644
--- a/arch/hexagon/include/asm/pgalloc.h
+++ b/arch/hexagon/include/asm/pgalloc.h
@@ -90,7 +90,7 @@ static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmd,
 #define __pte_free_tlb(tlb, pte, addr)				\
 do {								\
 	pagetable_dtor((page_ptdesc(pte)));			\
-	tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte)));	\
+	tlb_remove_page((tlb), (pte));				\
 } while (0)
 
 #endif
diff --git a/arch/loongarch/include/asm/pgalloc.h b/arch/loongarch/include/asm/pgalloc.h
index 7211dff8c969..5a4f22aeb618 100644
--- a/arch/loongarch/include/asm/pgalloc.h
+++ b/arch/loongarch/include/asm/pgalloc.h
@@ -58,7 +58,7 @@ static inline pte_t *pte_alloc_one_kernel(struct mm_struct *mm)
 #define __pte_free_tlb(tlb, pte, address)			\
 do {								\
 	pagetable_dtor(page_ptdesc(pte));			\
-	tlb_remove_page_ptdesc((tlb), page_ptdesc(pte));	\
+	tlb_remove_page((tlb), (pte));				\
 } while (0)
 
 #ifndef __PAGETABLE_PMD_FOLDED
diff --git a/arch/m68k/include/asm/sun3_pgalloc.h b/arch/m68k/include/asm/sun3_pgalloc.h
index 2b626cb3ad0a..63d9f95f5e3d 100644
--- a/arch/m68k/include/asm/sun3_pgalloc.h
+++ b/arch/m68k/include/asm/sun3_pgalloc.h
@@ -20,7 +20,7 @@ extern const char bad_pmd_string[];
 #define __pte_free_tlb(tlb, pte, addr)				\
 do {								\
 	pagetable_dtor(page_ptdesc(pte));			\
-	tlb_remove_page_ptdesc((tlb), page_ptdesc(pte));	\
+	tlb_remove_page((tlb), (pte));				\
 } while (0)
 
 static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmd, pte_t *pte)
diff --git a/arch/mips/include/asm/pgalloc.h b/arch/mips/include/asm/pgalloc.h
index 36d9805033c4..bbee21345154 100644
--- a/arch/mips/include/asm/pgalloc.h
+++ b/arch/mips/include/asm/pgalloc.h
@@ -57,7 +57,7 @@ static inline void pgd_free(struct mm_struct *mm, pgd_t *pgd)
 #define __pte_free_tlb(tlb, pte, address)			\
 do {								\
 	pagetable_dtor(page_ptdesc(pte));			\
-	tlb_remove_page_ptdesc((tlb), page_ptdesc(pte));	\
+	tlb_remove_page((tlb), (pte));				\
 } while (0)
 
 #ifndef __PAGETABLE_PMD_FOLDED
diff --git a/arch/nios2/include/asm/pgalloc.h b/arch/nios2/include/asm/pgalloc.h
index 12a536b7bfbd..641cec8fb2a2 100644
--- a/arch/nios2/include/asm/pgalloc.h
+++ b/arch/nios2/include/asm/pgalloc.h
@@ -31,7 +31,7 @@ extern pgd_t *pgd_alloc(struct mm_struct *mm);
 #define __pte_free_tlb(tlb, pte, addr)					\
 	do {								\
 		pagetable_dtor(page_ptdesc(pte));			\
-		tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte)));	\
+		tlb_remove_page((tlb), (pte));				\
 	} while (0)
 
 #endif /* _ASM_NIOS2_PGALLOC_H */
diff --git a/arch/openrisc/include/asm/pgalloc.h b/arch/openrisc/include/asm/pgalloc.h
index 596e2355824e..e9b9bc53ece0 100644
--- a/arch/openrisc/include/asm/pgalloc.h
+++ b/arch/openrisc/include/asm/pgalloc.h
@@ -69,7 +69,7 @@ extern pte_t *pte_alloc_one_kernel(struct mm_struct *mm);
 #define __pte_free_tlb(tlb, pte, addr)				\
 do {								\
 	pagetable_dtor(page_ptdesc(pte));			\
-	tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte)));	\
+	tlb_remove_page((tlb), (pte));				\
 } while (0)
 
 #endif
diff --git a/arch/riscv/include/asm/pgalloc.h b/arch/riscv/include/asm/pgalloc.h
index fc50d1401024..baedbd2546b9 100644
--- a/arch/riscv/include/asm/pgalloc.h
+++ b/arch/riscv/include/asm/pgalloc.h
@@ -26,13 +26,13 @@
  * comment below 'ifdef CONFIG_MMU_GATHER_RCU_TABLE_FREE' in include/asm-generic/tlb.h
  * for more details.
  */
-static inline void riscv_tlb_remove_ptdesc(struct mmu_gather *tlb, void *pt)
+static inline void riscv_tlb_remove_table(struct mmu_gather *tlb, void *pt)
 {
 	if (riscv_use_sbi_for_rfence()) {
-		tlb_remove_ptdesc(tlb, pt);
+		tlb_remove_table(tlb, pt);
 	} else {
 		pagetable_dtor(pt);
-		tlb_remove_page_ptdesc(tlb, pt);
+		tlb_remove_page(tlb, pt);
 	}
 }
 
@@ -120,7 +120,7 @@ static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pud,
 				  unsigned long addr)
 {
 	if (pgtable_l4_enabled)
-		riscv_tlb_remove_ptdesc(tlb, virt_to_ptdesc(pud));
+		riscv_tlb_remove_table(tlb, virt_to_page(pud));
 }
 
 #define p4d_alloc_one p4d_alloc_one
@@ -143,7 +143,7 @@ static inline void __p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d,
 				  unsigned long addr)
 {
 	if (pgtable_l5_enabled)
-		riscv_tlb_remove_ptdesc(tlb, virt_to_ptdesc(p4d));
+		riscv_tlb_remove_table(tlb, virt_to_page(p4d));
 }
 #endif /* __PAGETABLE_PMD_FOLDED */
 
@@ -172,7 +172,7 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm)
 static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd,
 				  unsigned long addr)
 {
-	riscv_tlb_remove_ptdesc(tlb, virt_to_ptdesc(pmd));
+	riscv_tlb_remove_table(tlb, virt_to_page(pmd));
 }
 
 #endif /* __PAGETABLE_PMD_FOLDED */
@@ -180,7 +180,7 @@ static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd,
 static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte,
 				  unsigned long addr)
 {
-	riscv_tlb_remove_ptdesc(tlb, page_ptdesc(pte));
+	riscv_tlb_remove_table(tlb, pte);
 }
 #endif /* CONFIG_MMU */
 
diff --git a/arch/s390/include/asm/tlb.h b/arch/s390/include/asm/tlb.h
index 705278074034..fba11949dd2e 100644
--- a/arch/s390/include/asm/tlb.h
+++ b/arch/s390/include/asm/tlb.h
@@ -86,7 +86,7 @@ static inline void pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte,
 	tlb->cleared_pmds = 1;
 	if (mm_alloc_pgste(tlb->mm))
 		gmap_unlink(tlb->mm, (unsigned long *)pte, address);
-	tlb_remove_ptdesc(tlb, pte);
+	tlb_remove_table(tlb, pte);
 }
 
 /*
@@ -105,7 +105,7 @@ static inline void pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd,
 	tlb->mm->context.flush_mm = 1;
 	tlb->freed_tables = 1;
 	tlb->cleared_puds = 1;
-	tlb_remove_ptdesc(tlb, pmd);
+	tlb_remove_table(tlb, pmd);
 }
 
 /*
@@ -123,7 +123,7 @@ static inline void pud_free_tlb(struct mmu_gather *tlb, pud_t *pud,
 	tlb->mm->context.flush_mm = 1;
 	tlb->freed_tables = 1;
 	tlb->cleared_p4ds = 1;
-	tlb_remove_ptdesc(tlb, pud);
+	tlb_remove_table(tlb, pud);
 }
 
 /*
@@ -141,7 +141,7 @@ static inline void p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d,
 	__tlb_adjust_range(tlb, address, PAGE_SIZE);
 	tlb->mm->context.flush_mm = 1;
 	tlb->freed_tables = 1;
-	tlb_remove_ptdesc(tlb, p4d);
+	tlb_remove_table(tlb, p4d);
 }
 
 #endif /* _S390_TLB_H */
diff --git a/arch/sh/include/asm/pgalloc.h b/arch/sh/include/asm/pgalloc.h
index 96d938fdf224..43812b2363ef 100644
--- a/arch/sh/include/asm/pgalloc.h
+++ b/arch/sh/include/asm/pgalloc.h
@@ -35,7 +35,7 @@ static inline void pmd_populate(struct mm_struct *mm, pmd_t *pmd,
 #define __pte_free_tlb(tlb, pte, addr)				\
 do {								\
 	pagetable_dtor(page_ptdesc(pte));			\
-	tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte)));	\
+	tlb_remove_page((tlb), (pte));				\
 } while (0)
 
 #endif /* __ASM_SH_PGALLOC_H */
diff --git a/arch/um/include/asm/pgalloc.h b/arch/um/include/asm/pgalloc.h
index f0af23c3aeb2..aa6063dc5b1e 100644
--- a/arch/um/include/asm/pgalloc.h
+++ b/arch/um/include/asm/pgalloc.h
@@ -28,7 +28,7 @@ extern pgd_t *pgd_alloc(struct mm_struct *);
 #define __pte_free_tlb(tlb, pte, address)			\
 do {								\
 	pagetable_dtor(page_ptdesc(pte));			\
-	tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte)));	\
+	tlb_remove_page((tlb), (pte));				\
 } while (0)
 
 #if CONFIG_PGTABLE_LEVELS > 2
@@ -36,15 +36,15 @@ do {								\
 #define __pmd_free_tlb(tlb, pmd, address)			\
 do {								\
 	pagetable_dtor(virt_to_ptdesc(pmd));			\
-	tlb_remove_page_ptdesc((tlb), virt_to_ptdesc(pmd));	\
+	tlb_remove_page((tlb), virt_page(pmd));			\
 } while (0)
 
 #if CONFIG_PGTABLE_LEVELS > 3
 
 #define __pud_free_tlb(tlb, pud, address)			\
 do {								\
-	pagetable_dtor(virt_to_ptdesc(pud));		\
-	tlb_remove_page_ptdesc((tlb), virt_to_ptdesc(pud));	\
+	pagetable_dtor(virt_to_ptdesc(pud));			\
+	tlb_remove_page((tlb), virt_to_page(pud));		\
 } while (0)
 
 #endif
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index 939a813023d7..7991950e98f6 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -209,9 +209,9 @@ struct mmu_table_batch {
 	((PAGE_SIZE - sizeof(struct mmu_table_batch)) / sizeof(void *))
 
 #ifndef __HAVE_ARCH_TLB_REMOVE_TABLE
-static inline void __tlb_remove_table(void *_table)
+static inline void __tlb_remove_table(void *table)
 {
-	struct ptdesc *ptdesc = (struct ptdesc *)_table;
+	struct ptdesc *ptdesc = page_to_ptdesc(table);
 
 	pagetable_dtor(ptdesc);
 	pagetable_free(ptdesc);
@@ -499,17 +499,6 @@ static inline void tlb_remove_page(struct mmu_gather *tlb, struct page *page)
 	return tlb_remove_page_size(tlb, page, PAGE_SIZE);
 }
 
-static inline void tlb_remove_ptdesc(struct mmu_gather *tlb, void *pt)
-{
-	tlb_remove_table(tlb, pt);
-}
-
-/* Like tlb_remove_ptdesc, but for page-like page directories. */
-static inline void tlb_remove_page_ptdesc(struct mmu_gather *tlb, struct ptdesc *pt)
-{
-	tlb_remove_page(tlb, ptdesc_page(pt));
-}
-
 static inline void tlb_change_page_size(struct mmu_gather *tlb,
 						     unsigned int page_size)
 {

On 2024/12/16 20:00, Peter Zijlstra wrote:
> On Sat, Dec 14, 2024 at 05:02:57PM +0800, Qi Zheng wrote:

[...]

>>   
>> +#ifndef __HAVE_ARCH_TLB_REMOVE_TABLE
>> +static inline void __tlb_remove_table(void *_table)
>> +{
>> +	struct ptdesc *ptdesc = (struct ptdesc *)_table;
>> +
>> +	pagetable_dtor(ptdesc);
>> +	pagetable_free(ptdesc);
>> +}
>> +#endif
> 
> 
> Spot the fail...
> 
> That said, all this ptdesc stuff is another giant trainwreck. Let me
> clean that up for you.

It looks like you want to revert what was done in this patch series:

https://lore.kernel.org/all/20230807230513.102486-1-vishal.moola@gmail.com/

But why? It seems that splitting ptdesc from struct page is a good
thing?

> 
> ---
> diff --git a/arch/arm/include/asm/tlb.h b/arch/arm/include/asm/tlb.h
> index ea4fbe7b17f6..ac3881ec342f 100644
> --- a/arch/arm/include/asm/tlb.h
> +++ b/arch/arm/include/asm/tlb.h
> @@ -32,8 +32,6 @@
>   static inline void
>   __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte, unsigned long addr)
>   {
> -	struct ptdesc *ptdesc = page_ptdesc(pte);
> -
>   #ifndef CONFIG_ARM_LPAE
>   	/*
>   	 * With the classic ARM MMU, a pte page has two corresponding pmd
> @@ -43,16 +41,14 @@ __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte, unsigned long addr)
>   	__tlb_adjust_range(tlb, addr - PAGE_SIZE, 2 * PAGE_SIZE);
>   #endif
>   
> -	tlb_remove_ptdesc(tlb, ptdesc);
> +	tlb_remove_table(tlb, pte);
>   }
>   
>   static inline void
>   __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmdp, unsigned long addr)
>   {
>   #ifdef CONFIG_ARM_LPAE
> -	struct ptdesc *ptdesc = virt_to_ptdesc(pmdp);
> -
> -	tlb_remove_ptdesc(tlb, ptdesc);
> +	tlb_remove_table(tlb, virt_to_page(pmdp));
>   #endif
>   }
>   
> diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h
> index 8d762607285c..4a60569fed69 100644
> --- a/arch/arm64/include/asm/tlb.h
> +++ b/arch/arm64/include/asm/tlb.h
> @@ -75,18 +75,14 @@ static inline void tlb_flush(struct mmu_gather *tlb)
>   static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte,
>   				  unsigned long addr)
>   {
> -	struct ptdesc *ptdesc = page_ptdesc(pte);
> -
> -	tlb_remove_ptdesc(tlb, ptdesc);
> +	tlb_remove_table(tlb, pte);
>   }
>   
>   #if CONFIG_PGTABLE_LEVELS > 2
>   static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmdp,
>   				  unsigned long addr)
>   {
> -	struct ptdesc *ptdesc = virt_to_ptdesc(pmdp);
> -
> -	tlb_remove_ptdesc(tlb, ptdesc);
> +	tlb_remove_table(tlb, virt_to_page(pmdp));
>   }
>   #endif
>   
> @@ -94,12 +90,10 @@ static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmdp,
>   static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pudp,
>   				  unsigned long addr)
>   {
> -	struct ptdesc *ptdesc = virt_to_ptdesc(pudp);
> -
>   	if (!pgtable_l4_enabled())
>   		return;
>   
> -	tlb_remove_ptdesc(tlb, ptdesc);
> +	tlb_remove_table(tlb, virt_to_page(pudp));
>   }
>   #endif
>   
> @@ -107,12 +101,10 @@ static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pudp,
>   static inline void __p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4dp,
>   				  unsigned long addr)
>   {
> -	struct ptdesc *ptdesc = virt_to_ptdesc(p4dp);
> -
>   	if (!pgtable_l5_enabled())
>   		return;
>   
> -	tlb_remove_ptdesc(tlb, ptdesc);
> +	tlb_remove_table(tlb, virt_to_page(p4dp));
>   }
>   #endif
>   
> diff --git a/arch/csky/include/asm/pgalloc.h b/arch/csky/include/asm/pgalloc.h
> index f1ce5b7b28f2..2c0897624699 100644
> --- a/arch/csky/include/asm/pgalloc.h
> +++ b/arch/csky/include/asm/pgalloc.h
> @@ -64,7 +64,7 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm)
>   #define __pte_free_tlb(tlb, pte, address)		\
>   do {							\
>   	pagetable_dtor(page_ptdesc(pte));		\
> -	tlb_remove_page_ptdesc(tlb, page_ptdesc(pte));	\
> +	tlb_remove_page(tlb, pte);			\
>   } while (0)
>   
>   extern void pagetable_init(void);
> diff --git a/arch/hexagon/include/asm/pgalloc.h b/arch/hexagon/include/asm/pgalloc.h
> index 40e42a0e7167..8b1550498f1b 100644
> --- a/arch/hexagon/include/asm/pgalloc.h
> +++ b/arch/hexagon/include/asm/pgalloc.h
> @@ -90,7 +90,7 @@ static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmd,
>   #define __pte_free_tlb(tlb, pte, addr)				\
>   do {								\
>   	pagetable_dtor((page_ptdesc(pte)));			\
> -	tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte)));	\
> +	tlb_remove_page((tlb), (pte));				\
>   } while (0)
>   
>   #endif
> diff --git a/arch/loongarch/include/asm/pgalloc.h b/arch/loongarch/include/asm/pgalloc.h
> index 7211dff8c969..5a4f22aeb618 100644
> --- a/arch/loongarch/include/asm/pgalloc.h
> +++ b/arch/loongarch/include/asm/pgalloc.h
> @@ -58,7 +58,7 @@ static inline pte_t *pte_alloc_one_kernel(struct mm_struct *mm)
>   #define __pte_free_tlb(tlb, pte, address)			\
>   do {								\
>   	pagetable_dtor(page_ptdesc(pte));			\
> -	tlb_remove_page_ptdesc((tlb), page_ptdesc(pte));	\
> +	tlb_remove_page((tlb), (pte));				\
>   } while (0)
>   
>   #ifndef __PAGETABLE_PMD_FOLDED
> diff --git a/arch/m68k/include/asm/sun3_pgalloc.h b/arch/m68k/include/asm/sun3_pgalloc.h
> index 2b626cb3ad0a..63d9f95f5e3d 100644
> --- a/arch/m68k/include/asm/sun3_pgalloc.h
> +++ b/arch/m68k/include/asm/sun3_pgalloc.h
> @@ -20,7 +20,7 @@ extern const char bad_pmd_string[];
>   #define __pte_free_tlb(tlb, pte, addr)				\
>   do {								\
>   	pagetable_dtor(page_ptdesc(pte));			\
> -	tlb_remove_page_ptdesc((tlb), page_ptdesc(pte));	\
> +	tlb_remove_page((tlb), (pte));				\
>   } while (0)
>   
>   static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmd, pte_t *pte)
> diff --git a/arch/mips/include/asm/pgalloc.h b/arch/mips/include/asm/pgalloc.h
> index 36d9805033c4..bbee21345154 100644
> --- a/arch/mips/include/asm/pgalloc.h
> +++ b/arch/mips/include/asm/pgalloc.h
> @@ -57,7 +57,7 @@ static inline void pgd_free(struct mm_struct *mm, pgd_t *pgd)
>   #define __pte_free_tlb(tlb, pte, address)			\
>   do {								\
>   	pagetable_dtor(page_ptdesc(pte));			\
> -	tlb_remove_page_ptdesc((tlb), page_ptdesc(pte));	\
> +	tlb_remove_page((tlb), (pte));				\
>   } while (0)
>   
>   #ifndef __PAGETABLE_PMD_FOLDED
> diff --git a/arch/nios2/include/asm/pgalloc.h b/arch/nios2/include/asm/pgalloc.h
> index 12a536b7bfbd..641cec8fb2a2 100644
> --- a/arch/nios2/include/asm/pgalloc.h
> +++ b/arch/nios2/include/asm/pgalloc.h
> @@ -31,7 +31,7 @@ extern pgd_t *pgd_alloc(struct mm_struct *mm);
>   #define __pte_free_tlb(tlb, pte, addr)					\
>   	do {								\
>   		pagetable_dtor(page_ptdesc(pte));			\
> -		tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte)));	\
> +		tlb_remove_page((tlb), (pte));				\
>   	} while (0)
>   
>   #endif /* _ASM_NIOS2_PGALLOC_H */
> diff --git a/arch/openrisc/include/asm/pgalloc.h b/arch/openrisc/include/asm/pgalloc.h
> index 596e2355824e..e9b9bc53ece0 100644
> --- a/arch/openrisc/include/asm/pgalloc.h
> +++ b/arch/openrisc/include/asm/pgalloc.h
> @@ -69,7 +69,7 @@ extern pte_t *pte_alloc_one_kernel(struct mm_struct *mm);
>   #define __pte_free_tlb(tlb, pte, addr)				\
>   do {								\
>   	pagetable_dtor(page_ptdesc(pte));			\
> -	tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte)));	\
> +	tlb_remove_page((tlb), (pte));				\
>   } while (0)
>   
>   #endif
> diff --git a/arch/riscv/include/asm/pgalloc.h b/arch/riscv/include/asm/pgalloc.h
> index fc50d1401024..baedbd2546b9 100644
> --- a/arch/riscv/include/asm/pgalloc.h
> +++ b/arch/riscv/include/asm/pgalloc.h
> @@ -26,13 +26,13 @@
>    * comment below 'ifdef CONFIG_MMU_GATHER_RCU_TABLE_FREE' in include/asm-generic/tlb.h
>    * for more details.
>    */
> -static inline void riscv_tlb_remove_ptdesc(struct mmu_gather *tlb, void *pt)
> +static inline void riscv_tlb_remove_table(struct mmu_gather *tlb, void *pt)
>   {
>   	if (riscv_use_sbi_for_rfence()) {
> -		tlb_remove_ptdesc(tlb, pt);
> +		tlb_remove_table(tlb, pt);
>   	} else {
>   		pagetable_dtor(pt);
> -		tlb_remove_page_ptdesc(tlb, pt);
> +		tlb_remove_page(tlb, pt);
>   	}
>   }
>   
> @@ -120,7 +120,7 @@ static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pud,
>   				  unsigned long addr)
>   {
>   	if (pgtable_l4_enabled)
> -		riscv_tlb_remove_ptdesc(tlb, virt_to_ptdesc(pud));
> +		riscv_tlb_remove_table(tlb, virt_to_page(pud));
>   }
>   
>   #define p4d_alloc_one p4d_alloc_one
> @@ -143,7 +143,7 @@ static inline void __p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d,
>   				  unsigned long addr)
>   {
>   	if (pgtable_l5_enabled)
> -		riscv_tlb_remove_ptdesc(tlb, virt_to_ptdesc(p4d));
> +		riscv_tlb_remove_table(tlb, virt_to_page(p4d));
>   }
>   #endif /* __PAGETABLE_PMD_FOLDED */
>   
> @@ -172,7 +172,7 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm)
>   static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd,
>   				  unsigned long addr)
>   {
> -	riscv_tlb_remove_ptdesc(tlb, virt_to_ptdesc(pmd));
> +	riscv_tlb_remove_table(tlb, virt_to_page(pmd));
>   }
>   
>   #endif /* __PAGETABLE_PMD_FOLDED */
> @@ -180,7 +180,7 @@ static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd,
>   static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte,
>   				  unsigned long addr)
>   {
> -	riscv_tlb_remove_ptdesc(tlb, page_ptdesc(pte));
> +	riscv_tlb_remove_table(tlb, pte);
>   }
>   #endif /* CONFIG_MMU */
>   
> diff --git a/arch/s390/include/asm/tlb.h b/arch/s390/include/asm/tlb.h
> index 705278074034..fba11949dd2e 100644
> --- a/arch/s390/include/asm/tlb.h
> +++ b/arch/s390/include/asm/tlb.h
> @@ -86,7 +86,7 @@ static inline void pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte,
>   	tlb->cleared_pmds = 1;
>   	if (mm_alloc_pgste(tlb->mm))
>   		gmap_unlink(tlb->mm, (unsigned long *)pte, address);
> -	tlb_remove_ptdesc(tlb, pte);
> +	tlb_remove_table(tlb, pte);
>   }
>   
>   /*
> @@ -105,7 +105,7 @@ static inline void pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd,
>   	tlb->mm->context.flush_mm = 1;
>   	tlb->freed_tables = 1;
>   	tlb->cleared_puds = 1;
> -	tlb_remove_ptdesc(tlb, pmd);
> +	tlb_remove_table(tlb, pmd);
>   }
>   
>   /*
> @@ -123,7 +123,7 @@ static inline void pud_free_tlb(struct mmu_gather *tlb, pud_t *pud,
>   	tlb->mm->context.flush_mm = 1;
>   	tlb->freed_tables = 1;
>   	tlb->cleared_p4ds = 1;
> -	tlb_remove_ptdesc(tlb, pud);
> +	tlb_remove_table(tlb, pud);
>   }
>   
>   /*
> @@ -141,7 +141,7 @@ static inline void p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d,
>   	__tlb_adjust_range(tlb, address, PAGE_SIZE);
>   	tlb->mm->context.flush_mm = 1;
>   	tlb->freed_tables = 1;
> -	tlb_remove_ptdesc(tlb, p4d);
> +	tlb_remove_table(tlb, p4d);
>   }
>   
>   #endif /* _S390_TLB_H */
> diff --git a/arch/sh/include/asm/pgalloc.h b/arch/sh/include/asm/pgalloc.h
> index 96d938fdf224..43812b2363ef 100644
> --- a/arch/sh/include/asm/pgalloc.h
> +++ b/arch/sh/include/asm/pgalloc.h
> @@ -35,7 +35,7 @@ static inline void pmd_populate(struct mm_struct *mm, pmd_t *pmd,
>   #define __pte_free_tlb(tlb, pte, addr)				\
>   do {								\
>   	pagetable_dtor(page_ptdesc(pte));			\
> -	tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte)));	\
> +	tlb_remove_page((tlb), (pte));				\
>   } while (0)
>   
>   #endif /* __ASM_SH_PGALLOC_H */
> diff --git a/arch/um/include/asm/pgalloc.h b/arch/um/include/asm/pgalloc.h
> index f0af23c3aeb2..aa6063dc5b1e 100644
> --- a/arch/um/include/asm/pgalloc.h
> +++ b/arch/um/include/asm/pgalloc.h
> @@ -28,7 +28,7 @@ extern pgd_t *pgd_alloc(struct mm_struct *);
>   #define __pte_free_tlb(tlb, pte, address)			\
>   do {								\
>   	pagetable_dtor(page_ptdesc(pte));			\
> -	tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte)));	\
> +	tlb_remove_page((tlb), (pte));				\
>   } while (0)
>   
>   #if CONFIG_PGTABLE_LEVELS > 2
> @@ -36,15 +36,15 @@ do {								\
>   #define __pmd_free_tlb(tlb, pmd, address)			\
>   do {								\
>   	pagetable_dtor(virt_to_ptdesc(pmd));			\
> -	tlb_remove_page_ptdesc((tlb), virt_to_ptdesc(pmd));	\
> +	tlb_remove_page((tlb), virt_page(pmd));			\
>   } while (0)
>   
>   #if CONFIG_PGTABLE_LEVELS > 3
>   
>   #define __pud_free_tlb(tlb, pud, address)			\
>   do {								\
> -	pagetable_dtor(virt_to_ptdesc(pud));		\
> -	tlb_remove_page_ptdesc((tlb), virt_to_ptdesc(pud));	\
> +	pagetable_dtor(virt_to_ptdesc(pud));			\
> +	tlb_remove_page((tlb), virt_to_page(pud));		\
>   } while (0)
>   
>   #endif
> diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
> index 939a813023d7..7991950e98f6 100644
> --- a/include/asm-generic/tlb.h
> +++ b/include/asm-generic/tlb.h
> @@ -209,9 +209,9 @@ struct mmu_table_batch {
>   	((PAGE_SIZE - sizeof(struct mmu_table_batch)) / sizeof(void *))
>   
>   #ifndef __HAVE_ARCH_TLB_REMOVE_TABLE
> -static inline void __tlb_remove_table(void *_table)
> +static inline void __tlb_remove_table(void *table)
>   {
> -	struct ptdesc *ptdesc = (struct ptdesc *)_table;
> +	struct ptdesc *ptdesc = page_to_ptdesc(table);
>   
>   	pagetable_dtor(ptdesc);
>   	pagetable_free(ptdesc);
> @@ -499,17 +499,6 @@ static inline void tlb_remove_page(struct mmu_gather *tlb, struct page *page)
>   	return tlb_remove_page_size(tlb, page, PAGE_SIZE);
>   }
>   
> -static inline void tlb_remove_ptdesc(struct mmu_gather *tlb, void *pt)
> -{
> -	tlb_remove_table(tlb, pt);
> -}
> -
> -/* Like tlb_remove_ptdesc, but for page-like page directories. */
> -static inline void tlb_remove_page_ptdesc(struct mmu_gather *tlb, struct ptdesc *pt)
> -{
> -	tlb_remove_page(tlb, ptdesc_page(pt));
> -}
> -
>   static inline void tlb_change_page_size(struct mmu_gather *tlb,
>   						     unsigned int page_size)
>   {

On Mon, Dec 16, 2024 at 08:52:06PM +0800, Qi Zheng wrote:
> 
> 
> On 2024/12/16 20:00, Peter Zijlstra wrote:
> > On Sat, Dec 14, 2024 at 05:02:57PM +0800, Qi Zheng wrote:
> 
> [...]
> 
> > > +#ifndef __HAVE_ARCH_TLB_REMOVE_TABLE
> > > +static inline void __tlb_remove_table(void *_table)
> > > +{
> > > +	struct ptdesc *ptdesc = (struct ptdesc *)_table;
> > > +
> > > +	pagetable_dtor(ptdesc);
> > > +	pagetable_free(ptdesc);
> > > +}
> > > +#endif
> > 
> > 
> > Spot the fail...
> > 
> > That said, all this ptdesc stuff is another giant trainwreck. Let me
> > clean that up for you.
> 
> It looks like you want to revert what was done in this patch series:
> 
> https://lore.kernel.org/all/20230807230513.102486-1-vishal.moola@gmail.com/
> 
> But why? It seems that splitting ptdesc from struct page is a good
> thing?

Because we're explicitly allocating pages for the page-tables, and also,
code like:

tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte)));

static inline void tlb_remove_page_ptdesc(struct mmu_gather *tlb, struct ptdesc *pt)
{
	tlb_remove_page(tlb, ptdesc_page(pt));
}

Just makes me upset.

Just bloody write tlb_remove_page() and call it a day.

All that nonsense is just obfuscation at this point.

On 2024/12/17 02:12, Peter Zijlstra wrote:
> On Mon, Dec 16, 2024 at 08:52:06PM +0800, Qi Zheng wrote:
>>
>>
>> On 2024/12/16 20:00, Peter Zijlstra wrote:
>>> On Sat, Dec 14, 2024 at 05:02:57PM +0800, Qi Zheng wrote:
>>
>> [...]
>>
>>>> +#ifndef __HAVE_ARCH_TLB_REMOVE_TABLE
>>>> +static inline void __tlb_remove_table(void *_table)
>>>> +{
>>>> +	struct ptdesc *ptdesc = (struct ptdesc *)_table;
>>>> +
>>>> +	pagetable_dtor(ptdesc);
>>>> +	pagetable_free(ptdesc);
>>>> +}
>>>> +#endif
>>>
>>>
>>> Spot the fail...
>>>
>>> That said, all this ptdesc stuff is another giant trainwreck. Let me
>>> clean that up for you.
>>
>> It looks like you want to revert what was done in this patch series:
>>
>> https://lore.kernel.org/all/20230807230513.102486-1-vishal.moola@gmail.com/
>>
>> But why? It seems that splitting ptdesc from struct page is a good
>> thing?
> 
> Because we're explicitly allocating pages for the page-tables, and also,
> code like:
> 
> tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte)));
> 
> static inline void tlb_remove_page_ptdesc(struct mmu_gather *tlb, struct ptdesc *pt)
> {
> 	tlb_remove_page(tlb, ptdesc_page(pt));
> }
> 
> Just makes me upset.

Aha, this strikes me as odd too.

Also +CC Vishal Moola, the author of ptdesc, who may be able to provide
more background information. If Vishal has no objection, I will try to
remove tlb_remove_ptdesc() and tlb_remove_page_ptdesc() as Peter suggested.

> 
> Just bloody write tlb_remove_page() and call it a day.
> 
> All that nonsense is just obfuscation at this point.

In addition, will remove the duplicates of __tlb_remove_table,
asm-generic/pgalloc.h pte_free(), pmd_free(), __pud_free() and
__p4d_free() as follows:

diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h
index 3673e9c29504e..370f5b579ff88 100644
--- a/include/asm-generic/pgalloc.h
+++ b/include/asm-generic/pgalloc.h
@@ -107,10 +107,7 @@ static inline pgtable_t pte_alloc_one_noprof(struct 
mm_struct *mm)
   */
  static inline void pte_free(struct mm_struct *mm, struct page *pte_page)
  {
-       struct ptdesc *ptdesc = page_ptdesc(pte_page);
-
-       pagetable_dtor(ptdesc);
-       pagetable_free(ptdesc);
+       pagetable_dtor_free(pte_page);
  }


@@ -150,11 +147,7 @@ static inline pmd_t *pmd_alloc_one_noprof(struct 
mm_struct *mm, unsigned long ad
  #ifndef __HAVE_ARCH_PMD_FREE
  static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
  {
-       struct ptdesc *ptdesc = virt_to_ptdesc(pmd);
-
-       BUG_ON((unsigned long)pmd & (PAGE_SIZE-1));
-       pagetable_dtor(ptdesc);
-       pagetable_free(ptdesc);
+       pagetable_dtor_free(virt_to_page(pmd));
  }
  #endif

@@ -199,11 +192,7 @@ static inline pud_t *pud_alloc_one_noprof(struct 
mm_struct *mm, unsigned long ad

  static inline void __pud_free(struct mm_struct *mm, pud_t *pud)
  {
-       struct ptdesc *ptdesc = virt_to_ptdesc(pud);
-
-       BUG_ON((unsigned long)pud & (PAGE_SIZE-1));
-       pagetable_dtor(ptdesc);
-       pagetable_free(ptdesc);
+       pagetable_dtor_free(virt_to_page(pud));
  }

  #ifndef __HAVE_ARCH_PUD_FREE
@@ -254,11 +243,7 @@ static inline p4d_t *p4d_alloc_one_noprof(struct 
mm_struct *mm, unsigned long ad

  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_dtor(ptdesc);
-       pagetable_free(ptdesc);
+       pagetable_dtor_free(virt_to_page(p4d));
  }

  #ifndef __HAVE_ARCH_P4D_FREE
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index 939a813023d7e..b34d014c23ef0 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -211,10 +211,7 @@ struct mmu_table_batch {
  #ifndef __HAVE_ARCH_TLB_REMOVE_TABLE
  static inline void __tlb_remove_table(void *_table)
  {
-       struct ptdesc *ptdesc = (struct ptdesc *)_table;
-
-       pagetable_dtor(ptdesc);
-       pagetable_free(ptdesc);
+       pagetable_dtor_free(_table);
  }
  #endif

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 497035a78849b..11829860ec05e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3064,6 +3064,14 @@ static inline void pagetable_dtor(struct ptdesc 
*ptdesc)
         lruvec_stat_sub_folio(folio, NR_PAGETABLE);
  }

+static inline void pagetable_dtor_free(void *table)
+{
+       struct ptdesc *ptdesc = page_ptdesc((struct page *)table);
+
+        pagetable_dtor(ptdesc);
+        pagetable_dtor(ptdesc);
+}
+
  static inline bool pagetable_pte_ctor(struct ptdesc *ptdesc)
  {
         struct folio *folio = ptdesc_folio(ptdesc);

Thanks!

On Tue, Dec 17, 2024 at 11:42:02AM +0800, Qi Zheng wrote:
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 497035a78849b..11829860ec05e 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3064,6 +3064,14 @@ static inline void pagetable_dtor(struct ptdesc
> *ptdesc)
>         lruvec_stat_sub_folio(folio, NR_PAGETABLE);
>  }
> 
> +static inline void pagetable_dtor_free(void *table)
> +{
> +       struct ptdesc *ptdesc = page_ptdesc((struct page *)table);
> +
> +        pagetable_dtor(ptdesc);
> +        pagetable_dtor(ptdesc);
> +}

Right, that works, except you have whitespace issues and I think you'll
find it'll work better if you don't call _dtor twice but instead replace
that last one with _free() :-)

On 2024/12/17 17:02, Peter Zijlstra wrote:
> On Tue, Dec 17, 2024 at 11:42:02AM +0800, Qi Zheng wrote:
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 497035a78849b..11829860ec05e 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -3064,6 +3064,14 @@ static inline void pagetable_dtor(struct ptdesc
>> *ptdesc)
>>          lruvec_stat_sub_folio(folio, NR_PAGETABLE);
>>   }
>>
>> +static inline void pagetable_dtor_free(void *table)
>> +{
>> +       struct ptdesc *ptdesc = page_ptdesc((struct page *)table);
>> +
>> +        pagetable_dtor(ptdesc);
>> +        pagetable_dtor(ptdesc);
>> +}
> 
> Right, that works, except you have whitespace issues and I think you'll
> find it'll work better if you don't call _dtor twice but instead replace
> that last one with _free() :-)

Ah, stupid thing I did, please ignore it. ;)

Will add this to v2.

Thanks!

On Mon, Dec 16, 2024 at 01:00:43PM +0100, Peter Zijlstra wrote:
> On Sat, Dec 14, 2024 at 05:02:57PM +0800, Qi Zheng wrote:
> 
> > diff --git a/arch/s390/mm/pgalloc.c b/arch/s390/mm/pgalloc.c
> > index c73b89811a264..3e002dea6278f 100644
> > --- a/arch/s390/mm/pgalloc.c
> > +++ b/arch/s390/mm/pgalloc.c
> > @@ -193,13 +193,6 @@ void page_table_free(struct mm_struct *mm, unsigned long *table)
> >  	pagetable_dtor_free(ptdesc);
> >  }
> >  
> > -void __tlb_remove_table(void *table)
> > -{
> > -	struct ptdesc *ptdesc = virt_to_ptdesc(table);
> > -
> > -	pagetable_dtor_free(ptdesc);
> > -}
> > -
> >  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> >  static void pte_free_now(struct rcu_head *head)
> >  {
> 
> > diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
> > index 709830274b756..939a813023d7e 100644
> > --- a/include/asm-generic/tlb.h
> > +++ b/include/asm-generic/tlb.h
> 
> >  #define MAX_TABLE_BATCH		\
> >  	((PAGE_SIZE - sizeof(struct mmu_table_batch)) / sizeof(void *))
> >  
> > +#ifndef __HAVE_ARCH_TLB_REMOVE_TABLE
> > +static inline void __tlb_remove_table(void *_table)
> > +{
> > +	struct ptdesc *ptdesc = (struct ptdesc *)_table;
> > +
> > +	pagetable_dtor(ptdesc);
> > +	pagetable_free(ptdesc);
> > +}
> > +#endif
> 
> 
> Spot the fail...
> 
> That said, all this ptdesc stuff is another giant trainwreck. Let me
> clean that up for you.
> 
> ---

> -static inline void __tlb_remove_table(void *_table)
> +static inline void __tlb_remove_table(void *table)
>  {
> -	struct ptdesc *ptdesc = (struct ptdesc *)_table;
> +	struct ptdesc *ptdesc = page_to_ptdesc(table);
>  
>  	pagetable_dtor(ptdesc);
>  	pagetable_free(ptdesc);

And now observe that __tlb_remove_table() is identical to
asm-generic/pgalloc.h pte_free(), pmd_free(), __pud_free() and
__p4d_free().

And I'm sure we don't need 5 copies of this :-), wdyt?

On Mon, Dec 16, 2024 at 01:03:13PM +0100, Peter Zijlstra wrote:
> On Mon, Dec 16, 2024 at 01:00:43PM +0100, Peter Zijlstra wrote:
> > On Sat, Dec 14, 2024 at 05:02:57PM +0800, Qi Zheng wrote:
> > 
> > > diff --git a/arch/s390/mm/pgalloc.c b/arch/s390/mm/pgalloc.c
> > > index c73b89811a264..3e002dea6278f 100644
> > > --- a/arch/s390/mm/pgalloc.c
> > > +++ b/arch/s390/mm/pgalloc.c
> > > @@ -193,13 +193,6 @@ void page_table_free(struct mm_struct *mm, unsigned long *table)
> > >  	pagetable_dtor_free(ptdesc);
> > >  }
> > >  
> > > -void __tlb_remove_table(void *table)
> > > -{
> > > -	struct ptdesc *ptdesc = virt_to_ptdesc(table);
> > > -
> > > -	pagetable_dtor_free(ptdesc);
> > > -}
> > > -
> > >  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > >  static void pte_free_now(struct rcu_head *head)
> > >  {
> > 
> > > diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
> > > index 709830274b756..939a813023d7e 100644
> > > --- a/include/asm-generic/tlb.h
> > > +++ b/include/asm-generic/tlb.h
> > 
> > >  #define MAX_TABLE_BATCH		\
> > >  	((PAGE_SIZE - sizeof(struct mmu_table_batch)) / sizeof(void *))
> > >  
> > > +#ifndef __HAVE_ARCH_TLB_REMOVE_TABLE
> > > +static inline void __tlb_remove_table(void *_table)
> > > +{
> > > +	struct ptdesc *ptdesc = (struct ptdesc *)_table;
> > > +
> > > +	pagetable_dtor(ptdesc);
> > > +	pagetable_free(ptdesc);
> > > +}
> > > +#endif
> > 
> > 
> > Spot the fail...
> > 
> > That said, all this ptdesc stuff is another giant trainwreck. Let me
> > clean that up for you.
> > 
> > ---
> 
> > -static inline void __tlb_remove_table(void *_table)
> > +static inline void __tlb_remove_table(void *table)
> >  {
> > -	struct ptdesc *ptdesc = (struct ptdesc *)_table;
> > +	struct ptdesc *ptdesc = page_to_ptdesc(table);
> >  
> >  	pagetable_dtor(ptdesc);
> >  	pagetable_free(ptdesc);
> 
> And now observe that __tlb_remove_table() is identical to
> asm-generic/pgalloc.h pte_free(), pmd_free(), __pud_free() and
> __p4d_free().
> 
> And I'm sure we don't need 5 copies of this :-), wdyt?

Argh, nearly, it has the whole page vs virt nonsense still going on.
Bah.

On 2024/12/16 20:05, Peter Zijlstra wrote:
> On Mon, Dec 16, 2024 at 01:03:13PM +0100, Peter Zijlstra wrote:
>> On Mon, Dec 16, 2024 at 01:00:43PM +0100, Peter Zijlstra wrote:
>>> On Sat, Dec 14, 2024 at 05:02:57PM +0800, Qi Zheng wrote:
>>>
>>>> diff --git a/arch/s390/mm/pgalloc.c b/arch/s390/mm/pgalloc.c
>>>> index c73b89811a264..3e002dea6278f 100644
>>>> --- a/arch/s390/mm/pgalloc.c
>>>> +++ b/arch/s390/mm/pgalloc.c
>>>> @@ -193,13 +193,6 @@ void page_table_free(struct mm_struct *mm, unsigned long *table)
>>>>   	pagetable_dtor_free(ptdesc);
>>>>   }
>>>>   
>>>> -void __tlb_remove_table(void *table)
>>>> -{
>>>> -	struct ptdesc *ptdesc = virt_to_ptdesc(table);
>>>> -
>>>> -	pagetable_dtor_free(ptdesc);
>>>> -}
>>>> -
>>>>   #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>>>   static void pte_free_now(struct rcu_head *head)
>>>>   {
>>>
>>>> diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
>>>> index 709830274b756..939a813023d7e 100644
>>>> --- a/include/asm-generic/tlb.h
>>>> +++ b/include/asm-generic/tlb.h
>>>
>>>>   #define MAX_TABLE_BATCH		\
>>>>   	((PAGE_SIZE - sizeof(struct mmu_table_batch)) / sizeof(void *))
>>>>   
>>>> +#ifndef __HAVE_ARCH_TLB_REMOVE_TABLE
>>>> +static inline void __tlb_remove_table(void *_table)
>>>> +{
>>>> +	struct ptdesc *ptdesc = (struct ptdesc *)_table;
>>>> +
>>>> +	pagetable_dtor(ptdesc);
>>>> +	pagetable_free(ptdesc);
>>>> +}
>>>> +#endif
>>>
>>>
>>> Spot the fail...
>>>
>>> That said, all this ptdesc stuff is another giant trainwreck. Let me
>>> clean that up for you.
>>>
>>> ---
>>
>>> -static inline void __tlb_remove_table(void *_table)
>>> +static inline void __tlb_remove_table(void *table)
>>>   {
>>> -	struct ptdesc *ptdesc = (struct ptdesc *)_table;
>>> +	struct ptdesc *ptdesc = page_to_ptdesc(table);
>>>   
>>>   	pagetable_dtor(ptdesc);
>>>   	pagetable_free(ptdesc);
>>
>> And now observe that __tlb_remove_table() is identical to
>> asm-generic/pgalloc.h pte_free(), pmd_free(), __pud_free() and
>> __p4d_free().
>>
>> And I'm sure we don't need 5 copies of this :-), wdyt?
> 
> Argh, nearly, it has the whole page vs virt nonsense still going on.
> Bah.

Yes, maybe it can be unified into struct page parameter, which seems
to be more convenient for conversion:

static inline void pagtable_dtor_free(struct mm_struct *mm, void *table)
{
	struct ptdesc *ptdesc = page_to_ptdesc(table);

	pagetable_dtor(ptdesc);
	pagetable_free(ptdesc);
}