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 1B968C67861 for ; Mon, 8 Apr 2024 09:35:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A655A6B0099; Mon, 8 Apr 2024 05:35:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9EF426B009B; Mon, 8 Apr 2024 05:35:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8B6706B009C; Mon, 8 Apr 2024 05:35:55 -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 69F9E6B0099 for ; Mon, 8 Apr 2024 05:35:55 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 2A674A0C62 for ; Mon, 8 Apr 2024 09:35:55 +0000 (UTC) X-FDA: 81985857870.03.94A11C2 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf24.hostedemail.com (Postfix) with ESMTP id 54E8D180011 for ; Mon, 8 Apr 2024 09:35:53 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=none; spf=pass (imf24.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1712568953; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=yPrCcd2MOSnMWnlPnHprdhV1SNyI2CtuEmPTUwq4gw0=; b=qHhu5utcLPS7HTd4L2wslThE6O8dBQRjl8FM/fr2chkvC+TLqLLBdf1ktupHDocAwifTEi vlIbErhjQbPYvo4OpfUt/QVApWVyy84DjBpDi2grd9/hW6ax4Md+T7n98cN0Zc1H+NK98u 13d9ynLeX+pDfi6nHgrI+jQ7ykOqD1M= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712568953; a=rsa-sha256; cv=none; b=4wdpQsnt4DZOxFK+BYWmQJ6q9ErLvWYFVnjmCue6im8Y/JTR9+agdZn9TLOa8JigYohbwd SO+ZYM+NpU3Ub/pj580KatQMVlQrV6EtyJ8CuMnuHSPjINTjO1eMTN5IYU9dhtfyN1kfEC bqvOG0CwKqaPWDw5Cdrz+3FKFZINpPs= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=none; spf=pass (imf24.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id EC0C41007; Mon, 8 Apr 2024 02:36:22 -0700 (PDT) Received: from [10.57.73.169] (unknown [10.57.73.169]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CAE133F766; Mon, 8 Apr 2024 02:35:49 -0700 (PDT) Message-ID: <521b8246-e327-400b-ae04-8ed97f98703c@arm.com> Date: Mon, 8 Apr 2024 10:35:48 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 4/6] mm: swap: Allow storage of all mTHP orders Content-Language: en-GB To: David Hildenbrand , "Huang, Ying" Cc: Andrew Morton , Matthew Wilcox , Gao Xiang , Yu Zhao , Yang Shi , Michal Hocko , Kefeng Wang , Barry Song <21cnbao@gmail.com>, Chris Li , Lance Yang , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20240403114032.1162100-1-ryan.roberts@arm.com> <20240403114032.1162100-5-ryan.roberts@arm.com> <87edbhaexj.fsf@yhuang6-desk2.ccr.corp.intel.com> <10f8227a-c8e1-4873-aff3-6260cbe4378c@redhat.com> From: Ryan Roberts In-Reply-To: <10f8227a-c8e1-4873-aff3-6260cbe4378c@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 54E8D180011 X-Rspam-User: X-Stat-Signature: kdgtemxu5taguzy4jducedypxs16gmbw X-Rspamd-Server: rspam03 X-HE-Tag: 1712568953-496413 X-HE-Meta: U2FsdGVkX1+82/pQ9qzjavLyxi6cOd/uYHdv3GSlC82Ema9BddSMzAJxI0jzgAuN1PRBAZQnJ5Rv9LCnmlwfiSAefviPXkVeczkmCj/MRUVv4GJQb6TTWWHOWg3grcB6iPYJfCliJ6J/u5Ww9ERFWa0zbrRcDD2JBtHWtPAXiKWAybeTPT7pWBln6fE3B3Umt/usPRzSyDtKQa2l9e0SQl2xyG4YQVg3d9m2GXMSL/wnR/sf4nRtvpORcHbyHZKG36e3QAFMBz9Qui0Cp+LK3+ps9W8KfftNnF2r9WI2Io7GEebL69m0ssRT9yOC25iJqa7pl3v4aaMoZYJkJURJ0x/LHIRv6oGhYeL06DTGCf2zoOL7SZg4p1aQRYROB0MHKJWa0BDJESvQGKp6moUhmeyB86R2fPRQt3OLdvjbBXG1CaPGWeXIb7p/YOaG5cIgiDSRgZUzcx6srwvUUqbZ1h9BygiCX7zREzDTqiOw5asraHiEx0BuIz1T4TNbsbEpTtZZ5t5ygkRQdNFZIMwqImFAe8BD5NyVcAAju196XcUySdUtAWIO7iISq5vgbfypfRlQbBAVzfsWLMZJuVAdN1s3I9gwoM3Lu/CNCI5STGKsXiockCA8yA390S3ZAGI54ypfSyhohLkbcetIKkHFbtYx9Zme/6ksC/i9G/HOn3GQ+oe/uN6RSwhJMnZwLXtv8iVmg9Bg5OtaRHPBaJaiUOIrgVsOJGneG4P7M3qWSyhjyqDKHffCzxIKwtgHuJTgKXLb2gFzGR9uG1E/jwIvNZPf65uAIs++fsqoJPEEfYf2+YyEKnzICD/lXSoqFujh/+ZJ8EWjz1CzoPCPDzuilA09yDY72JZ/bvL4EFWWi5ZsawOshgYTiDGHALs4ornQ8oyN9Eba5a0ouisNoSgJ4HT26N6Z6F/wY4TOxm+vrmJuWqavHJvgimZQtIoygu1TcLxh7qzo7Q5A1+2JvTR sjzcpgtS 3l+xXca5Gh23bDELwQQQn3ibFFkrqf20iU3sdQOxu0LuKMhzLwIYZ1suTnPb4ED1GiUvgCBAJ3IIDT81n6JD1lDb8z7vUI6v6d+0vj9sYwaZj4T2178I+NdwgdJr9NTYKetYqEpJIo0JhWNQkdpG+9K5i1MiW36hmTWHMdVJTfbok0F6edoZcCbi0/zPB0zX+FcR8FaHVSdBBu509HqV+SXBnnpUgd4WLgMkM8wfhYvwviaiPY0mXjBvaFH2gPbiFGBkHCpDBAMEeulIZeRsjMYwuOw== 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: On 08/04/2024 10:33, David Hildenbrand wrote: > On 08.04.24 11:24, Ryan Roberts wrote: >> On 07/04/2024 07:02, Huang, Ying wrote: >>> David Hildenbrand writes: >>> >>>> On 03.04.24 13:40, Ryan Roberts wrote: >>>>> 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. >>>>> Reviewed-by: "Huang, Ying" >>>>> 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)); >>>> >>>> The only comment I have is that this nr_pages -> order conversion adds >>>> a bit of noise to this patch. >>>> >>>> AFAIKS, it's primarily only required for "cluster->next[order]", >>>> everything else doesn't really require the order. >>>> >>>> I'd just have split that out into a separate patch, or simply >>>> converted nr_pages -> order where required. >>>> >>>> Nothing jumped at me, but I'm not an expert on that code, so I'm >>>> mostly trusting the others ;) >>> >>> The nr_pages -> order conversion replaces ilog2(nr_pages) with >>> (1<>> don't need to worry about whether nr_pages is a power of 2.  Do you >>> think that this makes sense? >> >> I think that David's point was that I should just split out that change to its >> own patch to aid readability? I'm happy to do that if no one objects. > > Yes. Or avoiding it and not caring about a ilog vs. 1<