| 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
| ||
|
Message-ID: <87qzu97zyi.fsf@DESKTOP-5N7EMDA>
Date: Sat, 08 Nov 2025 15:20:21 +0800
From: "Huang, Ying" <ying.huang@...ux.alibaba.com>
To: "David Hildenbrand (Red Hat)" <davidhildenbrandkernel@...il.com>
Cc: Catalin Marinas <catalin.marinas@....com>, Will Deacon
<will@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>, Ryan
Roberts <ryan.roberts@....com>, Barry Song <baohua@...nel.org>, Lorenzo
Stoakes <lorenzo.stoakes@...cle.com>, Vlastimil Babka <vbabka@...e.cz>,
Zi Yan <ziy@...dia.com>, Baolin Wang <baolin.wang@...ux.alibaba.com>,
Yang Shi <yang@...amperecomputing.com>, "Christoph Lameter (Ampere)"
<cl@...two.org>, Dev Jain <dev.jain@....com>, Anshuman Khandual
<anshuman.khandual@....com>, Kefeng Wang <wangkefeng.wang@...wei.com>,
Kevin Brodsky <kevin.brodsky@....com>, Yin Fengwei
<fengwei_yin@...ux.alibaba.com>, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH -v4 2/2] arm64, tlbflush: don't TLBI broadcast if page
reused in write fault
Hi, David,
"David Hildenbrand (Red Hat)" <davidhildenbrandkernel@...il.com> writes:
> On 04.11.25 10:55, Huang Ying wrote:
>> A multi-thread customer workload with large memory footprint uses
>> fork()/exec() to run some external programs every tens seconds. When
>> running the workload on an arm64 server machine, it's observed that
>> quite some CPU cycles are spent in the TLB flushing functions. While
>> running the workload on the x86_64 server machine, it's not. This
>> causes the performance on arm64 to be much worse than that on x86_64.
>> During the workload running, after fork()/exec() write-protects all
>> pages in the parent process, memory writing in the parent process
>> will cause a write protection fault. Then the page fault handler
>> will make the PTE/PDE writable if the page can be reused, which is
>> almost always true in the workload. On arm64, to avoid the write
>> protection fault on other CPUs, the page fault handler flushes the TLB
>> globally with TLBI broadcast after changing the PTE/PDE. However, this
>> isn't always necessary. Firstly, it's safe to leave some stale
>> read-only TLB entries as long as they will be flushed finally.
>> Secondly, it's quite possible that the original read-only PTE/PDEs
>> aren't cached in remote TLB at all if the memory footprint is large.
>> In fact, on x86_64, the page fault handler doesn't flush the remote
>> TLB in this situation, which benefits the performance a lot.
>> To improve the performance on arm64, make the write protection fault
>> handler flush the TLB locally instead of globally via TLBI broadcast
>> after making the PTE/PDE writable. If there are stale read-only TLB
>> entries in the remote CPUs, the page fault handler on these CPUs will
>> regard the page fault as spurious and flush the stale TLB entries.
>> To test the patchset, make the usemem.c from
>> vm-scalability (https://git.kernel.org/pub/scm/linux/kernel/git/wfg/vm-scalability.git).
>> support calling fork()/exec() periodically. To mimic the behavior of
>> the customer workload, run usemem with 4 threads, access 100GB memory,
>> and call fork()/exec() every 40 seconds. Test results show that with
>> the patchset the score of usemem improves ~40.6%. The cycles% of TLB
>> flush functions reduces from ~50.5% to ~0.3% in perf profile.
>>
>
> All makes sense to me.
>
> Some smaller comments below.
Thanks!
> [...]
>
>> +
>> +static inline void local_flush_tlb_page_nonotify(
>> + struct vm_area_struct *vma, unsigned long uaddr)
>
> NIT: "struct vm_area_struct *vma" fits onto the previous line.
Sure.
>> +{
>> + __local_flush_tlb_page_nonotify_nosync(vma->vm_mm, uaddr);
>> + dsb(nsh);
>> +}
>> +
>> +static inline void local_flush_tlb_page(struct vm_area_struct *vma,
>> + unsigned long uaddr)
>> +{
>> + __local_flush_tlb_page_nonotify_nosync(vma->vm_mm, uaddr);
>> + mmu_notifier_arch_invalidate_secondary_tlbs(vma->vm_mm, uaddr & PAGE_MASK,
>> + (uaddr & PAGE_MASK) + PAGE_SIZE);
>> + dsb(nsh);
>> +}
>> +
>> static inline void __flush_tlb_page_nosync(struct mm_struct *mm,
>> unsigned long uaddr)
>> {
>> @@ -472,6 +512,22 @@ static inline void __flush_tlb_range(struct vm_area_struct *vma,
>> dsb(ish);
>> }
>> +static inline void local_flush_tlb_contpte(struct vm_area_struct
>> *vma,
>> + unsigned long addr)
>> +{
>> + unsigned long asid;
>> +
>> + addr = round_down(addr, CONT_PTE_SIZE);
>> +
>> + dsb(nshst);
>> + asid = ASID(vma->vm_mm);
>> + __flush_tlb_range_op(vale1, addr, CONT_PTES, PAGE_SIZE, asid,
>> + 3, true, lpa2_is_enabled());
>> + mmu_notifier_arch_invalidate_secondary_tlbs(vma->vm_mm, addr,
>> + addr + CONT_PTE_SIZE);
>> + dsb(nsh);
>> +}
>> +
>> static inline void flush_tlb_range(struct vm_area_struct *vma,
>> unsigned long start, unsigned long end)
>> {
>> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
>> index c0557945939c..589bcf878938 100644
>> --- a/arch/arm64/mm/contpte.c
>> +++ b/arch/arm64/mm/contpte.c
>> @@ -622,8 +622,7 @@ int contpte_ptep_set_access_flags(struct vm_area_struct *vma,
>> __ptep_set_access_flags(vma, addr, ptep, entry, 0);
>> if (dirty)
>> - __flush_tlb_range(vma, start_addr, addr,
>> - PAGE_SIZE, true, 3);
>> + local_flush_tlb_contpte(vma, start_addr);
>
> In this case, we now flush a bigger range than we used to, no?
>
> Probably I am missing something (should this change be explained in
> more detail in the cover letter), but I'm wondering why this contpte
> handling wasn't required before on this level.
As Ryan explained in his replay email. The flush range doesn't change
here. We just replace global TLB flush with local TLB flush.
>> } else {
>> __contpte_try_unfold(vma->vm_mm, addr, ptep, orig_pte);
>> __ptep_set_access_flags(vma, addr, ptep, entry, dirty);
>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>> index d816ff44faff..22f54f5afe3f 100644
>> --- a/arch/arm64/mm/fault.c
>> +++ b/arch/arm64/mm/fault.c
>> @@ -235,7 +235,7 @@ int __ptep_set_access_flags(struct vm_area_struct *vma,
>> /* Invalidate a stale read-only entry */
>
> I would expand this comment to also explain how remote TLBs are
> handled very briefly -> flush_tlb_fix_spurious_fault().
Sure.
>> if (dirty)
>> - flush_tlb_page(vma, address);
>> + local_flush_tlb_page(vma, address);
>> return 1;
>> }
>>
---
Best Regards,
Huang, Ying
Powered by blists - more mailing lists