diff mbox series

[2/4] mm: rmap: Allow platforms without mm_cpumask to defer TLB flush

Message ID 20220707125242.425242-3-21cnbao@gmail.com (mailing list archive)
State New
Headers show
Series mm: arm64: bring up BATCHED_UNMAP_TLB_FLUSH | expand

Commit Message

Barry Song July 7, 2022, 12:52 p.m. UTC
From: Barry Song <v-songbaohua@oppo.com>

Platforms like ARM64 have hareware TLB shootdown broadcast. They
don't maintain mm_cpumask and they just send tlbi and related
sync instructions for TLB flush.
So if mm_cpumask is empty, we also allow deferred TLB flush

Cc: Nadav Amit <namit@vmware.com>
Cc: Mel Gorman <mgorman@suse.de>
Signed-off-by: Barry Song <v-songbaohua@oppo.com>>
---
 mm/rmap.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Nadav Amit July 8, 2022, 6:36 a.m. UTC | #1
On Jul 7, 2022, at 5:52 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Barry Song <v-songbaohua@oppo.com>
> 
> Platforms like ARM64 have hareware TLB shootdown broadcast. They
> don't maintain mm_cpumask and they just send tlbi and related
> sync instructions for TLB flush.
> So if mm_cpumask is empty, we also allow deferred TLB flush
> 
> Cc: Nadav Amit <namit@vmware.com>
> Cc: Mel Gorman <mgorman@suse.de>
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>>
> ---
> mm/rmap.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 5bcb334cd6f2..d320c29a4ad8 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -692,8 +692,13 @@ static bool should_defer_flush(struct mm_struct *mm, enum ttu_flags flags)
> 	if (!(flags & TTU_BATCH_FLUSH))
> 		return false;
> 
> -	/* If remote CPUs need to be flushed then defer batch the flush */
> -	if (cpumask_any_but(mm_cpumask(mm), get_cpu()) < nr_cpu_ids)
> +	/*
> +	 * If remote CPUs need to be flushed then defer batch the flush;
> +	 * If ARCHs like ARM64 have hardware TLB flush broadcast, thus
> +	 * they don't maintain mm_cpumask() at all, defer batch as well.
> +	 */
> +	if (cpumask_any_but(mm_cpumask(mm), get_cpu()) < nr_cpu_ids ||
> +	    cpumask_empty(mm_cpumask(mm)))

The cpumask_empty() is indeed just another memory access, which is most
likely ok. But wouldn’t adding something like CONFIG_ARCH_HAS_MM_CPUMASK
make the code simpler and (slightly, certainly slightly) more performant?
Barry Song July 8, 2022, 6:59 a.m. UTC | #2
> The cpumask_empty() is indeed just another memory access, which is most
> likely ok. But wouldn’t adding something like CONFIG_ARCH_HAS_MM_CPUMASK
> make the code simpler and (slightly, certainly slightly) more performant?

Yep. good suggestion, Nadav. So the code will be as below, right?

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index be0b95e51df6..a91d73866238 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -81,6 +81,7 @@ config X86
 	select ARCH_HAS_KCOV			if X86_64
 	select ARCH_HAS_MEM_ENCRYPT
 	select ARCH_HAS_MEMBARRIER_SYNC_CORE
+	select ARCH_HAS_MM_CPUMASK
 	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
 	select ARCH_HAS_PMEM_API		if X86_64
 	select ARCH_HAS_PTE_DEVMAP		if X86_64
diff --git a/mm/Kconfig b/mm/Kconfig
index 169e64192e48..7bf54f57ca01 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -951,6 +951,9 @@ config ARCH_HAS_CURRENT_STACK_POINTER
 	  register alias named "current_stack_pointer", this config can be
 	  selected.

+config ARCH_HAS_MM_CPUMASK
+	bool
+
 config ARCH_HAS_VM_GET_PAGE_PROT
 	bool

