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 6A71BCD11C2 for ; Wed, 3 Apr 2024 03:09:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 940F06B0083; Tue, 2 Apr 2024 23:09:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8C9B36B0085; Tue, 2 Apr 2024 23:09:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 743196B0087; Tue, 2 Apr 2024 23:09:36 -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 539686B0083 for ; Tue, 2 Apr 2024 23:09:36 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id C87AD160C14 for ; Wed, 3 Apr 2024 03:09:35 +0000 (UTC) X-FDA: 81966740310.03.74A13AC Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.11]) by imf02.hostedemail.com (Postfix) with ESMTP id 3D57280008 for ; Wed, 3 Apr 2024 03:09:33 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b="OA++KZS/"; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf02.hostedemail.com: domain of ying.huang@intel.com designates 198.175.65.11 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1712113773; 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=xyJX6AUQu7BouHyORQkY6Zh+BKzZlr7G+/4/7hV/g9E=; b=ACRyZ0TOnUSQLtd7gNyGJAErqZpXIAHzKDoR5D99iFptmLGY1iCHNfQhh8eQK6+OBXrNAb 1jjqz5j1ujv1dtwqj2qbZo6YY67Krsz42uZUWiLLRPd6PPlyfhavXNP63KlHuvdytYV1bA ulr7JN2RUSQxeAeMu6j8rgrNCM/4fcU= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b="OA++KZS/"; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf02.hostedemail.com: domain of ying.huang@intel.com designates 198.175.65.11 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712113773; a=rsa-sha256; cv=none; b=kzBr01g4dZJFlSP2wnP8yu/WosZa1trPsL3PVq1vXYD5Kge0b6UMMqAoBmOnh4WN/IVKj2 SmsRBO8mn2S3qDh8ksrD3rh4O5p1oTk5KArbYF7tSKfK8zRVwznMGKETmueBfgjtHaRxxO tUMJLx3uwHurdrq98BZEnUR5SA/yHJA= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1712113773; x=1743649773; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=KLxWiajNJqsVtcQxbeUrQEGhnijgxFfycvDn+on9l3E=; b=OA++KZS/5XihB68TuZZhj17hfa3dNXO4EJXbTQzOmehNh/CID11awXfc ZfzdutOvAksChthxRgSpjsodnPTLqim/2Nnx09U+PWH4cxgCpBTC1mLh4 Qs4rDqdnEL7q1BESE2CgpCoNAiwTtvWBG+mVqFNwMjDAve0WRuoM/iUAi XgWeCdzLI+t1E6qsz77Yb6GojKnndWD2vhIJ21qko94tvAnpjpW/lZbkh 4rMc/2IxS6Ewron6VZ4lkblTj2MZXpT9IlKMoA1ida86MU3zZcKOXIH7i vi5+tjZvwZMKGaz5tIdItnuJ7ai+gzcTxSh74oUtEpRdu7z5aMOEMn51N g==; X-CSE-ConnectionGUID: AEXgBejqRpWnexJuCNd3EQ== X-CSE-MsgGUID: DWFgYzoORQGkalsdiXUBnw== X-IronPort-AV: E=McAfee;i="6600,9927,11032"; a="17894852" X-IronPort-AV: E=Sophos;i="6.07,176,1708416000"; d="scan'208";a="17894852" Received: from orviesa002.jf.intel.com ([10.64.159.142]) by orvoesa103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Apr 2024 20:09:31 -0700 X-CSE-ConnectionGUID: 8q55C4BiT5eCCaWQ6ui1lg== X-CSE-MsgGUID: dgIC37qBQSSCiIelVtvSfA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.07,176,1708416000"; d="scan'208";a="49272587" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by orviesa002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Apr 2024 20:09:28 -0700 From: "Huang, Ying" To: Ryan Roberts Cc: Andrew Morton , David Hildenbrand , Matthew Wilcox , Gao Xiang , Yu Zhao , Yang Shi , Michal Hocko , Kefeng Wang , Barry Song <21cnbao@gmail.com>, Chris Li , Lance Yang , , Subject: Re: [PATCH v5 4/6] mm: swap: Allow storage of all mTHP orders In-Reply-To: <017632d3-7de8-407a-aaa4-caaa5ebab057@arm.com> (Ryan Roberts's message of "Tue, 2 Apr 2024 12:18:41 +0100") References: <20240327144537.4165578-1-ryan.roberts@arm.com> <20240327144537.4165578-5-ryan.roberts@arm.com> <87o7atkc3i.fsf@yhuang6-desk2.ccr.corp.intel.com> <017632d3-7de8-407a-aaa4-caaa5ebab057@arm.com> Date: Wed, 03 Apr 2024 11:07:34 +0800 Message-ID: <8734s3f8jt.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspamd-Queue-Id: 3D57280008 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: kqsfihgdexng3i9z1afy7pjyjh83ponm X-HE-Tag: 1712113773-209061 X-HE-Meta: U2FsdGVkX19sUAI7hSENJ4Af+oIt9E/0D8lyu4RklZYxOPH0naQGGIkw5nHMs4Lq0hn3QmOtoiArnLbLzx1/8gp6aEU6Q5OJFNA70MnNrMwV/bb08wxUqIpxBQz8351YIt/jRyJuh34pcYMEdo8MoRhwkfZ8zqIZgT9Kc0IqGYnuq/s9qFK6PfKGes6kM5Taoc0Dxrc1Fbku0QqYWjONQkltnhPW/h7/i+VpJXGkP6UYAsB6kYUlrSoRX6hpQBG9DHom8PLZLktW6hc1Lnxk86K4ISN8r40hviVNK6VBU81pyTvj3J6Z8AHUNUWDG3VQRNRbEan2cieFSvQ2/MA34QrbjWdoJ6p1s7XqCH4ye/yp9qBcy8WsTEklP4Oh9HAk54E4KthI1GtptoLV5FJEyUjDAQhXbgFVScYLm2aEgJky1cvOCO/AC06CDmQnygwZd0suhNWjyLF0nAGiXhqocOzBVCl8IUEqGx8j5oFHCyVAnOIYPWobeUbQqUjjB2fUvEs8w/wYKuM5N9bVb0tDPlVlOxNjvb/FtVVEnHHAN83xq8wEAo3eLziS8jw5Eul0vVbfsiG5FOOsJdh6dT9IIr/DwN24knUthQ63rA66Ul5y3jI+8iNAeFE4He3SlcMxoki4pbmf60HgN1g+X93D1D3JQpoWF/WzIodwyVsZuYHsdu9QatdsRS3p2kY6CZOECwnDNRmY6lgt7IOJgyMLYC1cIVdKh4Wrdj5lkVSh1iBXz/d1Xm5WrU4FbW/3OTo0DkprkZXP4oyMabRJ8VI7hCb6SCo0oYUKF5FE7H0SCUdKf0lx2XeYP2ukUDSkRZGXhWnEDZ4Y33Fs+8fpWt+Alt+xMjLnFWLWAQRQ6onwVyTBfCuZe0/DKsWNjBIPEXH05edo9V1ylkqn1hbhHutLT9lrolAgtMJuFrQ95Cv8xB3WaaUUY2zrl+z/cTgAkSS+NMNIkkwUBXwDnCwkukN seyfxu7Z 0TxgrvcQtfCF6wtN0XTKqhbJrd1ipDMAVmZON139hueagnBNfeLusGqpXmEev1xJJFP+n64EWm9GMY2i2qXciHE98lXmQo4oPHzjONzUOjfGLmov9tVIk2vDhGi9TZNDM8jqsWw0f0aBhT37YNrcwMEuf9zHoem1E7W/DrnINn6zLR+yEk7BSHYvgBCceev7pnfgnBYakemX1jYU= 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: Ryan Roberts writes: > On 01/04/2024 04:15, Huang, Ying wrote: >> Ryan Roberts writes: >> >>> Multi-size THP enables performance improvements by allocating large, >>> pte-mapped folios for anonymous memory. However I've observed that on an >>> arm64 system running a parallel workload (e.g. kernel compilation) >>> across many cores, under high memory pressure, the speed regresses. This >>> is due to bottlenecking on the increased number of TLBIs added due to >>> all the extra folio splitting when the large folios are swapped out. >>> >>> Therefore, solve this regression by adding support for swapping out mTHP >>> without needing to split the folio, just like is already done for >>> PMD-sized THP. This change only applies when CONFIG_THP_SWAP is enabled, >>> and when the swap backing store is a non-rotating block device. These >>> are the same constraints as for the existing PMD-sized THP swap-out >>> support. >>> >>> Note that no attempt is made to swap-in (m)THP here - this is still done >>> page-by-page, like for PMD-sized THP. But swapping-out mTHP is a >>> prerequisite for swapping-in mTHP. >>> >>> The main change here is to improve the swap entry allocator so that it >>> can allocate any power-of-2 number of contiguous entries between [1, (1 >>> << PMD_ORDER)]. This is done by allocating a cluster for each distinct >>> order and allocating sequentially from it until the cluster is full. >>> This ensures that we don't need to search the map and we get no >>> fragmentation due to alignment padding for different orders in the >>> cluster. If there is no current cluster for a given order, we attempt to >>> allocate a free cluster from the list. If there are no free clusters, we >>> fail the allocation and the caller can fall back to splitting the folio >>> and allocates individual entries (as per existing PMD-sized THP >>> fallback). >>> >>> The per-order current clusters are maintained per-cpu using the existing >>> infrastructure. This is done to avoid interleving pages from different >>> tasks, which would prevent IO being batched. This is already done for >>> the order-0 allocations so we follow the same pattern. >>> >>> As is done for order-0 per-cpu clusters, the scanner now can steal >>> order-0 entries from any per-cpu-per-order reserved cluster. This >>> ensures that when the swap file is getting full, space doesn't get tied >>> up in the per-cpu reserves. >>> >>> This change only modifies swap to be able to accept any order mTHP. It >>> doesn't change the callers to elide doing the actual split. That will be >>> done in separate changes. >>> >>> Signed-off-by: Ryan Roberts >>> --- >>> include/linux/swap.h | 10 ++- >>> mm/swap_slots.c | 6 +- >>> mm/swapfile.c | 175 ++++++++++++++++++++++++------------------- >>> 3 files changed, 109 insertions(+), 82 deletions(-) >>> >>> diff --git a/include/linux/swap.h b/include/linux/swap.h >>> index 5e1e4f5bf0cb..11c53692f65f 100644 >>> --- a/include/linux/swap.h >>> +++ b/include/linux/swap.h >>> @@ -268,13 +268,19 @@ struct swap_cluster_info { >>> */ >>> #define SWAP_NEXT_INVALID 0 >>> >>> +#ifdef CONFIG_THP_SWAP >>> +#define SWAP_NR_ORDERS (PMD_ORDER + 1) >>> +#else >>> +#define SWAP_NR_ORDERS 1 >>> +#endif >>> + >>> /* >>> * We assign a cluster to each CPU, so each CPU can allocate swap entry from >>> * its own cluster and swapout sequentially. The purpose is to optimize swapout >>> * throughput. >>> */ >>> struct percpu_cluster { >>> - unsigned int next; /* Likely next allocation offset */ >>> + unsigned int next[SWAP_NR_ORDERS]; /* Likely next allocation offset */ >>> }; >>> >>> struct swap_cluster_list { >>> @@ -471,7 +477,7 @@ swp_entry_t folio_alloc_swap(struct folio *folio); >>> bool folio_free_swap(struct folio *folio); >>> void put_swap_folio(struct folio *folio, swp_entry_t entry); >>> extern swp_entry_t get_swap_page_of_type(int); >>> -extern int get_swap_pages(int n, swp_entry_t swp_entries[], int entry_size); >>> +extern int get_swap_pages(int n, swp_entry_t swp_entries[], int order); >>> extern int add_swap_count_continuation(swp_entry_t, gfp_t); >>> extern void swap_shmem_alloc(swp_entry_t); >>> extern int swap_duplicate(swp_entry_t); >>> diff --git a/mm/swap_slots.c b/mm/swap_slots.c >>> index 53abeaf1371d..13ab3b771409 100644 >>> --- a/mm/swap_slots.c >>> +++ b/mm/swap_slots.c >>> @@ -264,7 +264,7 @@ static int refill_swap_slots_cache(struct swap_slots_cache *cache) >>> cache->cur = 0; >>> if (swap_slot_cache_active) >>> cache->nr = get_swap_pages(SWAP_SLOTS_CACHE_SIZE, >>> - cache->slots, 1); >>> + cache->slots, 0); >>> >>> return cache->nr; >>> } >>> @@ -311,7 +311,7 @@ swp_entry_t folio_alloc_swap(struct folio *folio) >>> >>> if (folio_test_large(folio)) { >>> if (IS_ENABLED(CONFIG_THP_SWAP)) >>> - get_swap_pages(1, &entry, folio_nr_pages(folio)); >>> + get_swap_pages(1, &entry, folio_order(folio)); >>> goto out; >>> } >>> >>> @@ -343,7 +343,7 @@ swp_entry_t folio_alloc_swap(struct folio *folio) >>> goto out; >>> } >>> >>> - get_swap_pages(1, &entry, 1); >>> + get_swap_pages(1, &entry, 0); >>> out: >>> if (mem_cgroup_try_charge_swap(folio, entry)) { >>> put_swap_folio(folio, entry); >>> diff --git a/mm/swapfile.c b/mm/swapfile.c >>> index 1393966b77af..d56cdc547a06 100644 >>> --- a/mm/swapfile.c >>> +++ b/mm/swapfile.c >>> @@ -278,15 +278,15 @@ static void discard_swap_cluster(struct swap_info_struct *si, >>> #ifdef CONFIG_THP_SWAP >>> #define SWAPFILE_CLUSTER HPAGE_PMD_NR >>> >>> -#define swap_entry_size(size) (size) >>> +#define swap_entry_order(order) (order) >>> #else >>> #define SWAPFILE_CLUSTER 256 >>> >>> /* >>> - * Define swap_entry_size() as constant to let compiler to optimize >>> + * Define swap_entry_order() as constant to let compiler to optimize >>> * out some code if !CONFIG_THP_SWAP >>> */ >>> -#define swap_entry_size(size) 1 >>> +#define swap_entry_order(order) 0 >>> #endif >>> #define LATENCY_LIMIT 256 >>> >>> @@ -551,10 +551,12 @@ static void free_cluster(struct swap_info_struct *si, unsigned long idx) >>> >>> /* >>> * The cluster corresponding to page_nr will be used. The cluster will be >>> - * removed from free cluster list and its usage counter will be increased. >>> + * removed from free cluster list and its usage counter will be increased by >>> + * count. >>> */ >>> -static void inc_cluster_info_page(struct swap_info_struct *p, >>> - struct swap_cluster_info *cluster_info, unsigned long page_nr) >>> +static void add_cluster_info_page(struct swap_info_struct *p, >>> + struct swap_cluster_info *cluster_info, unsigned long page_nr, >>> + unsigned long count) >>> { >>> unsigned long idx = page_nr / SWAPFILE_CLUSTER; >>> >>> @@ -563,9 +565,19 @@ static void inc_cluster_info_page(struct swap_info_struct *p, >>> if (cluster_is_free(&cluster_info[idx])) >>> alloc_cluster(p, idx); >>> >>> - VM_BUG_ON(cluster_count(&cluster_info[idx]) >= SWAPFILE_CLUSTER); >>> + VM_BUG_ON(cluster_count(&cluster_info[idx]) + count > SWAPFILE_CLUSTER); >>> cluster_set_count(&cluster_info[idx], >>> - cluster_count(&cluster_info[idx]) + 1); >>> + cluster_count(&cluster_info[idx]) + count); >>> +} >>> + >>> +/* >>> + * The cluster corresponding to page_nr will be used. The cluster will be >>> + * removed from free cluster list and its usage counter will be increased by 1. >>> + */ >>> +static void inc_cluster_info_page(struct swap_info_struct *p, >>> + struct swap_cluster_info *cluster_info, unsigned long page_nr) >>> +{ >>> + add_cluster_info_page(p, cluster_info, page_nr, 1); >>> } >>> >>> /* >>> @@ -595,7 +607,7 @@ static void dec_cluster_info_page(struct swap_info_struct *p, >>> */ >>> static bool >>> scan_swap_map_ssd_cluster_conflict(struct swap_info_struct *si, >>> - unsigned long offset) >>> + unsigned long offset, int order) >>> { >>> struct percpu_cluster *percpu_cluster; >>> bool conflict; >>> @@ -609,24 +621,39 @@ scan_swap_map_ssd_cluster_conflict(struct swap_info_struct *si, >>> return false; >>> >>> percpu_cluster = this_cpu_ptr(si->percpu_cluster); >>> - percpu_cluster->next = SWAP_NEXT_INVALID; >>> + percpu_cluster->next[order] = SWAP_NEXT_INVALID; >>> + return true; >>> +} >>> + >>> +static inline bool swap_range_empty(char *swap_map, unsigned int start, >>> + unsigned int nr_pages) >>> +{ >>> + unsigned int i; >>> + >>> + for (i = 0; i < nr_pages; i++) { >>> + if (swap_map[start + i]) >>> + return false; >>> + } >>> + >>> return true; >>> } >>> >>> /* >>> - * Try to get a swap entry from current cpu's swap entry pool (a cluster). This >>> - * might involve allocating a new cluster for current CPU too. >>> + * Try to get swap entries with specified order from current cpu's swap entry >>> + * pool (a cluster). This might involve allocating a new cluster for current CPU >>> + * too. >>> */ >>> static bool scan_swap_map_try_ssd_cluster(struct swap_info_struct *si, >>> - unsigned long *offset, unsigned long *scan_base) >>> + unsigned long *offset, unsigned long *scan_base, int order) >>> { >>> + unsigned int nr_pages = 1 << order; >> >> Use swap_entry_order()? > > I had previously convinced myself that the compiler should be smart enough to > propagate the constant from > > get_swap_pages -> scan_swap_map_slots -> scan_swap_map_try_ssd_cluster Do some experiments via calling function with constants and check the compiled code. It seems that "interprocedural constant propagation" in compiler can optimize the code at least if the callee is "static". > But I'll add the explicit macro for the next version, as you suggest. So, I will leave it to you to decide whether to do that. -- Best Regards, Huang, Ying [snip]