lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8e603deb-7023-5de5-c958-8911971aec24@huawei.com>
Date:   Mon, 18 Jul 2022 21:28:30 +0800
From:   Yicong Yang <yangyicong@...wei.com>
To:     Barry Song <21cnbao@...il.com>, <xhao@...ux.alibaba.com>
CC:     Andrew Morton <akpm@...ux-foundation.org>,
        Linux-MM <linux-mm@...ck.org>,
        LAK <linux-arm-kernel@...ts.infradead.org>, x86 <x86@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        Jonathan Corbet <corbet@....net>,
        Arnd Bergmann <arnd@...db.de>,
        LKML <linux-kernel@...r.kernel.org>,
        Darren Hart <darren@...amperecomputing.com>,
        <huzhanyuan@...o.com>,
        李培锋(wink) <lipeifeng@...o.com>,
        张诗明(Simon Zhang) 
        <zhangshiming@...o.com>, 郭健 <guojian@...o.com>,
        real mz <realmz6@...il.com>, <linux-mips@...r.kernel.org>,
        <openrisc@...ts.librecores.org>, <linuxppc-dev@...ts.ozlabs.org>,
        <linux-riscv@...ts.infradead.org>, <linux-s390@...r.kernel.org>,
        <yangyicong@...ilicon.com>, "tiantao (H)" <tiantao6@...ilicon.com>
Subject: Re: [PATCH v2 0/4] mm: arm64: bring up BATCHED_UNMAP_TLB_FLUSH

On 2022/7/14 12:51, Barry Song wrote:
> On Thu, Jul 14, 2022 at 3:29 PM Xin Hao <xhao@...ux.alibaba.com> wrote:
>>
>> Hi barry.
>>
>> I do some test on Kunpeng arm64 machine use Unixbench.
>>
>> The test  result as below.
>>
>> One core, we can see the performance improvement above +30%.
> 
> I am really pleased to see the 30%+ improvement on unixbench on single core.
> 
>> ./Run -c 1 -i 1 shell1
>> w/o
>> System Benchmarks Partial Index              BASELINE RESULT INDEX
>> Shell Scripts (1 concurrent)                     42.4 5481.0 1292.7
>> ========
>> System Benchmarks Index Score (Partial Only)                         1292.7
>>
>> w/
>> System Benchmarks Partial Index              BASELINE RESULT INDEX
>> Shell Scripts (1 concurrent)                     42.4 6974.6 1645.0
>> ========
>> System Benchmarks Index Score (Partial Only)                         1645.0
>>
>>
>> But with whole cores, there have little performance degradation above -5%
> 
> That is sad as we might get more concurrency between mprotect(), madvise(),
> mremap(), zap_pte_range() and the deferred tlbi.
> 
>>
>> ./Run -c 96 -i 1 shell1
>> w/o
>> Shell Scripts (1 concurrent)                  80765.5 lpm   (60.0 s, 1
>> samples)
>> System Benchmarks Partial Index              BASELINE RESULT INDEX
>> Shell Scripts (1 concurrent)                     42.4 80765.5 19048.5
>> ========
>> System Benchmarks Index Score (Partial Only)                        19048.5
>>
>> w
>> Shell Scripts (1 concurrent)                  76333.6 lpm   (60.0 s, 1
>> samples)
>> System Benchmarks Partial Index              BASELINE RESULT INDEX
>> Shell Scripts (1 concurrent)                     42.4 76333.6 18003.2
>> ========
>> System Benchmarks Index Score (Partial Only)                        18003.2
>>
>> ----------------------------------------------------------------------------------------------
>>
>>
>> After discuss with you, and do some changes in the patch.
>>
>> ndex a52381a680db..1ecba81f1277 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -727,7 +727,11 @@ void flush_tlb_batched_pending(struct mm_struct *mm)
>>          int flushed = batch >> TLB_FLUSH_BATCH_FLUSHED_SHIFT;
>>
>>          if (pending != flushed) {
>> +#ifdef CONFIG_ARCH_HAS_MM_CPUMASK
>>                  flush_tlb_mm(mm);
>> +#else
>> +               dsb(ish);
>> +#endif
>>
> 
> i was guessing the problem might be flush_tlb_batched_pending()
> so i asked you to change this to verify my guess.
> 

flush_tlb_batched_pending() looks like the critical path for this issue then the code
above can mitigate this.

I cannot reproduce this on a 2P 128C Kunpeng920 server. The kernel is based on the
v5.19-rc6 and unixbench of version 5.1.3. The result of `./Run -c 128 -i 1 shell1` is:
      iter-1      iter-2     iter-3
w/o  17708.1     17637.1    17630.1
w    17766.0     17752.3    17861.7

And flush_tlb_batched_pending()isn't the hot spot with the patch:
   7.00%  sh        [kernel.kallsyms]      [k] ptep_clear_flush
   4.17%  sh        [kernel.kallsyms]      [k] ptep_set_access_flags
   2.43%  multi.sh  [kernel.kallsyms]      [k] ptep_clear_flush
   1.98%  sh        [kernel.kallsyms]      [k] _raw_spin_unlock_irqrestore
   1.69%  sh        [kernel.kallsyms]      [k] next_uptodate_page
   1.66%  sort      [kernel.kallsyms]      [k] ptep_clear_flush
   1.56%  multi.sh  [kernel.kallsyms]      [k] ptep_set_access_flags
   1.27%  sh        [kernel.kallsyms]      [k] page_counter_cancel
   1.11%  sh        [kernel.kallsyms]      [k] page_remove_rmap
   1.06%  sh        [kernel.kallsyms]      [k] perf_event_alloc

Hi Xin Hao,

I'm not sure the test setup as well as the config is same with yours. (96C vs 128C
should not be the reason I think). Did you check that the 5% is a fluctuation or
not? It'll be helpful if more information provided for reproducing this issue.