diff --git a/mm/rmap.c b/mm/rmap.c
index 5bcb334cd6f2..13d4f9a1d4f1 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -692,6 +692,10 @@ static bool should_defer_flush(struct mm_struct *mm, enum ttu_flags flags)
 	if (!(flags & TTU_BATCH_FLUSH))
 		return false;

+#ifndef CONFIG_ARCH_HAS_MM_CPUMASK
+	return true;
+#endif
+
 	/* If remote CPUs need to be flushed then defer batch the flush */
 	if (cpumask_any_but(mm_cpumask(mm), get_cpu()) < nr_cpu_ids)
 		should_defer = true;

Thanks
Barry
Nadav Amit July 8, 2022, 8:08 a.m. UTC | #3
On Jul 7, 2022, at 11:59 PM, Barry Song <21cnbao@gmail.com> wrote:

>> The cpumask_empty() is indeed just another memory access, which is most
>> likely ok. But wouldn’t adding something like CONFIG_ARCH_HAS_MM_CPUMASK
>> make the code simpler and (slightly, certainly slightly) more performant?
> 
> Yep. good suggestion, Nadav. So the code will be as below, right?

Hmmm… Although it is likely to work (because only x86 and arm would use this
batch flushing), I think that for consistency ARCH_HAS_MM_CPUMASK should be
correct for all architectures.

Is it really only x86 that has mm_cpumask()?
Barry Song July 8, 2022, 8:17 a.m. UTC | #4
On Fri, Jul 8, 2022 at 8:08 PM Nadav Amit <namit@vmware.com> wrote:
>
> On Jul 7, 2022, at 11:59 PM, Barry Song <21cnbao@gmail.com> wrote:
>
> >> The cpumask_empty() is indeed just another memory access, which is most
> >> likely ok. But wouldn’t adding something like CONFIG_ARCH_HAS_MM_CPUMASK
> >> make the code simpler and (slightly, certainly slightly) more performant?
> >
> > Yep. good suggestion, Nadav. So the code will be as below, right?
>
> Hmmm… Although it is likely to work (because only x86 and arm would use this
> batch flushing), I think that for consistency ARCH_HAS_MM_CPUMASK should be
> correct for all architectures.
>
> Is it really only x86 that has mm_cpumask()?

i am quite sure there are some other platforms having mm_cpumask().
for example, arm(not arm64).
but i am not exactly sure of the situation of each individual arch. thus,
i don't risk changing their source code.
but arm64 is the second platform looking for tlbbatch, and
ARCH_HAS_MM_CPUMASK only affects tlbbatch. so i would
expect those platforms to fill their ARCH_HAS_MM_CPUMASK
while they start to bringup their tlbbatch? for this moment,
we only need to make certain we don't break x86?
does it make sense?

Thanks
Barry


>
Peter Zijlstra July 8, 2022, 8:27 a.m. UTC | #5
On Fri, Jul 08, 2022 at 08:08:45AM +0000, Nadav Amit wrote:

> Is it really only x86 that has mm_cpumask()?

Unlikely, everybody who needs to IPI (eg. doesn't have broadcast
invalidate) has benefit to track this mask more accurately.

The below greps for clearing CPUs in the mask and ought to be a fair
indicator:

