[PATCH v6 02/12] x86/mm: remove pv_ops.mmu.tlb_remove_table call

Rik van Riel posted 12 patches 2 weeks, 2 days ago
There is a newer version of this series
[PATCH v6 02/12] x86/mm: remove pv_ops.mmu.tlb_remove_table call
Posted by Rik van Riel 2 weeks, 2 days ago
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(-)

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,
-- 
2.47.1
Re: [PATCH v6 02/12] x86/mm: remove pv_ops.mmu.tlb_remove_table call
Posted by David Hildenbrand 2 weeks, 1 day ago
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?


-- 
Cheers,

David / dhildenb
Re: [PATCH v6 02/12] x86/mm: remove pv_ops.mmu.tlb_remove_table call
Posted by Rik van Riel 2 weeks, 1 day ago
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.

-- 
All Rights Reversed.
Re: [PATCH v6 02/12] x86/mm: remove pv_ops.mmu.tlb_remove_table call
Posted by David Hildenbrand 2 weeks, 1 day ago
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.

-- 
Cheers,

David / dhildenb
Re: [PATCH v6 02/12] x86/mm: remove pv_ops.mmu.tlb_remove_table call
Posted by Peter Zijlstra 2 weeks, 1 day ago
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
Re: [PATCH v6 02/12] x86/mm: remove pv_ops.mmu.tlb_remove_table call
Posted by Rik van Riel 1 week, 6 days ago
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.


-- 
All Rights Reversed.