| Message ID | 20250120024104.1924753-3-riel@surriel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | AMD broadcast TLB invalidation | expand |
On 20.01.25 03:40, Rik van Riel wrote: > Every pv_ops.mmu.tlb_remove_table call ends up calling tlb_remove_table. > Indeed, but the !CONFIG_PARAVIRT variant paravirt_tlb_remove_table() however calls tlb_remove_page(). tlb_remove_page() ends up in __tlb_remove_page_size()->__tlb_remove_folio_pages_size(), not in tlb_remove_table() ... but maybe I am looking at the wrong tree, so I wonder if this is okay an simply not spelled out here explicitly?
On Mon, 2025-01-20 at 20:47 +0100, David Hildenbrand wrote: > On 20.01.25 03:40, Rik van Riel wrote: > > Every pv_ops.mmu.tlb_remove_table call ends up calling > > tlb_remove_table. > > > > Indeed, but the !CONFIG_PARAVIRT variant paravirt_tlb_remove_table() > however calls tlb_remove_page(). Patch 1/12 from this series removes that. After patch 1/12, we always call tlb_remove_table everywhere.
On 21.01.25 02:03, Rik van Riel wrote: > On Mon, 2025-01-20 at 20:47 +0100, David Hildenbrand wrote: >> On 20.01.25 03:40, Rik van Riel wrote: >>> Every pv_ops.mmu.tlb_remove_table call ends up calling >>> tlb_remove_table. >>> >> >> Indeed, but the !CONFIG_PARAVIRT variant paravirt_tlb_remove_table() >> however calls tlb_remove_page(). > > Patch 1/12 from this series removes that. > > After patch 1/12, we always call tlb_remove_table everywhere. This patch contains the hunk: -#ifndef CONFIG_PARAVIRT -static inline -void paravirt_tlb_remove_table(struct mmu_gather *tlb, void *table) -{ - tlb_remove_page(tlb, table); -} -#endif - That is the source of my confusion.
On Tue, Jan 21, 2025 at 08:46:04AM +0100, David Hildenbrand wrote: > On 21.01.25 02:03, Rik van Riel wrote: > > On Mon, 2025-01-20 at 20:47 +0100, David Hildenbrand wrote: > > > On 20.01.25 03:40, Rik van Riel wrote: > > > > Every pv_ops.mmu.tlb_remove_table call ends up calling > > > > tlb_remove_table. > > > > > > > > > > Indeed, but the !CONFIG_PARAVIRT variant paravirt_tlb_remove_table() > > > however calls tlb_remove_page(). > > > > Patch 1/12 from this series removes that. > > > > After patch 1/12, we always call tlb_remove_table everywhere. > > This patch contains the hunk: > > -#ifndef CONFIG_PARAVIRT > -static inline > -void paravirt_tlb_remove_table(struct mmu_gather *tlb, void *table) > -{ > - tlb_remove_page(tlb, table); > -} > -#endif > - > > That is the source of my confusion. Ah, that hunk should probably go to patch 1
On Tue, 2025-01-21 at 09:54 +0100, Peter Zijlstra wrote: > On Tue, Jan 21, 2025 at 08:46:04AM +0100, David Hildenbrand wrote: > > On 21.01.25 02:03, Rik van Riel wrote: > > > On Mon, 2025-01-20 at 20:47 +0100, David Hildenbrand wrote: > > > > On 20.01.25 03:40, Rik van Riel wrote: > > > > > Every pv_ops.mmu.tlb_remove_table call ends up calling > > > > > tlb_remove_table. > > > > > > > > > > > > > Indeed, but the !CONFIG_PARAVIRT variant > > > > paravirt_tlb_remove_table() > > > > however calls tlb_remove_page(). > > > > > > Patch 1/12 from this series removes that. > > > > > > After patch 1/12, we always call tlb_remove_table everywhere. > > > > This patch contains the hunk: > > > > -#ifndef CONFIG_PARAVIRT > > -static inline > > -void paravirt_tlb_remove_table(struct mmu_gather *tlb, void > > *table) > > -{ > > - tlb_remove_page(tlb, table); > > -} > > -#endif > > - > > > > That is the source of my confusion. > > Ah, that hunk should probably go to patch 1 > Moved that over for the next version.
diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c index 1cc113200ff5..cbe6c71e17c1 100644 --- a/arch/x86/hyperv/mmu.c +++ b/arch/x86/hyperv/mmu.c @@ -240,5 +240,4 @@ void hyperv_setup_mmu_ops(void) pr_info("Using hypercall for remote TLB flush\n"); pv_ops.mmu.flush_tlb_multi = hyperv_flush_tlb_multi; - pv_ops.mmu.tlb_remove_table = tlb_remove_table; } diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h index d4eb9e1d61b8..794ba3647c6c 100644 --- a/arch/x86/include/asm/paravirt.h +++ b/arch/x86/include/asm/paravirt.h @@ -91,11 +91,6 @@ static inline void __flush_tlb_multi(const struct cpumask *cpumask, PVOP_VCALL2(mmu.flush_tlb_multi, cpumask, info); } -static inline void paravirt_tlb_remove_table(struct mmu_gather *tlb, void *table) -{ - PVOP_VCALL2(mmu.tlb_remove_table, tlb, table); -} - static inline void paravirt_arch_exit_mmap(struct mm_struct *mm) { PVOP_VCALL1(mmu.exit_mmap, mm); diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h index 8d4fbe1be489..13405959e4db 100644 --- a/arch/x86/include/asm/paravirt_types.h +++ b/arch/x86/include/asm/paravirt_types.h @@ -136,8 +136,6 @@ struct pv_mmu_ops { void (*flush_tlb_multi)(const struct cpumask *cpus, const struct flush_tlb_info *info); - void (*tlb_remove_table)(struct mmu_gather *tlb, void *table); - /* Hook for intercepting the destruction of an mm_struct. */ void (*exit_mmap)(struct mm_struct *mm); void (*notify_page_enc_status_changed)(unsigned long pfn, int npages, bool enc); diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c index 7a422a6c5983..3be9b3342c67 100644 --- a/arch/x86/kernel/kvm.c +++ b/arch/x86/kernel/kvm.c @@ -838,7 +838,6 @@ static void __init kvm_guest_init(void) #ifdef CONFIG_SMP if (pv_tlb_flush_supported()) { pv_ops.mmu.flush_tlb_multi = kvm_flush_tlb_multi; - pv_ops.mmu.tlb_remove_table = tlb_remove_table; pr_info("KVM setup pv remote TLB flush\n"); } diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c index 2b78a6b466ed..c019771e0123 100644 --- a/arch/x86/kernel/paravirt.c +++ b/arch/x86/kernel/paravirt.c @@ -186,7 +186,6 @@ struct paravirt_patch_template pv_ops = { .mmu.flush_tlb_kernel = native_flush_tlb_global, .mmu.flush_tlb_one_user = native_flush_tlb_one_user, .mmu.flush_tlb_multi = native_flush_tlb_multi, - .mmu.tlb_remove_table = tlb_remove_table, .mmu.exit_mmap = paravirt_nop, .mmu.notify_page_enc_status_changed = paravirt_nop, diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c index 5745a354a241..3dc4af1f7868 100644 --- a/arch/x86/mm/pgtable.c +++ b/arch/x86/mm/pgtable.c @@ -18,14 +18,6 @@ EXPORT_SYMBOL(physical_mask); #define PGTABLE_HIGHMEM 0 #endif -#ifndef CONFIG_PARAVIRT -static inline -void paravirt_tlb_remove_table(struct mmu_gather *tlb, void *table) -{ - tlb_remove_page(tlb, table); -} -#endif - gfp_t __userpte_alloc_gfp = GFP_PGTABLE_USER | PGTABLE_HIGHMEM; pgtable_t pte_alloc_one(struct mm_struct *mm) @@ -54,7 +46,7 @@ void ___pte_free_tlb(struct mmu_gather *tlb, struct page *pte) { pagetable_pte_dtor(page_ptdesc(pte)); paravirt_release_pte(page_to_pfn(pte)); - paravirt_tlb_remove_table(tlb, pte); + tlb_remove_table(tlb, pte); } #if CONFIG_PGTABLE_LEVELS > 2 @@ -70,7 +62,7 @@ void ___pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd) tlb->need_flush_all = 1; #endif pagetable_pmd_dtor(ptdesc); - paravirt_tlb_remove_table(tlb, ptdesc_page(ptdesc)); + tlb_remove_table(tlb, ptdesc_page(ptdesc)); } #if CONFIG_PGTABLE_LEVELS > 3 @@ -80,14 +72,14 @@ void ___pud_free_tlb(struct mmu_gather *tlb, pud_t *pud) pagetable_pud_dtor(ptdesc); paravirt_release_pud(__pa(pud) >> PAGE_SHIFT); - paravirt_tlb_remove_table(tlb, virt_to_page(pud)); + tlb_remove_table(tlb, virt_to_page(pud)); } #if CONFIG_PGTABLE_LEVELS > 4 void ___p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d) { paravirt_release_p4d(__pa(p4d) >> PAGE_SHIFT); - paravirt_tlb_remove_table(tlb, virt_to_page(p4d)); + tlb_remove_table(tlb, virt_to_page(p4d)); } #endif /* CONFIG_PGTABLE_LEVELS > 4 */ #endif /* CONFIG_PGTABLE_LEVELS > 3 */ diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c index 55a4996d0c04..041e17282af0 100644 --- a/arch/x86/xen/mmu_pv.c +++ b/arch/x86/xen/mmu_pv.c @@ -2137,7 +2137,6 @@ static const typeof(pv_ops) xen_mmu_ops __initconst = { .flush_tlb_kernel = xen_flush_tlb, .flush_tlb_one_user = xen_flush_tlb_one_user, .flush_tlb_multi = xen_flush_tlb_multi, - .tlb_remove_table = tlb_remove_table, .pgd_alloc = xen_pgd_alloc, .pgd_free = xen_pgd_free,
Every pv_ops.mmu.tlb_remove_table call ends up calling tlb_remove_table. Get rid of the indirection by simply calling tlb_remove_table directly, and not going through the paravirt function pointers. Signed-off-by: Rik van Riel <riel@surriel.com> Suggested-by: Qi Zheng <zhengqi.arch@bytedance.com> --- arch/x86/hyperv/mmu.c | 1 - arch/x86/include/asm/paravirt.h | 5 ----- arch/x86/include/asm/paravirt_types.h | 2 -- arch/x86/kernel/kvm.c | 1 - arch/x86/kernel/paravirt.c | 1 - arch/x86/mm/pgtable.c | 16 ++++------------ arch/x86/xen/mmu_pv.c | 1 - 7 files changed, 4 insertions(+), 23 deletions(-)