$ git grep -l "cpumask_clear_cpu.*mm_cpumask" arch/
arch/arm/include/asm/mmu_context.h
arch/loongarch/include/asm/mmu_context.h
arch/loongarch/mm/tlb.c
arch/mips/include/asm/mmu_context.h
arch/openrisc/mm/tlb.c
arch/powerpc/include/asm/book3s/64/mmu.h
arch/powerpc/mm/book3s64/radix_tlb.c
arch/riscv/mm/context.c
arch/s390/kernel/smp.c
arch/um/include/asm/mmu_context.h
arch/x86/mm/tlb.c
Barry Song July 11, 2022, 12:30 a.m. UTC | #6
On Fri, Jul 8, 2022 at 8:28 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, Jul 08, 2022 at 08:08:45AM +0000, Nadav Amit wrote:
>
> > Is it really only x86 that has mm_cpumask()?
>
> Unlikely, everybody who needs to IPI (eg. doesn't have broadcast
> invalidate) has benefit to track this mask more accurately.
>
> The below greps for clearing CPUs in the mask and ought to be a fair
> indicator:
>
> $ git grep -l "cpumask_clear_cpu.*mm_cpumask" arch/
> arch/arm/include/asm/mmu_context.h
> arch/loongarch/include/asm/mmu_context.h
> arch/loongarch/mm/tlb.c
> arch/mips/include/asm/mmu_context.h
> arch/openrisc/mm/tlb.c
> arch/powerpc/include/asm/book3s/64/mmu.h
> arch/powerpc/mm/book3s64/radix_tlb.c
> arch/riscv/mm/context.c
> arch/s390/kernel/smp.c
> arch/um/include/asm/mmu_context.h
> arch/x86/mm/tlb.c

so i suppose we need the below at this moment. i am not able to
test all of them. but since only x86 has already got tlbbatch
and arm64 is the second one to have tlbbatch now, i suppose the
below changes won't break those archs without tlbbatch. i would
expect people bringing up tlbbatch in those platforms to test
them later,

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 7630ba9cb6cc..25c42747f488 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -13,6 +13,7 @@ config ARM
 	select ARCH_HAS_KEEPINITRD
 	select ARCH_HAS_KCOV
 	select ARCH_HAS_MEMBARRIER_SYNC_CORE
+	select ARCH_HAS_MM_CPUMASK
 	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
 	select ARCH_HAS_PTE_SPECIAL if ARM_LPAE
 	select ARCH_HAS_PHYS_TO_DMA
diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 1920d52653b4..4b737c0d17a2 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -7,6 +7,7 @@ config LOONGARCH
 	select ARCH_ENABLE_MEMORY_HOTPLUG
 	select ARCH_ENABLE_MEMORY_HOTREMOVE
 	select ARCH_HAS_ACPI_TABLE_UPGRADE	if ACPI
+	select ARCH_HAS_MM_CPUMASK
 	select ARCH_HAS_PHYS_TO_DMA
 	select ARCH_HAS_PTE_SPECIAL
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index db09d45d59ec..1b196acdeca3 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -9,6 +9,7 @@ config MIPS
 	select ARCH_HAS_FORTIFY_SOURCE
 	select ARCH_HAS_KCOV
 	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE if !EVA
+	select ARCH_HAS_MM_CPUMASK
 	select ARCH_HAS_PTE_SPECIAL if !(32BIT && CPU_HAS_RIXI)
 	select ARCH_HAS_STRNCPY_FROM_USER
 	select ARCH_HAS_STRNLEN_USER
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index e814df4c483c..82483b192f4a 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -9,6 +9,7 @@ config OPENRISC
 	select ARCH_32BIT_OFF_T
 	select ARCH_HAS_DMA_SET_UNCACHED
 	select ARCH_HAS_DMA_CLEAR_UNCACHED
+	select ARCH_HAS_MM_CPUMASK
 	select ARCH_HAS_SYNC_DMA_FOR_DEVICE
 	select COMMON_CLK
 	select OF
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c2ce2e60c8f0..19061ffe73a0 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -127,6 +127,7 @@ config PPC
 	select ARCH_HAS_MEMBARRIER_SYNC_CORE
 	select ARCH_HAS_MEMREMAP_COMPAT_ALIGN	if PPC_64S_HASH_MMU
 	select ARCH_HAS_MMIOWB			if PPC64
+	select ARCH_HAS_MM_CPUMASK
 	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
 	select ARCH_HAS_PHYS_TO_DMA
 	select ARCH_HAS_PMEM_API
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index c22f58155948..7570c95a9cc8 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -25,6 +25,7 @@ config RISCV
 	select ARCH_HAS_GIGANTIC_PAGE
 	select ARCH_HAS_KCOV
 	select ARCH_HAS_MMIOWB
