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 B5D0BC43334 for ; Tue, 19 Jul 2022 04:03:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 33A4E6B0071; Tue, 19 Jul 2022 00:03:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2C52B8E0001; Tue, 19 Jul 2022 00:03:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 13F036B0074; Tue, 19 Jul 2022 00:03:23 -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 F19E86B0071 for ; Tue, 19 Jul 2022 00:03:22 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id C0A6034EF5 for ; Tue, 19 Jul 2022 04:03:22 +0000 (UTC) X-FDA: 79702504644.28.D118B1B Received: from mail-ed1-f54.google.com (mail-ed1-f54.google.com [209.85.208.54]) by imf18.hostedemail.com (Postfix) with ESMTP id 5CA3C1C007E for ; Tue, 19 Jul 2022 04:03:22 +0000 (UTC) Received: by mail-ed1-f54.google.com with SMTP id w12so17906168edd.13 for ; Mon, 18 Jul 2022 21:03:22 -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=toOXCdfAXdcb2wgJ3r5j7cCgO4iUGmeVIA/rz0PYNVI=; b=Q2ZEm6C2G05d0cDj3qlFqcALrxw9j7dsv8bd3uFALt4muGB2VyD2XDdKhAj3DdE7Vz ByViJpUZeSvWbuZypW/DHkFykP9KUeSBMqLfg9xaXvgLFs4NyLwdBoser6p0onLIxfhF PXDorA1B2oCYW2FqIN6Wkgv6EFVdrQagc8HR1VUm7RQrSui1XzAQULp+9lSZhR+vfweX y4vnMp5quQSKg/4TlUJv39lhYVsm72c8OE3rDmW/vGcjwh2nkEF1WZTlG5gmq8wRvO6N gLRCjE5TxPk3dX/RwEoE+YqVpZn/NDGX1rEm2cV7uuFQOaCEUvtME74hF5SaYC6x9XHh d2qg== 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=toOXCdfAXdcb2wgJ3r5j7cCgO4iUGmeVIA/rz0PYNVI=; b=R9T6eG1HX2B/5Wf4H6ex9KmS159GceC+es6jCb6uxk2Qd7CSdJ8vDdmDwH0jfyFTT8 bZ1mw70XSCDNlCIXg929YBWfzvXFNURSVuCW8rm0o2snOo/XpDCCYFvwR+cQdYpDpYUj YkC2vZ76tC+w2nQBNiUVRB/z5IacNv9porRxcqp48cvkz1Obk+HuYLmGHRh11VFnEHoa 8dA8qePZ/y9hqGade/noii0FPj6eLMkMNeur7260i62J1wmFjaqVCcePM/K5+38Es5OR QaSrRpRD5aDvf5fm3lhTAr/AFfxRWRpE3meL4bSvErMse0ryz9C6jkHy9aPN7riekLlb rXpw== X-Gm-Message-State: AJIora/k5KSji7u/bPAMM+Zmn3I2dq46VR6UnpSilSN84yiUzoMPwtGp lSLMIow+kq9Wpd3Wb8plYmuGLdVeX2NuiV9BooE= X-Google-Smtp-Source: AGRyM1vH2d7fXXXz0+NviCz6tUYvSYazrFLFzzdJsFYWYXRqtTTEfMVZOYUoZKhvDVerGk8xOT0BP6H0QjmlbnIkiRQ= X-Received: by 2002:aa7:da92:0:b0:43b:345d:f64 with SMTP id q18-20020aa7da92000000b0043b345d0f64mr26936715eds.129.1658203401223; Mon, 18 Jul 2022 21:03:21 -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> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Tue, 19 Jul 2022 16:03:10 +1200 Message-ID: Subject: Re: [RESEND PATCH v3] arm64: enable THP_SWAP for arm64 To: Anshuman Khandual Cc: "Huang, Ying" , 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; imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=Q2ZEm6C2; spf=pass (imf18.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.208.54 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658203402; a=rsa-sha256; cv=none; b=AAwe6g1hfinQ40Zg27lpAVafBP85utIyIutpRpP5lKGZqxmFSoYcnyLP843Bih2+90/dj8 rOQeQQErZ0MGv31CG3G18qsuwEhIZSw3qdi83L5mkVnlvA3Nn+ETz7WYMMsV3reYbtTkgQ 5g8PuKaJ2QlLU+M/CLsPAH0Sd3dXSA0= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1658203402; 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=toOXCdfAXdcb2wgJ3r5j7cCgO4iUGmeVIA/rz0PYNVI=; b=JcMb83KOx6Qc+hZUg9kaz7jho6JEdQgvf7Mk2pciOLCJf6ELbh3aSAfjh0ZfrMmRPO1dGY Z0hdhMGjMXwn4vT9nFfdtXFYjY10xSRUn7sG2ARVTgXF3tpPB0co5+PxbnWIaxFFp91Rmg qGxHDmF+SSnkRSsz9jtwH8jvO/oNEaE= Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=Q2ZEm6C2; spf=pass (imf18.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.208.54 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspam-User: X-Rspamd-Server: rspam01 X-Stat-Signature: k3c3z5ayai4ofynfjdbc6gkdkq9h3pgb X-Rspamd-Queue-Id: 5CA3C1C007E X-HE-Tag: 1658203402-986793 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 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); } > 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