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 1FFD0C43334 for ; Tue, 19 Jul 2022 06:13:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 73AD96B0071; Tue, 19 Jul 2022 02:13:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6C46B6B0073; Tue, 19 Jul 2022 02:13:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 58C986B0074; Tue, 19 Jul 2022 02:13:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 48E9F6B0071 for ; Tue, 19 Jul 2022 02:13:46 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 181C52183A for ; Tue, 19 Jul 2022 06:13:46 +0000 (UTC) X-FDA: 79702833252.25.EB62346 Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by imf31.hostedemail.com (Postfix) with ESMTP id 62F2220003 for ; Tue, 19 Jul 2022 06:13:45 +0000 (UTC) Received: by mail-ed1-f46.google.com with SMTP id y4so18203443edc.4 for ; Mon, 18 Jul 2022 23:13:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=7hkzYipv6QANdIAdRAg9h0BtLWb126ab0LzpcCvcOQM=; b=BuDNkzf1smCEFzAxlPTgGFRgpgH+zyqAym6t26Sbn4+B4S1ZCBTYHqrps8aetR2D4J 5B0/1amoFrDnelpWHNGZbSxcMLW7KtuwE2UKarGqVTEHAPB9JzqOOmXLiJofMGGzxDuc 2crlL2Htv/HK3EfbruPAlp6RO7pRrx7Fjq78Yf0cK/5WnNQi9RTDqaRoBvE7okJdoGce a2fSiE30PiMLxO2uC2SMd64tZw+tM6VsA7vFhd75iDthpqzy6HCuY3tmQQ7p+WYMDdDc NI3PuKXhSbr9owsGxesgZVf3W0kI43qYhAmRhhKbSjnoEwUDEXIQab/YMqo1jOS8XX+E 5jtA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=7hkzYipv6QANdIAdRAg9h0BtLWb126ab0LzpcCvcOQM=; b=h+iT1kTBunChDbIUlFM4iPfU6sehy7J0OlLDXSpLIwbWCH+xgzACTyp/Zy1eizP2RL GQcmpeN0rXV/gJGHa0If87V/5CG4jB04MSzh6qJ2RvR2MkpgrEsFcWVg2tcfPF11fmLN IEtZ+J9k4vz2zL6AJUInn+cqFaZbr6EIG5i0K6h1rsolG1KkrFU2YCfl3Czecm3Xsqgi NsLvFQrlKKXQhk2GKcgP7eMr+K5+lHSGGSpcRoKwZX98rOwgKGa0b0eo4IG3KBZkwwmH wo8Ptmaq162S4NakwcDv3VjthNR215dfYFbewUl2UOuzMulP1mD9T5L0xV0N3ZHrdcRn NqRA== X-Gm-Message-State: AJIora+n1P8k3ouhJ7V4JnInZ/GsYvqeu3dKHqE/LAcJArQExSF24kuK FMVps1ejAdo2us32ah4fq7dESi2k4bvOk9Y7k8c= X-Google-Smtp-Source: AGRyM1s8ILStky9/CfngvYEJvr8MP9YkikfhPxiKOdPPlC2R0k6OK9qqXKOnWoX/rcOyn9TDtq6xG41WOG0EXC2DfcQ= X-Received: by 2002:a05:6402:d53:b0:43b:a0cf:d970 with SMTP id ec19-20020a0564020d5300b0043ba0cfd970mr367240edb.277.1658211223937; Mon, 18 Jul 2022 23:13:43 -0700 (PDT) MIME-Version: 1.0 References: <20220718090050.2261-1-21cnbao@gmail.com> <87mtd62apo.fsf@yhuang6-desk2.ccr.corp.intel.com> <87zgh5232o.fsf@yhuang6-desk2.ccr.corp.intel.com> <416a06f6-ca7d-d4a9-2cda-af0ad6e28261@arm.com> <87o7xl1wpb.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <87o7xl1wpb.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Barry Song <21cnbao@gmail.com> Date: Tue, 19 Jul 2022 18:13:32 +1200 Message-ID: Subject: Re: [RESEND PATCH v3] arm64: enable THP_SWAP for arm64 To: "Huang, Ying" Cc: Anshuman Khandual , Andrew Morton , Catalin Marinas , LAK , Linux-MM , Steven Price , Will Deacon , Andrea Arcangeli , =?UTF-8?B?6YOt5YGl?= , hanchuanhua , Johannes Weiner , Hugh Dickins , LKML , Minchan Kim , Yang Shi , Barry Song , =?UTF-8?B?5byg6K+X5piOKFNpbW9uIFpoYW5nKQ==?= Content-Type: text/plain; charset="UTF-8" ARC-Authentication-Results: i=1; imf31.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=BuDNkzf1; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf31.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.208.46 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1658211225; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=7hkzYipv6QANdIAdRAg9h0BtLWb126ab0LzpcCvcOQM=; b=WC9lpujeMc8Y3O39dsNE+KarlU8PkAqXTQVd9Jh2IfG2YwFTywMaj4QqOJ/sXnjnYFMDvN XKsZqhgBRG7q1OBv1spnf2hXhy0PB6ZvMjQi9F9LCXJgN9OlgEY8NaqyWMS0SsHM4jfuwP j1f0jyZLRkw0PKev/WlLx9JXxziSh5M= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658211225; a=rsa-sha256; cv=none; b=fWkXBDSYBBDjhm+vw8PL8h7Pdyqx2zkoFlpieLMZbMUXg2DS65E/JPijKraoebwaY/kZ+v ndWA7o5YMi2XiDcymXF2b4OdJjSuaA2QUvdsC+5Q9b0riidnpTESAZ5Agl9XnOdLGhnkcg eH1ADtHnzhLZGMMZW0EQO4dAy25+hls= X-Stat-Signature: mn88sz5mod533bpaze3418hwzxf5t46g X-Rspamd-Queue-Id: 62F2220003 X-Rspamd-Server: rspam08 Authentication-Results: imf31.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=BuDNkzf1; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf31.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.208.46 as permitted sender) smtp.mailfrom=21cnbao@gmail.com X-Rspam-User: X-HE-Tag: 1658211225-868155 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 19, 2022 at 5:47 PM Huang, Ying wrote: > > Barry Song <21cnbao@gmail.com> writes: > > > On Tue, Jul 19, 2022 at 3:59 PM Barry Song <21cnbao@gmail.com> wrote: > >> > >> On Tue, Jul 19, 2022 at 3:35 PM Anshuman Khandual > >> wrote: > >> > > >> > > >> > > >> > On 7/19/22 08:58, Huang, Ying wrote: > >> > > Anshuman Khandual writes: > >> > > > >> > >> On 7/19/22 06:53, Barry Song wrote: > >> > >>> On Tue, Jul 19, 2022 at 12:44 PM Huang, Ying wrote: > >> > >>>> > >> > >>>> Barry Song <21cnbao@gmail.com> writes: > >> > >>>> > >> > >>>>> From: Barry Song > >> > >>>>> > >> > >>>>> THP_SWAP has been proven to improve the swap throughput significantly > >> > >>>>> on x86_64 according to commit bd4c82c22c367e ("mm, THP, swap: delay > >> > >>>>> splitting THP after swapped out"). > >> > >>>>> As long as arm64 uses 4K page size, it is quite similar with x86_64 > >> > >>>>> by having 2MB PMD THP. THP_SWAP is architecture-independent, thus, > >> > >>>>> enabling it on arm64 will benefit arm64 as well. > >> > >>>>> A corner case is that MTE has an assumption that only base pages > >> > >>>>> can be swapped. We won't enable THP_SWAP for ARM64 hardware with > >> > >>>>> MTE support until MTE is reworked to coexist with THP_SWAP. > >> > >>>>> > >> > >>>>> A micro-benchmark is written to measure thp swapout throughput as > >> > >>>>> below, > >> > >>>>> > >> > >>>>> unsigned long long tv_to_ms(struct timeval tv) > >> > >>>>> { > >> > >>>>> return tv.tv_sec * 1000 + tv.tv_usec / 1000; > >> > >>>>> } > >> > >>>>> > >> > >>>>> main() > >> > >>>>> { > >> > >>>>> struct timeval tv_b, tv_e;; > >> > >>>>> #define SIZE 400*1024*1024 > >> > >>>>> volatile void *p = mmap(NULL, SIZE, PROT_READ | PROT_WRITE, > >> > >>>>> MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > >> > >>>>> if (!p) { > >> > >>>>> perror("fail to get memory"); > >> > >>>>> exit(-1); > >> > >>>>> } > >> > >>>>> > >> > >>>>> madvise(p, SIZE, MADV_HUGEPAGE); > >> > >>>>> memset(p, 0x11, SIZE); /* write to get mem */ > >> > >>>>> > >> > >>>>> gettimeofday(&tv_b, NULL); > >> > >>>>> madvise(p, SIZE, MADV_PAGEOUT); > >> > >>>>> gettimeofday(&tv_e, NULL); > >> > >>>>> > >> > >>>>> printf("swp out bandwidth: %ld bytes/ms\n", > >> > >>>>> SIZE/(tv_to_ms(tv_e) - tv_to_ms(tv_b))); > >> > >>>>> } > >> > >>>>> > >> > >>>>> Testing is done on rk3568 64bit quad core processor Quad Core > >> > >>>>> Cortex-A55 platform - ROCK 3A. > >> > >>>>> thp swp throughput w/o patch: 2734bytes/ms (mean of 10 tests) > >> > >>>>> thp swp throughput w/ patch: 3331bytes/ms (mean of 10 tests) > >> > >>>>> > >> > >>>>> Cc: "Huang, Ying" > >> > >>>>> Cc: Minchan Kim > >> > >>>>> Cc: Johannes Weiner > >> > >>>>> Cc: Hugh Dickins > >> > >>>>> Cc: Andrea Arcangeli > >> > >>>>> Cc: Anshuman Khandual > >> > >>>>> Cc: Steven Price > >> > >>>>> Cc: Yang Shi > >> > >>>>> Signed-off-by: Barry Song > >> > >>>>> --- > >> > >>>>> -v3: > >> > >>>>> * refine the commit log; > >> > >>>>> * add a benchmark result; > >> > >>>>> * refine the macro of arch_thp_swp_supported > >> > >>>>> Thanks to the comments of Anshuman, Andrew, Steven > >> > >>>>> > >> > >>>>> arch/arm64/Kconfig | 1 + > >> > >>>>> arch/arm64/include/asm/pgtable.h | 6 ++++++ > >> > >>>>> include/linux/huge_mm.h | 12 ++++++++++++ > >> > >>>>> mm/swap_slots.c | 2 +- > >> > >>>>> 4 files changed, 20 insertions(+), 1 deletion(-) > >> > >>>>> > >> > >>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > >> > >>>>> index 1652a9800ebe..e1c540e80eec 100644 > >> > >>>>> --- a/arch/arm64/Kconfig > >> > >>>>> +++ b/arch/arm64/Kconfig > >> > >>>>> @@ -101,6 +101,7 @@ config ARM64 > >> > >>>>> select ARCH_WANT_HUGETLB_PAGE_OPTIMIZE_VMEMMAP > >> > >>>>> select ARCH_WANT_LD_ORPHAN_WARN > >> > >>>>> select ARCH_WANTS_NO_INSTR > >> > >>>>> + select ARCH_WANTS_THP_SWAP if ARM64_4K_PAGES > >> > >>>>> select ARCH_HAS_UBSAN_SANITIZE_ALL > >> > >>>>> select ARM_AMBA > >> > >>>>> select ARM_ARCH_TIMER > >> > >>>>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h > >> > >>>>> index 0b6632f18364..78d6f6014bfb 100644 > >> > >>>>> --- a/arch/arm64/include/asm/pgtable.h > >> > >>>>> +++ b/arch/arm64/include/asm/pgtable.h > >> > >>>>> @@ -45,6 +45,12 @@ > >> > >>>>> __flush_tlb_range(vma, addr, end, PUD_SIZE, false, 1) > >> > >>>>> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ > >> > >>>>> > >> > >>>>> +static inline bool arch_thp_swp_supported(void) > >> > >>>>> +{ > >> > >>>>> + return !system_supports_mte(); > >> > >>>>> +} > >> > >>>>> +#define arch_thp_swp_supported arch_thp_swp_supported > >> > >>>>> + > >> > >>>>> /* > >> > >>>>> * Outside of a few very special situations (e.g. hibernation), we always > >> > >>>>> * use broadcast TLB invalidation instructions, therefore a spurious page > >> > >>>>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > >> > >>>>> index de29821231c9..4ddaf6ad73ef 100644 > >> > >>>>> --- a/include/linux/huge_mm.h > >> > >>>>> +++ b/include/linux/huge_mm.h > >> > >>>>> @@ -461,4 +461,16 @@ static inline int split_folio_to_list(struct folio *folio, > >> > >>>>> return split_huge_page_to_list(&folio->page, list); > >> > >>>>> } > >> > >>>>> > >> > >>>>> +/* > >> > >>>>> + * archs that select ARCH_WANTS_THP_SWAP but don't support THP_SWP due to > >> > >>>>> + * limitations in the implementation like arm64 MTE can override this to > >> > >>>>> + * false > >> > >>>>> + */ > >> > >>>>> +#ifndef arch_thp_swp_supported > >> > >>>>> +static inline bool arch_thp_swp_supported(void) > >> > >>>>> +{ > >> > >>>>> + return true; > >> > >>>>> +} > >> > >>>> > >> > >>>> How about the following? > >> > >>>> > >> > >>>> static inline bool arch_wants_thp_swap(void) > >> > >>>> { > >> > >>>> return IS_ENABLED(ARCH_WANTS_THP_SWAP); > >> > >>>> } > >> > >>> > >> > >>> This looks good. then i'll need to change arm64 to > >> > >>> > >> > >>> +static inline bool arch_thp_swp_supported(void) > >> > >>> +{ > >> > >>> + return IS_ENABLED(ARCH_WANTS_THP_SWAP) && !system_supports_mte(); > >> > >>> +} > >> > >> > >> > >> Why ? CONFIG_THP_SWAP depends on ARCH_WANTS_THP_SWAP. In folio_alloc_swap(), > >> > >> IS_ENABLED(CONFIG_THP_SWAP) enabled, will also imply ARCH_WANTS_THP_SWAP too > >> > >> is enabled. Hence checking for ARCH_WANTS_THP_SWAP again does not make sense > >> > >> either in the generic fallback stub, or in arm64 platform override. Because > >> > >> without ARCH_WANTS_THP_SWAP enabled, arch_thp_swp_supported() should never > >> > >> be called in the first place. > >> > > > >> > > For the only caller now, the checking looks redundant. But the original > >> > > proposed implementation as follows, > >> > > > >> > > static inline bool arch_thp_swp_supported(void) > >> > > { > >> > > return true; > >> > > } > >> > > > >> > > will return true even on architectures that don't support/want THP swap. > >> > > >> > But the function will never be called on for those platforms. > >> > > >> > > That will confuse people too. > >> > > >> > I dont see how. > >> > > >> > > > >> > > And the "redundant" checking has no run time overhead, because compiler > >> > > will do the trick. > >> > I understand that, but dont think this indirection is necessary. > >> > >> Hi Anshuman, Hi Ying, > >> Thanks for the comments of both of you. Does the below look ok? > >> > >> generic, > >> > >> static inline bool arch_wants_thp_swap(void) > >> { > >> return IS_ENABLED(CONFIG_THP_SWAP); > >> } > >> > > > > sorry, i actually meant arch_thp_swp_supported() but not > > arch_wants_thp_swap() in generic code, > > > > static inline bool arch_thp_swp_supported(void) > > { > > return IS_ENABLED(CONFIG_THP_SWAP); > > } > > IS_ENABLED(CONFIG_THP_SWAP) doesn't match the name too. It's an option > selected by users. arch_thp_swp_supported() is to report the > capability. Hi Ying, CONFIG_THP_SWAP implicitly includes ARCH_WANTS_THP_SWAP. So it seems a bit odd to have still another arch_wants_thp_swap(). if the name of arch_thp_swp_supported is not sensible to you, will thp_swp_supported() without arch_ make more sense? a similar example is, static inline bool gigantic_page_runtime_supported(void) { return IS_ENABLED(CONFIG_ARCH_HAS_GIGANTIC_PAGE); } Otherwise, can we just keep the code as is according to Anshuman's suggestion? Thanks Barry } > > Best Regards, > Huang, Ying > > >> arm64, > >> > >> static inline bool arch_thp_swp_supported(void) > >> { > >> return IS_ENABLED(CONFIG_THP_SWAP) && !system_supports_mte(); > >> } > >> > >> caller, > >> > >> folio_alloc_swap(struct folio *folio) > >> { > >> > >> if (folio_test_large(folio)) { > >> - if (IS_ENABLED(CONFIG_THP_SWAP)) > >> + if (arch_thp_swp_supported()) > >> get_swap_pages(1, &entry, folio_nr_pages(folio)); > >> goto out; > >> } > >> > >> Thanks > >> Barry