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 3EB2AC25B48 for ; Thu, 26 Oct 2023 06:02:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9A6688D0020; Thu, 26 Oct 2023 02:02:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 92ED38D0001; Thu, 26 Oct 2023 02:02:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7D1058D0020; Thu, 26 Oct 2023 02:02:08 -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 67FD78D0001 for ; Thu, 26 Oct 2023 02:02:08 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 37A8F160D06 for ; Thu, 26 Oct 2023 06:02:08 +0000 (UTC) X-FDA: 81386567136.08.1915E2F Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf15.hostedemail.com (Postfix) with ESMTP id 24233A0006 for ; Thu, 26 Oct 2023 06:02:05 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf15.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=1698300126; 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=Hnusy6WI7GqzkNfEROFKbV+x1x8pta0ZIAdtCE0I2LI=; b=tzabMa3E+E15yJWniPZj6YDDj3TSaty72FkRXAa2C2biNItmdPCRBj4vw7FwHSTCogi3ZU wiSuvn+XJsBqBdy24e4GxTBGkIf5eGSBwAyFN8U8pWBEq0PFmSuJlS72vObEYxruLK2hMb dV4vKW55mbzbEKtJ6mlHDVkP3LLSR3k= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf15.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=1698300126; a=rsa-sha256; cv=none; b=T6p1jE5+oDcW3J3q0X2GKNkaRd92MpMr3kN1icRbt1QmAoQpqCZla4aAn+xCD7w1LLs/XT ucLXZL1W3pl/uSLez2yqpUdwNQBn6xZlyj2QKolb+m6OvNNLQR6zzpgRjZb5UHgw6N2knr Fhz9VXobadvFM6BsOrR/hg8jzyipXrw= 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 64E852F4; Wed, 25 Oct 2023 23:02:46 -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 F41F43F64C; Wed, 25 Oct 2023 23:02:01 -0700 (PDT) Message-ID: Date: Thu, 26 Oct 2023 11:31:59 +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: Barry Song <21cnbao@gmail.com> Cc: Baolin Wang , catalin.marinas@arm.com, will@kernel.org, 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: <2f55f62b-cae2-4eee-8572-1b662a170880@arm.com> From: Anshuman Khandual In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 24233A0006 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: qsqwzbuyzfwgc6xa5r7z81s1zm6a4scr X-HE-Tag: 1698300125-278012 X-HE-Meta: U2FsdGVkX18j6TkkY2gejGju6YN6Gtfn8IRqWdxCnhUgYa1ehsNvHsbQQJAe3Wtp5ndiXNPUBBJi74WN9DuarxX9So5xLRvOU/FFGbNJFRT8p9ftJUI+obvZwP6kjZ12c9WYndHe3q/cWXmTREs3aJZld1Ma2BBfkVOTbmlFCknV0MXX06I4SwSdybkFOenojs5hyhu5p7CLgdOnKr5C3V6F634pQSYqRI38goJoicvzoEePjetseBb4I/+tgqQtVRIqVEg0RKp4nFsvhLrkTAoPvw6o8MDvxmLNqEnTIAWnHSKVlrFeHLjMLqXq72dr6FWH1smNHSOzngx+Xo5YVbvQd4icKQFz1oSqPhJwZCvJ4Ks6ZTO8dV9l4iyu/Vd7KsH2JxRO5FSqcxrP150jnwj2jogprXCI7BuKi59qU4qC3+nSgLCDYMqZebFfOV7xKMCHGwrJsJpHqNv9liq187qXB8aAP+7z5QO3gX9OTB5P4147Mzy9D/yLoah+FA12deA7HDkKZdE3oIxk1cngiqxwIkzh437Fpb1muM1j9dTDGmhDTgwyhxbVl/T22fedxWv0cCqPyolzy6EjrK8raxihU01LPWaGXxUAiGnCR6+nfD8KXkzn48IrmcozPItgaMvVibFKSyMka1jm6aOk501stunT7JPQnNhHM7rbaZ8tP0ZXp4a6z+ZJvUKDD9IABI3s/ovV6t6Tr9lJgslW4f+M7SUHwLgF/j0/Fk0w9DANH3q0xLsvFw4C3wttH/kwvJnLEQ3oTjanqZHQGe55/5f+7r7ebEVX2tRqy71VfljAFGkLJuL/3rC0zBPDWiYXwUrWyGjv2Ek5aYVJxgtFVxHwZXish32ByG0Ir/VasF7JuJ0ctdavYGLHOByNk8C3WK4GjWtr3di8qND/92NLFqrhxPVgq5JAUOg8OmiJ8hz03unrHcVfSbIdfmbIspP8ZZK3cfltGey9VwKvZfP cXQqBxaJ kIYqnqcOZEoZOuXFrIXTd8SG3dXuz5YpCeJJsuI6q5riyNzlhQWm2odTAaRxJdCmXkny4EHNScrezXgcDOngVKiasHZDD2XtsEXSqPSVn129OD/Zd2ubsQfUlV/QHYMHy3RGUvw0SBbVpYpQNB9nDXcDnIWFXEmsoJtYo9vXphbl08ioUldReFP8dEISc/yoiJLcWcMPrps2wJEVUPj9yK9uHKz9GU1DdSvaEwkpC/14QRs0BDV34Cgz7zWhQ5sgZvMSoFRFES1VAKXKQjugy6VxHuGZ1n4NPnYP+KDoIhc6grzaL67A62XAD0w== 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/26/23 11:24, Barry Song wrote: > On Thu, Oct 26, 2023 at 12:55 PM Anshuman Khandual > wrote: >> >> >> >> 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 > > In terms of all of above concerns, though 2 is different, which is an > issue between > cpu and non-cpu, > i feel kernel has actually dropped tlb flush at least for mglru, there > is no flush in > lru_gen_look_around(), > > static bool folio_referenced_one(struct folio *folio, > struct vm_area_struct *vma, unsigned long address, void *arg) > { > ... > > if (pvmw.pte) { > if (lru_gen_enabled() && > pte_young(ptep_get(pvmw.pte))) { > lru_gen_look_around(&pvmw); > referenced++; > } > > if (ptep_clear_flush_young_notify(vma, address, > pvmw.pte)) > referenced++; > } > > return true; > } > > and so is in walk_pte_range() of vmscan. linux has been surviving with > all above concerns for a while, believing it or not :-) Although the first two concerns could be worked upon in the SW, kernel surviving after breaking arch rules explicitly is not a correct state to be in IMHO.