Thanks.

>      /*
>>                   * If the new TLB flushing is pending during flushing, leave
>>                   * mm->tlb_flush_batched as is, to avoid losing flushing.
>>
>> there have a performance improvement with whole cores, above +30%
> 
> But I don't think it is a proper patch. There is no guarantee the cpu calling
> flush_tlb_batched_pending is exactly the cpu sending the deferred
> tlbi. so the solution is unsafe. But since this temporary code can bring the
> 30%+ performance improvement back for high concurrency, we have huge
> potential to finally make it.
> 
> Unfortunately I don't have an arm64 server to debug on this. I only have
> 8 cores which are unlikely to reproduce regression which happens in
> high concurrency with 96 parallel tasks.
> 
> So I'd ask if @yicong or someone else working on kunpeng or other
> arm64 servers  is able to actually debug and figure out a proper
> patch for this, then add the patch as 5/5 into this series?
> 
>>
>> ./Run -c 96 -i 1 shell1
>> 96 CPUs in system; running 96 parallel copies of tests
>>
>> Shell Scripts (1 concurrent)                 109229.0 lpm   (60.0 s, 1 samples)
>> System Benchmarks Partial Index              BASELINE       RESULT    INDEX
>> Shell Scripts (1 concurrent)                     42.4     109229.0  25761.6
>>                                                                     ========
>> System Benchmarks Index Score (Partial Only)                        25761.6
>>
>>
>> Tested-by: Xin Hao<xhao@...ux.alibaba.com>
> 
> Thanks for your testing!
> 
>>
>> Looking forward to your next version patch.
>>
>> On 7/11/22 11:46 AM, Barry Song wrote:
>>> 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
>>> 4/4.
>>>
>>> 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()
>>> My 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.
>>>
>>>
>>> -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, Yicong, Peter, Dave and Nadav for your testing or reviewing
>>> , and comments.
>>>
>>> -v1:
>>> https://lore.kernel.org/lkml/20220707125242.425242-1-21cnbao@gmail.com/
>>>
>>> Barry Song (4):
>>>    Revert "Documentation/features: mark BATCHED_UNMAP_TLB_FLUSH doesn't
>>>      apply to ARM64"
>>>    mm: rmap: Allow platforms without mm_cpumask to defer TLB flush
>>>    mm: rmap: Extend tlbbatch APIs to fit new platforms
>>>    arm64: support batched/deferred tlb shootdown during page reclamation
>>>
>>>   Documentation/features/arch-support.txt       |  1 -
>>>   .../features/vm/TLB/arch-support.txt          |  2 +-
>>>   arch/arm/Kconfig                              |  1 +
>>>   arch/arm64/Kconfig                            |  1 +
>>>   arch/arm64/include/asm/tlbbatch.h             | 12 ++++++++++
>>>   arch/arm64/include/asm/tlbflush.h             | 23 +++++++++++++++++--
>>>   arch/loongarch/Kconfig                        |  1 +
>>>   arch/mips/Kconfig                             |  1 +
>>>   arch/openrisc/Kconfig                         |  1 +
>>>   arch/powerpc/Kconfig                          |  1 +
>>>   arch/riscv/Kconfig                            |  1 +
>>>   arch/s390/Kconfig                             |  1 +
>>>   arch/um/Kconfig                               |  1 +
>>>   arch/x86/Kconfig                              |  1 +
>>>   arch/x86/include/asm/tlbflush.h               |  3 ++-
>>>   mm/Kconfig                                    |  3 +++
>>>   mm/rmap.c                                     | 14 +++++++----
>>>   17 files changed, 59 insertions(+), 9 deletions(-)
>>>   create mode 100644 arch/arm64/include/asm/tlbbatch.h
>>>
>> --
>> Best Regards!
>> Xin Hao
>>
> 
> Thanks
> Barry
> .
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