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 7F46BC433EF for ; Wed, 20 Jul 2022 01:46:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0B37B6B0072; Tue, 19 Jul 2022 21:46:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 062E46B0073; Tue, 19 Jul 2022 21:46:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E71006B0074; Tue, 19 Jul 2022 21:46:19 -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 D76936B0072 for ; Tue, 19 Jul 2022 21:46:19 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id A5E4D40460 for ; Wed, 20 Jul 2022 01:46:19 +0000 (UTC) X-FDA: 79705788078.26.3430DE4 Received: from mail-ej1-f44.google.com (mail-ej1-f44.google.com [209.85.218.44]) by imf28.hostedemail.com (Postfix) with ESMTP id 4CD65C0091 for ; Wed, 20 Jul 2022 01:46:19 +0000 (UTC) Received: by mail-ej1-f44.google.com with SMTP id va17so30532497ejb.0 for ; Tue, 19 Jul 2022 18:46:19 -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=smOmG3X0yecGrPRXdwVDL8bVeA0/AdKRqOv8wWBd1AQ=; b=DPXl1rZlTt7e2ZCoy2vOwa77C/dawaIq+HckyAkPsPm+XpaEZ0f5zY2+qvCkMp228y PXvjy8wZq9Q+IOA5jj309VqdhA/4xE7rEAP2hMgJPGMme7p+tcJG3xreNqdDp8U9JyC4 Ks+S3F38S6/J3svM7Rh+tBa0MtTFLqjhsMOXOkZaO9ot8wrdYPxlb13rqT7eiulx+hST Alqk56iROWOr/Z3OOxG6f1EtNQjz0U07FUaahvtOOHfmQX5FfkU8jaUiV9JAOlq3Uc+J WOnUmQ32PwM3SF5+UpuSwdAmwzl7/NoHsmQocki6YTNr4mrQvRYSZRYPzYDi9KLbyKsg NJvA== 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=smOmG3X0yecGrPRXdwVDL8bVeA0/AdKRqOv8wWBd1AQ=; b=1aH6/HXl6X48vnNnwUSI9OYJXbNtexf0lQa4C34pWbqMqIUaFl3bvr+D3yGcCUz/sC FZOQtfTOvFu2jutHTcnO9IRR777hq+EeQwmGP4CYtXWcH3IfjyqMcshd6Lu21XRx7GRn 05EjfC+jJ6nbs9mkiQ8yKE/BZ3WxcdlLTW1J0wc6ohg2BHeDbXsY0GfndNOTc9sKPqSL tF80Sy79Z5r6ZvFzbasYtRFAvldXOxtwY7FSMaUtwufAC/8/jH7tb2jSCnzOlfz72SiS I+jOqnvK6BLvUrSVJ32xNHmlFZg8BdpGxLAP1jV014XctJyrCLxcV42Kq879/mP0MpBo el4g== X-Gm-Message-State: AJIora8OW02eZ799Ux3KlTDNvlyU9Ob+mYi7cu6/7hMvwh5Bp4Kd7mCd XBSCULCddKN8DVdNU8Y3WFq9uZ9zB7Zepy1qu3A= X-Google-Smtp-Source: AGRyM1uXYWpWuk8daws8N/wNniKutJkDLU46wfECB1sJJkjo6bqEkAsu37aI52PQDFMOsYgyU9u3SNRbaA5O+vEf4zM= X-Received: by 2002:a17:906:84f7:b0:72b:57c2:5e13 with SMTP id zp23-20020a17090684f700b0072b57c25e13mr34171630ejb.702.1658281577886; Tue, 19 Jul 2022 18:46:17 -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> <05639c8d-73f7-7e12-9941-cae3037e44b4@arm.com> In-Reply-To: <05639c8d-73f7-7e12-9941-cae3037e44b4@arm.com> From: Barry Song <21cnbao@gmail.com> Date: Wed, 20 Jul 2022 13:46:06 +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-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1658281579; 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=smOmG3X0yecGrPRXdwVDL8bVeA0/AdKRqOv8wWBd1AQ=; b=dcMYK1tY7NTHSLqmQB9k+Hkf20WckQIC3JAzS6NF/pIkS9zePXAF/q5eKkxzVY2w4ngeJ7 x6wM227fOcRKmCs11uE3Xu+Ies7vxIy3S4sM6gIrXF/fNKv+g4xGd/zKRStT5jtYzFnYtC 1AwlsUOIFSGNc6c7yh2B25cEUizx1IY= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658281579; a=rsa-sha256; cv=none; b=5ZNzI+a66RTCTPlBZbdUPb/gHmvI8X17zJEQmbqMuHeC+Qr+1568cRRMs8D1FMpSTw6CMy MQ+XfoehjQRRb15ICQ7awLhqu5iYLi7WYxiBBjH2zEaS/qjPeo80sHhKoKCFB3zPDkGhck Nm6+WVoV8nxAg477E+7Zu2GHv6qpvhg= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=DPXl1rZl; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf28.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.218.44 as permitted sender) smtp.mailfrom=21cnbao@gmail.com X-Rspamd-Queue-Id: 4CD65C0091 Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=DPXl1rZl; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf28.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.218.44 as permitted sender) smtp.mailfrom=21cnbao@gmail.com X-Rspamd-Server: rspam12 X-Rspam-User: X-Stat-Signature: eocjb3zftd44d5nf9hp7pqfnac799kpn X-HE-Tag: 1658281579-403336 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 4:04 PM Anshuman Khandual wrote: > > > > On 7/19/22 09:29, Barry Song 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); > > } > > > > 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; > > } > > Current proposal in this patch LGTM, I dont see any reason for these changes. OK, thanks, Anshuman. Can I collect this as a Reviewed-by? Barry