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@xxxxxxxxx/
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!
