From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-133.freemail.mail.aliyun.com (out30-133.freemail.mail.aliyun.com [115.124.30.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4F5C94A21 for ; Thu, 16 Oct 2025 01:35:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.133 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1760578518; cv=none; b=XsbULWS0jYIkAlzYVBHzEewZCPZidD4khPJLmea2H16VJjBEhg76bGFagubUewS9LBjYTpwKwmaSDsUGFPVDmRZ6F8p0oz22Tj4N5Mrq1V9Ew8QIiCU+7usYqXTv7H68On6hkDYOFtGnintBGRzRsaLQKEOvpJi1jmeE5C9VWVE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1760578518; c=relaxed/simple; bh=kN9cQ3wsBmwwepG+sAqfoNT6bKbxQmJJ3Xxi9L9N3YI=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=O6QDPI8NpsYzbC4c/R52fYIUnpFYOKU+EA1v7p/PZVwi5IRTdKtKcHGG3WcSl4QhBO/p7RozhZNqdx7/9ERE2goPhTDeXLvk8gHdPj9D8Ts9jsN/BC8J5N3os3oQ8dd4wWX74FuyiceCNTVBa+VaiIWcDoB3lyPUmkxnwAokf3s= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=OZIfaWuA; arc=none smtp.client-ip=115.124.30.133 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="OZIfaWuA" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1760578513; h=From:To:Subject:Date:Message-ID:MIME-Version:Content-Type; bh=23ZCF8KBb3dclACfCrat83b8Fd2iW62x98ewCs5uUOg=; b=OZIfaWuAv0nEawCK0Ty0XG+y1dXFgf+NlYu+559UdHLzklGB99yJIQj/rE1AM3M7W/848Nw+g39Xrts9yZ/ziX5U78dLwQwy5E+NConKz8+Ilbc2GfMjhQLszyUh30e92UXg9EfPggcPzSRBQDIyCRVGjwEWb2wd2SB4IPmynC0= Received: from DESKTOP-5N7EMDA(mailfrom:ying.huang@linux.alibaba.com fp:SMTPD_---0WqIOhL1_1760578511 cluster:ay36) by smtp.aliyun-inc.com; Thu, 16 Oct 2025 09:35:12 +0800 From: "Huang, Ying" To: Ryan Roberts Cc: Catalin Marinas , Will Deacon , Andrew Morton , David Hildenbrand , Lorenzo Stoakes , Vlastimil Babka , Zi Yan , Baolin Wang , Yang Shi , "Christoph Lameter (Ampere)" , Dev Jain , Barry Song , Anshuman Khandual , Yicong Yang , Kefeng Wang , Kevin Brodsky , Yin Fengwei , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH -v2 2/2] arm64, tlbflush: don't TLBI broadcast if page reused in write fault In-Reply-To: <9afcdd88-f8f9-4d2f-94d7-7c41b0a25ddf@arm.com> (Ryan Roberts's message of "Wed, 15 Oct 2025 16:28:31 +0100") References: <20251013092038.6963-1-ying.huang@linux.alibaba.com> <20251013092038.6963-3-ying.huang@linux.alibaba.com> <9afcdd88-f8f9-4d2f-94d7-7c41b0a25ddf@arm.com> Date: Thu, 16 Oct 2025 09:35:11 +0800 Message-ID: <877bwvmxww.fsf@DESKTOP-5N7EMDA> User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=ascii Hi, Ryan, Thanks for comments! Ryan Roberts writes: > On 13/10/2025 10:20, 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 stall > > nit: You keep using the word "stall" here and in the code. I think you mean "stale"? OOPS, my poor English :-( Yes, it should be "stale". Thanks for pointing this out, will fix it in the future versions. >> 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 stall 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 stall 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 (merged). 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. >> >> Signed-off-by: Huang Ying >> Cc: Catalin Marinas >> Cc: Will Deacon >> Cc: Andrew Morton >> Cc: David Hildenbrand >> Cc: Lorenzo Stoakes >> Cc: Vlastimil Babka >> Cc: Zi Yan >> Cc: Baolin Wang >> Cc: Ryan Roberts >> Cc: Yang Shi >> Cc: "Christoph Lameter (Ampere)" >> Cc: Dev Jain >> Cc: Barry Song >> Cc: Anshuman Khandual >> Cc: Yicong Yang >> Cc: Kefeng Wang >> Cc: Kevin Brodsky >> Cc: Yin Fengwei >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Cc: linux-mm@kvack.org >> --- >> arch/arm64/include/asm/pgtable.h | 14 +++++--- >> arch/arm64/include/asm/tlbflush.h | 56 +++++++++++++++++++++++++++++++ >> arch/arm64/mm/contpte.c | 3 +- >> arch/arm64/mm/fault.c | 2 +- >> 4 files changed, 67 insertions(+), 8 deletions(-) >> >> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h >> index aa89c2e67ebc..35bae2e4bcfe 100644 >> --- a/arch/arm64/include/asm/pgtable.h >> +++ b/arch/arm64/include/asm/pgtable.h >> @@ -130,12 +130,16 @@ static inline void arch_leave_lazy_mmu_mode(void) >> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ >> >> /* >> - * Outside of a few very special situations (e.g. hibernation), we always >> - * use broadcast TLB invalidation instructions, therefore a spurious page >> - * fault on one CPU which has been handled concurrently by another CPU >> - * does not need to perform additional invalidation. >> + * We use local TLB invalidation instruction when reusing page in >> + * write protection fault handler to avoid TLBI broadcast in the hot >> + * path. This will cause spurious page faults if stall read-only TLB >> + * entries exist. >> */ >> -#define flush_tlb_fix_spurious_fault(vma, address, ptep) do { } while (0) >> +#define flush_tlb_fix_spurious_fault(vma, address, ptep) \ >> + local_flush_tlb_page_nonotify(vma, address) >> + >> +#define flush_tlb_fix_spurious_fault_pmd(vma, address, pmdp) \ >> + local_flush_tlb_page_nonotify(vma, address) >> >> /* >> * ZERO_PAGE is a global shared page that is always zero: used >> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h >> index 18a5dc0c9a54..651b31fd18bb 100644 >> --- a/arch/arm64/include/asm/tlbflush.h >> +++ b/arch/arm64/include/asm/tlbflush.h >> @@ -249,6 +249,18 @@ static inline unsigned long get_trans_granule(void) >> * cannot be easily determined, the value TLBI_TTL_UNKNOWN will >> * perform a non-hinted invalidation. >> * >> + * local_flush_tlb_page(vma, addr) >> + * Local variant of flush_tlb_page(). Stale TLB entries may >> + * remain in remote CPUs. >> + * >> + * local_flush_tlb_page_nonotify(vma, addr) >> + * Same as local_flush_tlb_page() except MMU notifier will not be >> + * called. >> + * >> + * local_flush_tlb_contpte_range(vma, start, end) >> + * Invalidate the virtual-address range '[start, end)' mapped with >> + * contpte on local CPU for the user address space corresponding >> + * to 'vma->mm'. Stale TLB entries may remain in remote CPUs. >> * >> * Finally, take a look at asm/tlb.h to see how tlb_flush() is implemented >> * on top of these routines, since that is our interface to the mmu_gather >> @@ -282,6 +294,33 @@ static inline void flush_tlb_mm(struct mm_struct *mm) >> mmu_notifier_arch_invalidate_secondary_tlbs(mm, 0, -1UL); >> } >> >> +static inline void __local_flush_tlb_page_nonotify_nosync( >> + struct mm_struct *mm, unsigned long uaddr) >> +{ >> + unsigned long addr; >> + >> + dsb(nshst); >> + addr = __TLBI_VADDR(uaddr, ASID(mm)); >> + __tlbi(vale1, addr); >> + __tlbi_user(vale1, addr); >> +} >> + >> +static inline void local_flush_tlb_page_nonotify( >> + struct vm_area_struct *vma, unsigned long uaddr) >> +{ >> + __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 +511,23 @@ static inline void __flush_tlb_range(struct vm_area_struct *vma, >> dsb(ish); >> } >> >> +static inline void local_flush_tlb_contpte_range(struct vm_area_struct *vma, >> + unsigned long start, unsigned long end) > > This would be clearer as an API if it was like this: > > static inline void local_flush_tlb_contpte(struct vm_area_struct *vma, > unsigned long uaddr) > > i.e. the user doesn't set the range - it's implicitly CONT_PTE_SIZE starting at > round_down(uaddr, PAGE_SIZE). Sure. Will do this. > Thanks, > Ryan > >> +{ >> + unsigned long asid, pages; >> + >> + start = round_down(start, PAGE_SIZE); >> + end = round_up(end, PAGE_SIZE); >> + pages = (end - start) >> PAGE_SHIFT; >> + >> + dsb(nshst); >> + asid = ASID(vma->vm_mm); >> + __flush_tlb_range_op(vale1, start, pages, PAGE_SIZE, asid, >> + 3, true, lpa2_is_enabled()); >> + mmu_notifier_arch_invalidate_secondary_tlbs(vma->vm_mm, start, end); >> + 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..0f9bbb7224dc 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_range(vma, start_addr, addr); >> } 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 */ >> if (dirty) >> - flush_tlb_page(vma, address); >> + local_flush_tlb_page(vma, address); >> return 1; >> } >> --- Best Regards, Huang, Ying