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 D894DC43334 for ; Tue, 19 Jul 2022 05:47:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CD91F6B0071; Tue, 19 Jul 2022 01:47:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C89248E0001; Tue, 19 Jul 2022 01:47:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B776D6B0074; Tue, 19 Jul 2022 01:47:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id A8DD56B0071 for ; Tue, 19 Jul 2022 01:47:07 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 5490035DBE for ; Tue, 19 Jul 2022 05:47:07 +0000 (UTC) X-FDA: 79702766094.29.E35DDAA Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by imf31.hostedemail.com (Postfix) with ESMTP id 6EADA2005C for ; Tue, 19 Jul 2022 05:47:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1658209626; x=1689745626; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=jmjeSdIDmrExnySMY5k5cqgszPtMtpWqsk3OgxRtt0A=; b=fvqS2BGwF1oFo/Rpm7cYm8bZAbEFvBvJgPsewnYyIMyvMYQ0cQsWkQMs uP2Bm8nVGiSj2k7OiSIcPYRBxUIqm7QJK56ShObGpZkwtR05+Xx63YYIK UtwHchdZPj1Z+LNeUSi9P52QFRRasM2OaGVSMuNj7/P30llXCv6UfbrOy 2sgWi3NLxoyDr3rDUAvzF9fL/utArC43gbG/zMr6xzzQyqQimyHl1bsNl YSlyf8TEjg76JZ9uGCXEFejVMkSTGCCWu/AS8X44rrrBzoH5sXOuj4bfb nALP2VLRn13FWjU7rxsHyH+zvBa4aNrc/dZI/wFweGdyBAWLE2lS4R6S4 A==; X-IronPort-AV: E=McAfee;i="6400,9594,10412"; a="273231394" X-IronPort-AV: E=Sophos;i="5.92,283,1650956400"; d="scan'208";a="273231394" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jul 2022 22:47:03 -0700 X-IronPort-AV: E=Sophos;i="5.92,283,1650956400"; d="scan'208";a="665293232" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.239.13.94]) by fmsmga004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jul 2022 22:46:59 -0700 From: "Huang, Ying" To: Barry Song <21cnbao@gmail.com> 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==?= Subject: Re: [RESEND PATCH v3] arm64: enable THP_SWAP for arm64 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> Date: Tue, 19 Jul 2022 13:46:24 +0800 In-Reply-To: (Barry Song's message of "Tue, 19 Jul 2022 16:03:10 +1200") Message-ID: <87o7xl1wpb.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii ARC-Authentication-Results: i=1; imf31.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=fvqS2BGw; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf31.hostedemail.com: domain of ying.huang@intel.com has no SPF policy when checking 134.134.136.20) smtp.mailfrom=ying.huang@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1658209627; 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=at8EX1g9AgznRBN2wunOA3OPFYfZPYmK0U/XcQzOgRE=; b=EBrkzoj77o7RS9dFLxzkgbN0OZyjGDHZjWdB7aYnvQ2nfQnfFG2QTyBe/mYXKWo/iQgvAf TdnnDA1NU+UdBdVT1S6zSAnaB8zOCFzDuQtxLUgBHvlTR/ee0cJ3zbZf30Mug1wMvCx2B5 LI91UK/oDltH09vX7oo44pU+V+qgI2s= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658209627; a=rsa-sha256; cv=none; b=EfmPkUJPePGPSV5q2gnYoFgwYJuBZyQiP6tNggfpI3I7nwsSpGuQww98tQKdTTvxW+kTfC Tdl58uRmsFI9OSS/ySYKzPfBLnh8c4TGM5brJXR1DOKW2GuxoBDf5A2jkwjsUxvyrgFHBb 3QJVXjE9B6oHwx+irdXLvNPPWjk+0mg= X-Stat-Signature: djfhngzqcnmaqtexn19su73txhy4cepg X-Rspamd-Queue-Id: 6EADA2005C X-Rspam-User: Authentication-Results: imf31.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=fvqS2BGw; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf31.hostedemail.com: domain of ying.huang@intel.com has no SPF policy when checking 134.134.136.20) smtp.mailfrom=ying.huang@intel.com X-Rspamd-Server: rspam11 X-HE-Tag: 1658209626-73424 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: 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. 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