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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0D698C25B48 for ; Thu, 26 Oct 2023 04:55:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 331DD6B02F3; Thu, 26 Oct 2023 00:55:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2E1986B02F7; Thu, 26 Oct 2023 00:55:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1CFCC6B02F9; Thu, 26 Oct 2023 00:55:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 0DB606B02F3 for ; Thu, 26 Oct 2023 00:55:13 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id D9682B56B7 for ; Thu, 26 Oct 2023 04:55:12 +0000 (UTC) X-FDA: 81386398464.15.2CB8992 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf11.hostedemail.com (Postfix) with ESMTP id A0D604000A for ; Thu, 26 Oct 2023 04:55:10 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf11.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1698296111; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wKn3yhyvWnJ9Hget4fHDKydzLkm62ujKjw/CNygO1R0=; b=3iFTi612vuWpka/lN4fESZPgjCOSxmqiGjRobcja5ErII+ECoBDmlEnvo5RmA8A4OtUcNQ mjXv1nhBhiMXfljjou+j4HptwdffXlqKur1j7NSc9Jdt5WPKy5so1cnl0lq6y4m2qdqcDa 5murLgLUbGwaCXnOUTfU4MPwMcxDQkM= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf11.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1698296111; a=rsa-sha256; cv=none; b=E9xqbaKKPm73Y9r432jllv2hqECk3RKcv8B0mb5sQVPrDgOjYObEb8J4mE8rW2mDURyxp0 U2yTT8q2j0odyASOwfynm8sbuR2DifUALFOb534X7vI+v8jjYxnZRgX34Q/WCr3qQZ69sr kLmglT7DPxu7v6iQIwLCYhZuRcantwo= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2BBC72F4; Wed, 25 Oct 2023 21:55:51 -0700 (PDT) Received: from [10.162.41.8] (a077893.blr.arm.com [10.162.41.8]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E58483F762; Wed, 25 Oct 2023 21:55:06 -0700 (PDT) Message-ID: <2f55f62b-cae2-4eee-8572-1b662a170880@arm.com> Date: Thu, 26 Oct 2023 10:25:03 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] arm64: mm: drop tlb flush operation when clearing the access bit Content-Language: en-US To: Baolin Wang , catalin.marinas@arm.com, will@kernel.org Cc: akpm@linux-foundation.org, v-songbaohua@oppo.com, yuzhao@google.com, linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: From: Anshuman Khandual In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Stat-Signature: epz61mhuandpqw154x6kmnz5nr4kfoua X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: A0D604000A X-HE-Tag: 1698296110-207457 X-HE-Meta: U2FsdGVkX1/ewHqwb6P/RUG+DeaRZdjLGeHrNsLAU+AqbglVWLJTrt5bAvO4J77ba8qZA3u3T+J0mqT6NF02grLMS4A8Ymx/QXVnV8U/cOjq8vDGa7tf96CdmhXU/SRUW9uIgotbsZYybYqe/4tmot43Vo0rbFPhCC3TFH6LGqGyosp5RbwUcno7rTAgTzjxDxr7NfWI51ub/7cx4dtdpJdgrbkskmaoQRavItTJz80UjwxHqi6TpluTwWS3TWV2OFi3KUuwBvjv76oenJeDkaMmf2k8koNWyvtGXV0znsOfx6sb0vGEeCPvinghvPQ7GXU8iTuyvS5BvQ3KqIlOHxKKUwMcQeZ/A7OHdfJLWS4lQk9wimP4m0+Wg9MBlY0yfNFhHMmtgdAarT34pFoLvflO7k/XqiR/FGmZDS/uhWUpke1zrRk+f0hrmfdFsMPl0WVo52//d/XH2IFJPKPgPa/EankvZmErXdEFXQJs2pHsR1wXiYt2KMl+WkNby3LaEVSKiBnpJb+iMADCosPgjVdGhhJQaVXHWKYjtU7lFLJXKn98oi5luskAv3+z+q1ysnRifkZA26SbEb7/tHIgblNWyW4F/Uw6tQz3pY5+4/HJX5LlW88bYDxjCiZb1wu75mq8JVd10VTXZ98F4nujK8mFMZq2Dd8DtvCO+JpvzpLLdyhXNsH6eF6bHMWHtBXmiwy2SUJ+yyXv6u2IhL8KKQtQeAMGj95QSGRC+gosQuXtuM/BjK4UMVwbgsj3LOjDPCDzXarA5cFNLn763qIV4oqhSc6cvB3hk3TqIPK4SrUPdobLOQgFTPtSSGd0smk5aNiyjoy6JdDQDGHXsQajuILSQF6l1ECR9imSBuWdKOjiWQtjTl4d+omgXRh83rK0gqGdSYwa84H3WcjgFnG8asoXgtws3WuIRN1kAF8Z4fHgpjW0SZZ7C6TI+u6AdM8MfdS3/pOg3yrfqJl4KTY edP+mCXN X7BGGF8rEQQaoDsTK5bEsF4FNFTuYdfgEzLJHWiWB3lOBRWs7MGIvpFLRgd9jB6RIYMBmsqXbEPEevLWbrGHPm4Ul5+ZBalqg9jqoMu6r/A6X8vCD0mqtUTQYhtuJMnaeVM0fNubQnSDq39xGccfWrNSGm9O45wDBibC0VhzXjEv4dcmVsVQU5g1xVe+Hz/wJ5CzdUQ6gZaMA39sIOpt3ZK9f5rFSnJQqpYn6u6GkYrSu76YPwt3obi7ZDy59WZzqwIbR 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: List-Subscribe: List-Unsubscribe: On 10/24/23 18:26, Baolin Wang wrote: > Now ptep_clear_flush_young() is only called by folio_referenced() to > check if the folio was referenced, and now it will call a tlb flush on > ARM64 architecture. However the tlb flush can be expensive on ARM64 > servers, especially for the systems with a large CPU numbers. TLB flush would be expensive on *any* platform with large CPU numbers ? > > Similar to the x86 architecture, below comments also apply equally to > ARM64 architecture. So we can drop the tlb flush operation in > ptep_clear_flush_young() on ARM64 architecture to improve the performance. > " > /* Clearing the accessed bit without a TLB flush > * doesn't cause data corruption. [ It could cause incorrect > * page aging and the (mistaken) reclaim of hot pages, but the > * chance of that should be relatively low. ] > * > * So as a performance optimization don't flush the TLB when > * clearing the accessed bit, it will eventually be flushed by > * a context switch or a VM operation anyway. [ In the rare > * event of it not getting flushed for a long time the delay > * shouldn't really matter because there's no real memory > * pressure for swapout to react to. ] > */ If always true, this sounds generic enough for all platforms, why only x86 and arm64 ? > " > Running the thpscale to show some obvious improvements for compaction > latency with this patch: > base patched > Amean fault-both-1 1093.19 ( 0.00%) 1084.57 * 0.79%* > Amean fault-both-3 2566.22 ( 0.00%) 2228.45 * 13.16%* > Amean fault-both-5 3591.22 ( 0.00%) 3146.73 * 12.38%* > Amean fault-both-7 4157.26 ( 0.00%) 4113.67 * 1.05%* > Amean fault-both-12 6184.79 ( 0.00%) 5218.70 * 15.62%* > Amean fault-both-18 9103.70 ( 0.00%) 7739.71 * 14.98%* > Amean fault-both-24 12341.73 ( 0.00%) 10684.23 * 13.43%* > Amean fault-both-30 15519.00 ( 0.00%) 13695.14 * 11.75%* > Amean fault-both-32 16189.15 ( 0.00%) 14365.73 * 11.26%* > base patched > Duration User 167.78 161.03 > Duration System 1836.66 1673.01 > Duration Elapsed 2074.58 2059.75 Could you please point to the test repo you are running ? > > Barry Song submitted a similar patch [1] before, that replaces the > ptep_clear_flush_young_notify() with ptep_clear_young_notify() in > folio_referenced_one(). However, I'm not sure if removing the tlb flush > operation is applicable to every architecture in kernel, so dropping > the tlb flush for ARM64 seems a sensible change. The reasoning provided here sounds generic when true, hence there seems to be no justification to keep it limited just for arm64 and x86. Also what about pmdp_clear_flush_young_notify() when THP is enabled. Should that also not do a TLB flush after clearing access bit ? Although arm64 does not enable __HAVE_ARCH_PMDP_CLEAR_YOUNG_FLUSH, rather depends on the generic pmdp_clear_flush_young() which also does a TLB flush via flush_pmd_tlb_range() while clearing the access bit. > > Note: I am okay for both approach, if someone can help to ensure that > all architectures do not need the tlb flush when clearing the accessed > bit, then I also think Barry's patch is better (hope Barry can resend > his patch). This paragraph belongs after the '----' below and not part of the commit message. > > [1] https://lore.kernel.org/lkml/20220617070555.344368-1-21cnbao@gmail.com/ > Signed-off-by: Baolin Wang > --- > arch/arm64/include/asm/pgtable.h | 31 ++++++++++++++++--------------- > 1 file changed, 16 insertions(+), 15 deletions(-) > > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h > index 0bd18de9fd97..2979d796ba9d 100644 > --- a/arch/arm64/include/asm/pgtable.h > +++ b/arch/arm64/include/asm/pgtable.h > @@ -905,21 +905,22 @@ static inline int ptep_test_and_clear_young(struct vm_area_struct *vma, > static inline int ptep_clear_flush_young(struct vm_area_struct *vma, > unsigned long address, pte_t *ptep) > { > - int young = ptep_test_and_clear_young(vma, address, ptep); > - > - if (young) { > - /* > - * We can elide the trailing DSB here since the worst that can > - * happen is that a CPU continues to use the young entry in its > - * TLB and we mistakenly reclaim the associated page. The > - * window for such an event is bounded by the next > - * context-switch, which provides a DSB to complete the TLB > - * invalidation. > - */ > - flush_tlb_page_nosync(vma, address); > - } > - > - return young; > + /* > + * This comment is borrowed from x86, but applies equally to ARM64: > + * > + * Clearing the accessed bit without a TLB flush doesn't cause > + * data corruption. [ It could cause incorrect page aging and > + * the (mistaken) reclaim of hot pages, but the chance of that > + * should be relatively low. ] > + * > + * So as a performance optimization don't flush the TLB when > + * clearing the accessed bit, it will eventually be flushed by > + * a context switch or a VM operation anyway. [ In the rare > + * event of it not getting flushed for a long time the delay > + * shouldn't really matter because there's no real memory > + * pressure for swapout to react to. ] > + */ > + return ptep_test_and_clear_young(vma, address, ptep); > } > > #ifdef CONFIG_TRANSPARENT_HUGEPAGE There are three distinct concerns here 1) What are the chances of this misleading existing hot page reclaim process 2) How secondary MMU such as SMMU adapt to change in mappings without a flush 3) Could this break the architecture rule requiring a TLB flush after access bit clear on a page table entry