| Message ID | ded78e0082c6abe455a51f0945d4ea51662f871b.1734526570.git.zhengqi.arch@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | move pagetable_*_dtor() to __tlb_remove_table() | expand |
On Wed, Dec 18, 2024 at 09:04:47PM +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); > -} > diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h > index 709830274b756..69de47c7ef3c5 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 This is still broken... maybe later patches fix up, let me go check.
On Wed, Dec 18, 2024 at 09:04:47PM +0800, Qi Zheng wrote: > 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. s390 is nearly the same, but not exactly (see below). > arch/s390/include/asm/tlb.h | 1 - > arch/s390/mm/pgalloc.c | 7 ------- ... > --- 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); With this change you introduce these duplication in __tlb_remove_table() and in pagetable_dtor_free(): pagetable_dtor(ptdesc); pagetable_free(ptdesc); But I still would prefer to avoid __HAVE_ARCH_TLB_REMOVE_TABLE. Could it be possible to move pagetable_dtor_free() to asm-generic and make s390 __tlb_remove_table() version generic? > -} > - > #ifdef CONFIG_TRANSPARENT_HUGEPAGE > static void pte_free_now(struct rcu_head *head) > { Thanks!
On 2024/12/20 19:18, Alexander Gordeev wrote: > On Wed, Dec 18, 2024 at 09:04:47PM +0800, Qi Zheng wrote: >> 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. > > s390 is nearly the same, but not exactly (see below). > >> arch/s390/include/asm/tlb.h | 1 - >> arch/s390/mm/pgalloc.c | 7 ------- > ... >> --- 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); > > With this change you introduce these duplication in __tlb_remove_table() > and in pagetable_dtor_free(): > > pagetable_dtor(ptdesc); > pagetable_free(ptdesc); > > But I still would prefer to avoid __HAVE_ARCH_TLB_REMOVE_TABLE. > Could it be possible to move pagetable_dtor_free() to asm-generic and > make s390 __tlb_remove_table() version generic? Yes, see my reply to Peter below: https://lore.kernel.org/lkml/3f9a5475-930d-4f45-8fc8-8d2e8a6a08cc@bytedance.com/ I will do it in v3. And I have make pagetable_dtor_free() generic in [PATCH v2 15/15]. > >> -} >> - >> #ifdef CONFIG_TRANSPARENT_HUGEPAGE >> static void pte_free_now(struct rcu_head *head) >> { > > Thanks!
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..69de47c7ef3c5 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 */
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(-)