From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 038E9C433DF for ; Tue, 28 Jul 2020 11:46:27 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A47C020672 for ; Tue, 28 Jul 2020 11:46:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A47C020672 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 13E296B0025; Tue, 28 Jul 2020 07:46:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0EF166B0026; Tue, 28 Jul 2020 07:46:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 006B88D0003; Tue, 28 Jul 2020 07:46:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0059.hostedemail.com [216.40.44.59]) by kanga.kvack.org (Postfix) with ESMTP id DF3FE6B0025 for ; Tue, 28 Jul 2020 07:46:25 -0400 (EDT) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 761101EE6 for ; Tue, 28 Jul 2020 11:46:25 +0000 (UTC) X-FDA: 77087306730.12.coach73_3c1748926f69 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin12.hostedemail.com (Postfix) with ESMTP id 45618180559F1 for ; Tue, 28 Jul 2020 11:46:25 +0000 (UTC) X-HE-Tag: coach73_3c1748926f69 X-Filterd-Recvd-Size: 6313 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf15.hostedemail.com (Postfix) with ESMTP for ; Tue, 28 Jul 2020 11:46:24 +0000 (UTC) Received: from gaia (unknown [95.146.230.158]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B93532065C; Tue, 28 Jul 2020 11:46:21 +0000 (UTC) Date: Tue, 28 Jul 2020 12:46:19 +0100 From: Catalin Marinas To: Yu Xu Cc: Will Deacon , Linus Torvalds , Yang Shi , Andrew Morton , Johannes Weiner , Hillf Danton , Hugh Dickins , Josef Bacik , "Kirill A . Shutemov" , Linux-MM , mm-commits@vger.kernel.org, Matthew Wilcox Subject: Re: [patch 01/15] mm/memory.c: avoid access flag update TLB flush for retried page fault Message-ID: <20200728114619.GE706@gaia> References: <20200723211432.b31831a0df3bc2cbdae31b40@linux-foundation.org> <20200724041508.QlTbrHnfh%akpm@linux-foundation.org> <7de20d4a-f86c-8e1f-b238-65f02b560325@linux.alibaba.com> <20200725155841.GA14490@gaia> <20200728092220.GA21800@willie-the-truck> <20200728093910.GB706@gaia> <1f04a55e-ebea-beb2-a2d8-7bddbd296ba8@linux.alibaba.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1f04a55e-ebea-beb2-a2d8-7bddbd296ba8@linux.alibaba.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-Rspamd-Queue-Id: 45618180559F1 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam04 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Jul 28, 2020 at 06:07:44PM +0800, Yu Xu wrote: > On 7/28/20 5:39 PM, Catalin Marinas wrote: > > On Tue, Jul 28, 2020 at 10:22:20AM +0100, Will Deacon wrote: > > > On Sat, Jul 25, 2020 at 04:58:43PM +0100, Catalin Marinas wrote: > > > > On Fri, Jul 24, 2020 at 06:29:43PM -0700, Linus Torvalds wrote: > > > > > For any architecture that guarantees that a page fault will always > > > > > flush the old TLB entry for this kind of situation, that > > > > > flush_tlb_fix_spurious_fault() thing can be a no-op. > > > > > > > > > > So that's why on x86, we just do > > > > > > > > > > #define flush_tlb_fix_spurious_fault(vma, address) do { } while (0) > > > > > > > > > > and have no issues. > > > > > > > > > > Note that it does *not* need to do any cross-CPU flushing or anything > > > > > like that. So it's actually wrong (I think) to have that default > > > > > fallback for > > > > > > > > > > #define flush_tlb_fix_spurious_fault(vma, address) > > > > > flush_tlb_page(vma, address) > > > > > > > > > > because flush_tlb_page() is the serious "do cross CPU etc". > > > > > > > > > > Does the arm64 flush_tlb_page() perhaps do the whole expensive > > > > > cross-CPU thing rather than the much cheaper "just local invalidate" > > > > > version? > > > > > > > > I think it makes sense to have a local-only > > > > flush_tlb_fix_spurious_fault(), but with ptep_set_access_flags() updated > > > > to still issue the full broadcast TLBI. In addition, I added a minor > > > > optimisation to avoid the TLB flush if the old pte was not accessible. > > > > In a read-access fault case (followed by mkyoung), the TLB wouldn't have > > > > cached a non-accessible pte (not sure it makes much difference to Yang's > > > > case). Anyway, from ARMv8.1 onwards, the hardware handles the access > > > > flag automatically. > > > > > > > > I'm not sure the first dsb(nshst) below is of much use in this case. If > > > > we got a spurious fault, the write to the pte happened on a different > > > > CPU (IIUC, we shouldn't return to user with updated ptes without a TLB > > > > flush on the same CPU). Anyway, we can refine this if it solves Yang's > > > > performance regression. > > > > > > > > -------------8<----------------------- > > > > diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h > > > > index d493174415db..d1401cbad7d4 100644 > > > > --- a/arch/arm64/include/asm/tlbflush.h > > > > +++ b/arch/arm64/include/asm/tlbflush.h > > > > @@ -268,6 +268,20 @@ static inline void flush_tlb_page(struct vm_area_struct *vma, > > > > dsb(ish); > > > > } > > > > +static inline void local_flush_tlb_page(struct vm_area_struct *vma, > > > > + unsigned long uaddr) > > > > +{ > > > > + unsigned long addr = __TLBI_VADDR(uaddr, ASID(vma->vm_mm)); > > > > + > > > > + dsb(nshst); > > > > + __tlbi(vale1, addr); > > > > + __tlbi_user(vale1, addr); > > > > + dsb(nsh); > > > > +} > > > > + > > > > +#define flush_tlb_fix_spurious_fault(vma, address) \ > > > > + local_flush_tlb_page(vma, address) > > > > > > Why can't we just have flush_tlb_fix_spurious_fault() be a NOP on arm64? > > > > Possibly, as long as any other optimisations only defer the TLB flushing > > for relatively short time (the fault is transient, it will get a > > broadcast TLBI eventually). > > > > Either way, it's worth benchmarking the above patch but with > > flush_tlb_fix_spurious_fault() a no-op (we still need flush_tlb_page() > > in ptep_set_access_flags()). Xu, Yang, could you please give it a try? > > If I understand correctly, this should do as good as the patch of Linux > or Yang in will-it-scale page_fault3 testcase, which both avoid doing > flush_tlb_fix_spurious_fault(), in case of FAULT_FLAG_TRIED. That would be a no-op even when FAULT_FLAG_TRIED isn't set. Not sure whether it makes a difference for the benchmark though. > Shouldn't we be concerned about data integrity if to have > flush_tlb_fix_spurious_fault() be a nop on arm64? Not as long as it's only called from handle_pte_fault(). In the worst case (TLBI missing on another path that makes the pte dirty/writable), it would get stuck in a spurious fault loop. That's why your early report on the 128 threads test getting stuck worried me a bit. -- Catalin