+	select ARCH_HAS_MM_CPUMASK
 	select ARCH_HAS_PTE_SPECIAL
 	select ARCH_HAS_SET_DIRECT_MAP if MMU
 	select ARCH_HAS_SET_MEMORY if MMU
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 91c0b80a8bf0..48d91fa05bab 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -73,6 +73,7 @@ config S390
 	select ARCH_HAS_GIGANTIC_PAGE
 	select ARCH_HAS_KCOV
 	select ARCH_HAS_MEM_ENCRYPT
+	select ARCH_HAS_MM_CPUMASK
 	select ARCH_HAS_PTE_SPECIAL
 	select ARCH_HAS_SCALED_CPUTIME
 	select ARCH_HAS_SET_MEMORY
diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index 4ec22e156a2e..df29c729267b 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -8,6 +8,7 @@ config UML
 	select ARCH_EPHEMERAL_INODES
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_HAS_KCOV
+	select ARCH_HAS_MM_CPUMASK
 	select ARCH_HAS_STRNCPY_FROM_USER
 	select ARCH_HAS_STRNLEN_USER
 	select ARCH_NO_PREEMPT
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index be0b95e51df6..a91d73866238 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -81,6 +81,7 @@ config X86
 	select ARCH_HAS_KCOV			if X86_64
 	select ARCH_HAS_MEM_ENCRYPT
 	select ARCH_HAS_MEMBARRIER_SYNC_CORE
+	select ARCH_HAS_MM_CPUMASK
 	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
 	select ARCH_HAS_PMEM_API		if X86_64
 	select ARCH_HAS_PTE_DEVMAP		if X86_64
diff --git a/mm/Kconfig b/mm/Kconfig
index 169e64192e48..7bf54f57ca01 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -951,6 +951,9 @@ config ARCH_HAS_CURRENT_STACK_POINTER
 	  register alias named "current_stack_pointer", this config can be
 	  selected.

+config ARCH_HAS_MM_CPUMASK
+	bool
+
 config ARCH_HAS_VM_GET_PAGE_PROT
 	bool

diff --git a/mm/rmap.c b/mm/rmap.c
index 5bcb334cd6f2..13d4f9a1d4f1 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -692,6 +692,10 @@ static bool should_defer_flush(struct mm_struct *mm, enum ttu_flags flags)
 	if (!(flags & TTU_BATCH_FLUSH))
 		return false;

+#ifndef CONFIG_ARCH_HAS_MM_CPUMASK
+	return true;
+#endif
+
 	/* If remote CPUs need to be flushed then defer batch the flush */
 	if (cpumask_any_but(mm_cpumask(mm), get_cpu()) < nr_cpu_ids)
 		should_defer = true;

Thanks
Barry
diff mbox series

Patch

diff --git a/mm/rmap.c b/mm/rmap.c
index 5bcb334cd6f2..d320c29a4ad8 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -692,8 +692,13 @@  static bool should_defer_flush(struct mm_struct *mm, enum ttu_flags flags)
 	if (!(flags & TTU_BATCH_FLUSH))
 		return false;
 
-	/* If remote CPUs need to be flushed then defer batch the flush */
-	if (cpumask_any_but(mm_cpumask(mm), get_cpu()) < nr_cpu_ids)
+	/*
+	 * If remote CPUs need to be flushed then defer batch the flush;
+	 * If ARCHs like ARM64 have hardware TLB flush broadcast, thus
+	 * they don't maintain mm_cpumask() at all, defer batch as well.
+	 */
+	if (cpumask_any_but(mm_cpumask(mm), get_cpu()) < nr_cpu_ids ||
+	    cpumask_empty(mm_cpumask(mm)))
 		should_defer = true;
 	put_cpu();