Re: [PATCH v2 0/4] KVM: selftests: add powerpc support
On Sat, 8 Apr 2023 at 04:01, Nicholas Piggin wrote: > > This series adds initial KVM selftests support for powerpc > (64-bit, BookS, radix MMU). This means the tests won't work on power8. Perhaps you could add something like this? --- a/tools/testing/selftests/kvm/lib/powerpc/processor.c +++ b/tools/testing/selftests/kvm/lib/powerpc/processor.c @@ -33,6 +33,8 @@ void virt_arch_pgd_alloc(struct kvm_vm *vm) vm_paddr_t prtb, pgtb; size_t pgd_pages; + TEST_REQUIRE(kvm_has_cap(KVM_CAP_PPC_MMU_RADIX)); + > > Since v1: > - Update MAINTAINERS KVM PPC entry to include kvm selftests. > - Fixes and cleanups from Sean's review including new patch 1. > - Add 4K guest page support requiring new patch 2. > > Thanks, > Nick > > Nicholas Piggin (4): > KVM: selftests: Move pgd_created check into virt_pgd_alloc > KVM: selftests: Add aligned guest physical page allocator > KVM: PPC: selftests: add support for powerpc > KVM: PPC: selftests: add selftests sanity tests > > MAINTAINERS | 2 + > tools/testing/selftests/kvm/Makefile | 15 + > .../selftests/kvm/include/kvm_util_base.h | 27 ++ > .../selftests/kvm/include/powerpc/hcall.h | 21 + > .../selftests/kvm/include/powerpc/ppc_asm.h | 32 ++ > .../selftests/kvm/include/powerpc/processor.h | 33 ++ > .../selftests/kvm/lib/aarch64/processor.c | 4 - > tools/testing/selftests/kvm/lib/guest_modes.c | 3 + > tools/testing/selftests/kvm/lib/kvm_util.c| 56 ++- > .../selftests/kvm/lib/powerpc/handlers.S | 93 > .../testing/selftests/kvm/lib/powerpc/hcall.c | 45 ++ > .../selftests/kvm/lib/powerpc/processor.c | 429 ++ > .../testing/selftests/kvm/lib/powerpc/ucall.c | 30 ++ > .../selftests/kvm/lib/riscv/processor.c | 4 - > .../selftests/kvm/lib/s390x/processor.c | 4 - > .../selftests/kvm/lib/x86_64/processor.c | 7 +- > tools/testing/selftests/kvm/powerpc/helpers.h | 46 ++ > .../testing/selftests/kvm/powerpc/null_test.c | 166 +++ > .../selftests/kvm/powerpc/rtas_hcall.c| 146 ++ > 19 files changed, 1129 insertions(+), 34 deletions(-) > create mode 100644 tools/testing/selftests/kvm/include/powerpc/hcall.h > create mode 100644 tools/testing/selftests/kvm/include/powerpc/ppc_asm.h > create mode 100644 tools/testing/selftests/kvm/include/powerpc/processor.h > create mode 100644 tools/testing/selftests/kvm/lib/powerpc/handlers.S > create mode 100644 tools/testing/selftests/kvm/lib/powerpc/hcall.c > create mode 100644 tools/testing/selftests/kvm/lib/powerpc/processor.c > create mode 100644 tools/testing/selftests/kvm/lib/powerpc/ucall.c > create mode 100644 tools/testing/selftests/kvm/powerpc/helpers.h > create mode 100644 tools/testing/selftests/kvm/powerpc/null_test.c > create mode 100644 tools/testing/selftests/kvm/powerpc/rtas_hcall.c > > -- > 2.40.0 >
Re: [PATCH] KVM: PPC: BOOK3S: book3s_hv_nested.c: improve branch prediction for k.alloc
On 2023-04-07 09:01:29, Sean Christopherson wrote: > On Fri, Apr 07, 2023, Bagas Sanjaya wrote: > > On Fri, Apr 07, 2023 at 05:31:47AM -0400, Kautuk Consul wrote: > > > I used the unlikely() macro on the return values of the k.alloc > > > calls and found that it changes the code generation a bit. > > > Optimize all return paths of k.alloc calls by improving > > > branch prediction on return value of k.alloc. > > Nit, this is improving code generation, not branch prediction. Sorry my mistake. > > > What about below? > > > > "Improve branch prediction on kmalloc() and kzalloc() call by using > > unlikely() macro to optimize their return paths." > > Another nit, using unlikely() doesn't necessarily provide a measurable > optimization. > As above, it does often improve code generation for the happy path, but that > doesn't > always equate to improved performance, e.g. if the CPU can easily predict the > branch > and/or there is no impact on the cache footprint. I see. I will submit a v2 of the patch with a better and more accurate description. Does anyone else have any comments before I do so ?
[PATCH v9 0/2] arm64: support batched/deferred tlb shootdown during page reclamation/migration
From: Yicong Yang Though ARM64 has the hardware to do tlb shootdown, the hardware broadcasting is not free. A simplest micro benchmark shows even on snapdragon 888 with only 8 cores, the overhead for ptep_clear_flush is huge even for paging out one page mapped by only one process: 5.36% a.out[kernel.kallsyms] [k] ptep_clear_flush While pages are mapped by multiple processes or HW has more CPUs, the cost should become even higher due to the bad scalability of tlb shootdown. The same benchmark can result in 16.99% CPU consumption on ARM64 server with around 100 cores according to Yicong's test on patch 2/2. This patchset leverages the existing BATCHED_UNMAP_TLB_FLUSH by 1. only send tlbi instructions in the first stage - arch_tlbbatch_add_mm() 2. wait for the completion of tlbi by dsb while doing tlbbatch sync in arch_tlbbatch_flush() Testing on snapdragon shows the overhead of ptep_clear_flush is removed by the patchset. The micro benchmark becomes 5% faster even for one page mapped by single process on snapdragon 888. This support also optimize the page migration more than 50% with support of batched TLB flushing [*]. [*] https://lore.kernel.org/linux-mm/20230213123444.155149-1-ying.hu...@intel.com/ -v9: 1. Using a runtime tunable to control batched TLB flush, per Catalin in v7. Sorry for missing this on v8. Link: https://lore.kernel.org/all/20230329035512.57392-1-yangyic...@huawei.com/ -v8: 1. Rebase on 6.3-rc4 2. Tested the optimization on page migration and mentioned it in the commit 3. Thanks the review from Anshuman. Link: https://lore.kernel.org/linux-mm/20221117082648.47526-1-yangyic...@huawei.com/ -v7: 1. rename arch_tlbbatch_add_mm() to arch_tlbbatch_add_pending() as suggested, since it takes an extra address for arm64, per Nadav and Anshuman. Also mentioned in the commit. 2. add tags from Xin Hao, thanks. Link: https://lore.kernel.org/lkml/20221115031425.44640-1-yangyic...@huawei.com/ -v6: 1. comment we don't defer TLB flush on platforms affected by ARM64_WORKAROUND_REPEAT_TLBI 2. use cpus_have_const_cap() instead of this_cpu_has_cap() 3. add tags from Punit, Thanks. 4. default enable the feature when cpus >= 8 rather than > 8, since the original improvement is observed on snapdragon 888 with 8 cores. Link: https://lore.kernel.org/lkml/20221028081255.19157-1-yangyic...@huawei.com/ -v5: 1. Make ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH depends on EXPERT for this stage on arm64. 2. Make a threshold of CPU numbers for enabling batched TLP flush on arm64 Link: https://lore.kernel.org/linux-arm-kernel/20220921084302.43631-1-yangyic...@huawei.com/T/ -v4: 1. Add tags from Kefeng and Anshuman, Thanks. 2. Limit the TLB batch/defer on systems with >4 CPUs, per Anshuman 3. Merge previous Patch 1,2-3 into one, per Anshuman Link: https://lore.kernel.org/linux-mm/20220822082120.8347-1-yangyic...@huawei.com/ -v3: 1. Declare arch's tlbbatch defer support by arch_tlbbatch_should_defer() instead of ARCH_HAS_MM_CPUMASK, per Barry and Kefeng 2. Add Tested-by from Xin Hao Link: https://lore.kernel.org/linux-mm/20220711034615.482895-1-21cn...@gmail.com/ -v2: 1. Collected Yicong's test result on kunpeng920 ARM64 server; 2. Removed the redundant vma parameter in arch_tlbbatch_add_mm() according to the comments of Peter Zijlstra and Dave Hansen 3. Added ARCH_HAS_MM_CPUMASK rather than checking if mm_cpumask is empty according to the comments of Nadav Amit Thanks, Peter, Dave and Nadav for your testing or reviewing , and comments. -v1: https://lore.kernel.org/lkml/20220707125242.425242-1-21cn...@gmail.com/ Anshuman Khandual (1): mm/tlbbatch: Introduce arch_tlbbatch_should_defer() Barry Song (1): arm64: support batched/deferred tlb shootdown during page reclamation/migration .../features/vm/TLB/arch-support.txt | 2 +- arch/arm64/Kconfig| 1 + arch/arm64/include/asm/tlbbatch.h | 12 arch/arm64/include/asm/tlbflush.h | 33 - arch/arm64/mm/flush.c | 69 +++ arch/x86/include/asm/tlbflush.h | 17 - include/linux/mm_types_task.h | 4 +- mm/rmap.c | 21 +++--- 8 files changed, 139 insertions(+), 20 deletions(-) create mode 100644 arch/arm64/include/asm/tlbbatch.h -- 2.24.0
[PATCH v9 2/2] arm64: support batched/deferred tlb shootdown during page reclamation/migration
From: Barry Song
on x86, batched and deferred tlb shootdown has lead to 90%
performance increase on tlb shootdown. on arm64, HW can do
tlb shootdown without software IPI. But sync tlbi is still
quite expensive.
Even running a simplest program which requires swapout can
prove this is true,
#include
#include
#include
#include
int main()
{
#define SIZE (1 * 1024 * 1024)
volatile unsigned char *p = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,
MAP_SHARED | MAP_ANONYMOUS, -1, 0);
memset(p, 0x88, SIZE);
for (int k = 0; k < 1; k++) {
/* swap in */
for (int i = 0; i < SIZE; i += 4096) {
(void)p[i];
}
/* swap out */
madvise(p, SIZE, MADV_PAGEOUT);
}
}
Perf result on snapdragon 888 with 8 cores by using zRAM
as the swap block device.
~ # perf record taskset -c 4 ./a.out
[ perf record: Woken up 10 times to write data ]
[ perf record: Captured and wrote 2.297 MB perf.data (60084 samples) ]
~ # perf report
# To display the perf.data header info, please use --header/--header-only
options.
# To display the perf.data header info, please use --header/--header-only
options.
#
#
# Total Lost Samples: 0
#
# Samples: 60K of event 'cycles'
# Event count (approx.): 35706225414
#
# Overhead Command Shared Object Symbol
# ... .
.
#
21.07% a.out[kernel.kallsyms] [k] _raw_spin_unlock_irq
8.23% a.out[kernel.kallsyms] [k] _raw_spin_unlock_irqrestore
6.67% a.out[kernel.kallsyms] [k] filemap_map_pages
6.16% a.out[kernel.kallsyms] [k] __zram_bvec_write
5.36% a.out[kernel.kallsyms] [k] ptep_clear_flush
3.71% a.out[kernel.kallsyms] [k] _raw_spin_lock
3.49% a.out[kernel.kallsyms] [k] memset64
1.63% a.out[kernel.kallsyms] [k] clear_page
1.42% a.out[kernel.kallsyms] [k] _raw_spin_unlock
1.26% a.out[kernel.kallsyms] [k]
mod_zone_state.llvm.8525150236079521930
1.23% a.out[kernel.kallsyms] [k] xas_load
1.15% a.out[kernel.kallsyms] [k] zram_slot_lock
ptep_clear_flush() takes 5.36% CPU in the micro-benchmark
swapping in/out a page mapped by only one process. If the
page is mapped by multiple processes, typically, like more
than 100 on a phone, the overhead would be much higher as
we have to run tlb flush 100 times for one single page.
Plus, tlb flush overhead will increase with the number
of CPU cores due to the bad scalability of tlb shootdown
in HW, so those ARM64 servers should expect much higher
overhead.
Further perf annonate shows 95% cpu time of ptep_clear_flush
is actually used by the final dsb() to wait for the completion
of tlb flush. This provides us a very good chance to leverage
the existing batched tlb in kernel. The minimum modification
is that we only send async tlbi in the first stage and we send
dsb while we have to sync in the second stage.
With the above simplest micro benchmark, collapsed time to
finish the program decreases around 5%.
Typical collapsed time w/o patch:
~ # time taskset -c 4 ./a.out
0.21user 14.34system 0:14.69elapsed
w/ patch:
~ # time taskset -c 4 ./a.out
0.22user 13.45system 0:13.80elapsed
Also, Yicong Yang added the following observation.
Tested with benchmark in the commit on Kunpeng920 arm64 server,
observed an improvement around 12.5% with command
`time ./swap_bench`.
w/o w/
real0m13.460s 0m11.771s
user0m0.248s0m0.279s
sys 0m12.039s 0m11.458s
Originally it's noticed a 16.99% overhead of ptep_clear_flush()
which has been eliminated by this patch:
[root@localhost yang]# perf record -- ./swap_bench && perf report
[...]
16.99% swap_bench [kernel.kallsyms] [k] ptep_clear_flush
It is tested on 4,8,128 CPU platforms and shows to be beneficial on
large systems but may not have improvement on small systems like on
a 4 CPU platform. So make ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH depends
on CONFIG_EXPERT for this stage and add a runtime tunable to allow
to disable it according to the scenario.
Also this patch improve the performance of page migration. Using pmbench
and tries to migrate the pages of pmbench between node 0 and node 1 for
20 times, this patch decrease the time used more than 50% and saved the
time used by ptep_clear_flush().
This patch extends arch_tlbbatch_add_mm() to take an address of the
target page to support the feature on arm64. Also rename it to
arch_tlbbatch_add_pending() to better match its function since we
don't need to handle the mm on arm64 and add_mm is not proper.
add_pending will make sense to both as on x86 we're pending the
TLB flush operations while on arm64
[PATCH v9 1/2] mm/tlbbatch: Introduce arch_tlbbatch_should_defer()
From: Anshuman Khandual
The entire scheme of deferred TLB flush in reclaim path rests on the
fact that the cost to refill TLB entries is less than flushing out
individual entries by sending IPI to remote CPUs. But architecture
can have different ways to evaluate that. Hence apart from checking
TTU_BATCH_FLUSH in the TTU flags, rest of the decision should be
architecture specific.
Signed-off-by: Anshuman Khandual
[https://lore.kernel.org/linuxppc-dev/20171101101735.2318-2-khand...@linux.vnet.ibm.com/]
Signed-off-by: Yicong Yang
[Rebase and fix incorrect return value type]
Reviewed-by: Kefeng Wang
Reviewed-by: Anshuman Khandual
Reviewed-by: Barry Song
Reviewed-by: Xin Hao
Tested-by: Punit Agrawal
---
arch/x86/include/asm/tlbflush.h | 12
mm/rmap.c | 9 +
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index cda3118f3b27..8a497d902c16 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -240,6 +240,18 @@ static inline void flush_tlb_page(struct vm_area_struct
*vma, unsigned long a)
flush_tlb_mm_range(vma->vm_mm, a, a + PAGE_SIZE, PAGE_SHIFT, false);
}
+static inline bool arch_tlbbatch_should_defer(struct mm_struct *mm)
+{
+ bool should_defer = 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)
+ should_defer = true;
+ put_cpu();
+
+ return should_defer;
+}
+
static inline u64 inc_mm_tlb_gen(struct mm_struct *mm)
{
/*
diff --git a/mm/rmap.c b/mm/rmap.c
index 8632e02661ac..38ccb700748c 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -686,17 +686,10 @@ static void set_tlb_ubc_flush_pending(struct mm_struct
*mm, bool writable)
*/
static bool should_defer_flush(struct mm_struct *mm, enum ttu_flags flags)
{
- bool should_defer = false;
-
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)
- should_defer = true;
- put_cpu();
-
- return should_defer;
+ return arch_tlbbatch_should_defer(mm);
}
/*
--
2.24.0
Re: [PATCH] powerpc/32: Include thread_info.h in head_booke.h
On Fri, Apr 7, 2023 at 2:51 AM Nathan Chancellor wrote:
>
> When building with W=1 after commit 80b6093b55e3 ("kbuild: add -Wundef
> to KBUILD_CPPFLAGS for W=1 builds"), the following warning occurs.
>
> In file included from arch/powerpc/kvm/bookehv_interrupts.S:26:
> arch/powerpc/kvm/../kernel/head_booke.h:20:6: warning: "THREAD_SHIFT" is
> not defined, evaluates to 0 [-Wundef]
> 20 | #if (THREAD_SHIFT < 15)
> | ^~~~
>
> THREAD_SHIFT is defined in thread_info.h but it is not directly included
> in head_booke.h, so it is possible for THREAD_SHIFT to be undefined. Add
> the include to ensure that THREAD_SHIFT is always defined.
>
> Reported-by: kernel test robot
> Link: https://lore.kernel.org/202304050954.yskldczh-...@intel.com/
> Signed-off-by: Nathan Chancellor
> ---
Reviewed-by: Masahiro Yamada
Thanks.
> arch/powerpc/kernel/head_booke.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kernel/head_booke.h
> b/arch/powerpc/kernel/head_booke.h
> index 37d43c172676..b6b5b01a173c 100644
> --- a/arch/powerpc/kernel/head_booke.h
> +++ b/arch/powerpc/kernel/head_booke.h
> @@ -5,6 +5,7 @@
> #include /* for STACK_FRAME_REGS_MARKER */
> #include
> #include
> +#include/* for THREAD_SHIFT */
>
> #ifdef __ASSEMBLY__
>
>
> ---
> base-commit: b0bbe5a2915201e3231e788d716d39dc54493b03
> change-id: 20230406-wundef-thread_shift_booke-e08d806ed656
>
> Best regards,
> --
> Nathan Chancellor
>
--
Best Regards
Masahiro Yamada
