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 09FE5C48BC3 for ; Wed, 14 Feb 2024 16:41:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 933F26B0071; Wed, 14 Feb 2024 11:41:29 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8E3C76B00A0; Wed, 14 Feb 2024 11:41:29 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 737036B00A1; Wed, 14 Feb 2024 11:41:29 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 63E326B0071 for ; Wed, 14 Feb 2024 11:41:29 -0500 (EST) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 36E27140D51 for ; Wed, 14 Feb 2024 16:41:29 +0000 (UTC) X-FDA: 81790975098.30.DBB5AEC Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf15.hostedemail.com (Postfix) with ESMTP id 4626FA0029 for ; Wed, 14 Feb 2024 16:41:27 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf15.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707928887; 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=RfwgaZrYOvKEbHnQcdP+yPUuQSLTtaThVY1jzyaSLXk=; b=HU5ADUqvqHLwcYZzZ7aUvLCG+WPznFYKCO5ujtBeH7dAN3II4AX3HxClWIwKzp1vlzyJPK tpeFZmID+LXkcXkCGvgQltHE1MYTeL6bEgmFXNcAXOTUYlgHx4liMgE/wobumqG7WPLqlc fk7hxNInTh/GjpBtaDIW/uVRHIdJ57k= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf15.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707928887; a=rsa-sha256; cv=none; b=A3+nH2UOei3oH1lnMmombL6CsHcQ7uPEjdU/vTo500HeZmHUX5c23CjhZ/IZamsOQhauMJ HnexhFwRFdCrTkwoZuqp+EbvDbUWUu+cPwS2ucE3iiQkuQrY1Y4+BohNWtcB047DjgbMDQ 7w613eC4hrktA+y1zFPnkwtReJcEEu4= 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 6A7031FB; Wed, 14 Feb 2024 08:42:07 -0800 (PST) Received: from [10.57.64.120] (unknown [10.57.64.120]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C380B3F762; Wed, 14 Feb 2024 08:41:23 -0800 (PST) Message-ID: <78095f74-dc1c-4425-b390-fb6307a6e429@arm.com> Date: Wed, 14 Feb 2024 16:41:22 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 5/7] mm: thp: split huge page to any lower order pages (except order-1). Content-Language: en-GB To: Zi Yan Cc: "Pankaj Raghav (Samsung)" , linux-mm@kvack.org, "Matthew Wilcox (Oracle)" , David Hildenbrand , Yang Shi , Yu Zhao , "Kirill A . Shutemov" , =?UTF-8?Q?Michal_Koutn=C3=BD?= , Roman Gushchin , Zach O'Keefe , Hugh Dickins , Mcgrof Chamberlain , Andrew Morton , linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org References: <20240213215520.1048625-1-zi.yan@sent.com> <20240213215520.1048625-6-zi.yan@sent.com> <6859C8DA-5B7F-458E-895C-763BA782F4B9@nvidia.com> <6c986b83-e00d-46fe-8c88-374f8e6bd0fa@arm.com> <5D3CF5B4-FB16-4CE7-9D8E-CBFFA7A1FA43@nvidia.com> From: Ryan Roberts In-Reply-To: <5D3CF5B4-FB16-4CE7-9D8E-CBFFA7A1FA43@nvidia.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 4626FA0029 X-Stat-Signature: t8eosm5ibhi4da4d1ajir5uesd61yncj X-HE-Tag: 1707928887-289787 X-HE-Meta: U2FsdGVkX1+WPgv3CkpQty6SvjDSX+lWM3AwrlkO9URPVh+iI6CdRv2qmnbSbvmrsooBjlhVBd+/E92KMRWWpDCOknTEEydSVYhFo69IwFeu5sUkL56LCjQwU94yDB9HHMMqFrICZy0f9qkqAwgOauraGjJHCQxEcqCmRktIp98gm1259XcnyGQVXCItIMzFm89mIT7qeftGH/X3WKHSjMyrp3zIznLrTmMOSyZPwz/Co76YBy6+B7mMLwpzP7HKC6t8evrnJQM6OOQUpPdrMb5sOPmjdWVVRmbr9/7kTbv2sxrbzIEwWu6taJFcC/UylmWZYYg9zppXITSoJKNgiQDx9i4qYBnuDeKOWBunkqU18XMPlvhFFzje8fZKfLP/LNDDashDeNbg7pvrjAdv7Me3cDatzDWuwzS1c8vIXruiPcGTVJ/MKNgWwQNFmrDqM1QQV0Sv9tB0pMXfoRcRbNpXGqTaktJ7rKQh9C+hqIhxMc9pUbkwJNcZqngTlaVxvojBJTgdRMdvnOftCBHSxEvqjg+niA28SldLygnn1k5PY3XgJ2dRsiMdc2rSgv3Ra1xORa/j1cbO5Bp/YdD+gtO7RHpbAqpYayMue91o6BUPUQd25NPpH5Ripm4s6iWAUgm06ySPW9Mci8/IgY4EMfJ78Q475w2qCqLS1ulZsLwHRnx9OPpBgHAWzE7N5JL3Ow0rFsY/3yIvGUWcGAqRVlYBaYuY+3Utr2gNFD8tzLs16iut87X/DSemg2B9CMJ/Q7dsJJDlt2P/by12q6P6OoDLwlwewE1aqvqvP/adBcAMvVBe597/1zijPfoRn7d4/83v/e9JlYrQmHKylFBFWlHamDtq3dfU5g6N8h65yMFVqmTzoW6svQVvp8MHaYMpX8HJAWJvxLsdPbg5w73L6XNRgZiwy8CTIuuTYuKT1lxzKXgG3YPLhU8OAGrJuDPByg6pcKIg19QOAUS1etq /qF427hL f1zjd5JaFYb3IcH31y1Rl9UXKfVDv+9CFJcQJXTDj9K8ShU3lYZODDBmCFv/+Mk0UX0MefdxWECHYU0+XgjLtx04/iXECt8rNTvTnYkntJJ7vRZoWoIhRCSeTc+D0sg2EvdxXhA53VIrJ6QtqRbMj+5kcaUAwEipmNNofgrmENfkeI6w= 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 14/02/2024 16:28, Zi Yan wrote: > On 14 Feb 2024, at 11:22, Ryan Roberts wrote: > >> On 14/02/2024 16:11, Zi Yan wrote: >>> On 14 Feb 2024, at 5:38, Ryan Roberts wrote: >>> >>>> On 13/02/2024 21:55, Zi Yan wrote: >>>>> From: Zi Yan >>>>> >>>>> To split a THP to any lower order (except order-1) pages, we need to >>>>> reform THPs on subpages at given order and add page refcount based on the >>>>> new page order. Also we need to reinitialize page_deferred_list after >>>>> removing the page from the split_queue, otherwise a subsequent split will >>>>> see list corruption when checking the page_deferred_list again. >>>>> >>>>> It has many uses, like minimizing the number of pages after >>>>> truncating a huge pagecache page. For anonymous THPs, we can only split >>>>> them to order-0 like before until we add support for any size anonymous >>>>> THPs. >>>> >>>> multi-size THP is now upstream. Not sure if this comment still makes sense. >>> Will change it to reflect the fact that multi-size THP is already upstream. >>> >>>> Still its not completely clear to me how you would integrate this new machinery >>>> and decide what non-zero order to split anon THP to? >>> >>> Originally, it was developed along with my 1GB THP support. So it was intended >>> to split order-18 to order-9. But for now, like you and David said in the cover >>> letter email thread, we might not want to use it for anonymous large folios >>> until we find a necessary use case. >>> >>>>> >>>>> Order-1 folio is not supported because _deferred_list, which is used by >>>>> partially mapped folios, is stored in subpage 2 and an order-1 folio only >>>>> has subpage 0 and 1. >>>>> >>>>> Signed-off-by: Zi Yan >>>>> --- >>>>> include/linux/huge_mm.h | 21 +++++--- >>>>> mm/huge_memory.c | 114 +++++++++++++++++++++++++++++++--------- >>>>> 2 files changed, 101 insertions(+), 34 deletions(-) >>>>> >>>>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h >>>>> index 5adb86af35fc..de0c89105076 100644 >>>>> --- a/include/linux/huge_mm.h >>>>> +++ b/include/linux/huge_mm.h >>>>> @@ -265,10 +265,11 @@ unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr, >>>>> >>>>> void folio_prep_large_rmappable(struct folio *folio); >>>>> bool can_split_folio(struct folio *folio, int *pextra_pins); >>>>> -int split_huge_page_to_list(struct page *page, struct list_head *list); >>>>> +int split_huge_page_to_list_to_order(struct page *page, struct list_head *list, >>>>> + unsigned int new_order); >>>>> static inline int split_huge_page(struct page *page) >>>>> { >>>>> - return split_huge_page_to_list(page, NULL); >>>>> + return split_huge_page_to_list_to_order(page, NULL, 0); >>>>> } >>>>> void deferred_split_folio(struct folio *folio); >>>>> >>>>> @@ -422,7 +423,8 @@ can_split_folio(struct folio *folio, int *pextra_pins) >>>>> return false; >>>>> } >>>>> static inline int >>>>> -split_huge_page_to_list(struct page *page, struct list_head *list) >>>>> +split_huge_page_to_list_to_order(struct page *page, struct list_head *list, >>>>> + unsigned int new_order) >>>>> { >>>>> return 0; >>>>> } >>>>> @@ -519,17 +521,20 @@ static inline bool thp_migration_supported(void) >>>>> } >>>>> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ >>>>> >>>>> -static inline int split_folio_to_list(struct folio *folio, >>>>> - struct list_head *list) >>>>> +static inline int split_folio_to_list_to_order(struct folio *folio, >>>>> + struct list_head *list, int new_order) >>>>> { >>>>> - return split_huge_page_to_list(&folio->page, list); >>>>> + return split_huge_page_to_list_to_order(&folio->page, list, new_order); >>>>> } >>>>> >>>>> -static inline int split_folio(struct folio *folio) >>>>> +static inline int split_folio_to_order(struct folio *folio, int new_order) >>>>> { >>>>> - return split_folio_to_list(folio, NULL); >>>>> + return split_folio_to_list_to_order(folio, NULL, new_order); >>>>> } >>>>> >>>>> +#define split_folio_to_list(f, l) split_folio_to_list_to_order(f, l, 0) >>>>> +#define split_folio(f) split_folio_to_order(f, 0) >>>>> + >>>>> /* >>>>> * 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 >>>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >>>>> index ad7133c97428..d0e555a8ea98 100644 >>>>> --- a/mm/huge_memory.c >>>>> +++ b/mm/huge_memory.c >>>>> @@ -2718,11 +2718,14 @@ void vma_adjust_trans_huge(struct vm_area_struct *vma, >>>>> >>>>> static void unmap_folio(struct folio *folio) >>>>> { >>>>> - enum ttu_flags ttu_flags = TTU_RMAP_LOCKED | TTU_SPLIT_HUGE_PMD | >>>>> - TTU_SYNC | TTU_BATCH_FLUSH; >>>>> + enum ttu_flags ttu_flags = TTU_RMAP_LOCKED | TTU_SYNC | >>>>> + TTU_BATCH_FLUSH; >>>>> >>>>> VM_BUG_ON_FOLIO(!folio_test_large(folio), folio); >>>>> >>>>> + if (folio_test_pmd_mappable(folio)) >>>>> + ttu_flags |= TTU_SPLIT_HUGE_PMD; >>>> >>>> Should we split this change out? I think it makes sense independent of this series? >>>> >>> >>> Sure. Since multi-size THP is upstream, this avoid unnecessary code path if >>> the THP is not PMD-mapped. >>> >>>>> + >>>>> /* >>>>> * Anon pages need migration entries to preserve them, but file >>>>> * pages can simply be left unmapped, then faulted back on demand. >>>>> @@ -2756,7 +2759,6 @@ static void lru_add_page_tail(struct page *head, struct page *tail, >>>>> struct lruvec *lruvec, struct list_head *list) >>>>> { >>>>> VM_BUG_ON_PAGE(!PageHead(head), head); >>>>> - VM_BUG_ON_PAGE(PageCompound(tail), head); >>>>> VM_BUG_ON_PAGE(PageLRU(tail), head); >>>>> lockdep_assert_held(&lruvec->lru_lock); >>>>> >>>>> @@ -2777,7 +2779,8 @@ static void lru_add_page_tail(struct page *head, struct page *tail, >>>>> } >>>>> >>>>> static void __split_huge_page_tail(struct folio *folio, int tail, >>>>> - struct lruvec *lruvec, struct list_head *list) >>>>> + struct lruvec *lruvec, struct list_head *list, >>>>> + unsigned int new_order) >>>>> { >>>>> struct page *head = &folio->page; >>>>> struct page *page_tail = head + tail; >>>>> @@ -2847,10 +2850,15 @@ static void __split_huge_page_tail(struct folio *folio, int tail, >>>>> * which needs correct compound_head(). >>>>> */ >>>>> clear_compound_head(page_tail); >>>>> + if (new_order) { >>>>> + prep_compound_page(page_tail, new_order); >>>>> + folio_prep_large_rmappable(page_folio(page_tail)); >>>>> + } >>>>> >>>>> /* Finally unfreeze refcount. Additional reference from page cache. */ >>>>> - page_ref_unfreeze(page_tail, 1 + (!folio_test_anon(folio) || >>>>> - folio_test_swapcache(folio))); >>>>> + page_ref_unfreeze(page_tail, >>>>> + 1 + ((!folio_test_anon(folio) || folio_test_swapcache(folio)) ? >>>>> + folio_nr_pages(page_folio(page_tail)) : 0)); >>>>> >>>>> if (folio_test_young(folio)) >>>>> folio_set_young(new_folio); >>>>> @@ -2868,7 +2876,7 @@ static void __split_huge_page_tail(struct folio *folio, int tail, >>>>> } >>>>> >>>>> static void __split_huge_page(struct page *page, struct list_head *list, >>>>> - pgoff_t end) >>>>> + pgoff_t end, unsigned int new_order) >>>>> { >>>>> struct folio *folio = page_folio(page); >>>>> struct page *head = &folio->page; >>>>> @@ -2877,10 +2885,11 @@ static void __split_huge_page(struct page *page, struct list_head *list, >>>>> unsigned long offset = 0; >>>>> unsigned int nr = thp_nr_pages(head); >>>>> int i, nr_dropped = 0; >>>>> + unsigned int new_nr = 1 << new_order; >>>>> int order = folio_order(folio); >>>>> >>>>> /* complete memcg works before add pages to LRU */ >>>>> - split_page_memcg(head, order, 0); >>>>> + split_page_memcg(head, order, new_order); >>>>> >>>>> if (folio_test_anon(folio) && folio_test_swapcache(folio)) { >>>>> offset = swp_offset(folio->swap); >>>>> @@ -2893,8 +2902,8 @@ static void __split_huge_page(struct page *page, struct list_head *list, >>>>> >>>>> ClearPageHasHWPoisoned(head); >>>>> >>>>> - for (i = nr - 1; i >= 1; i--) { >>>>> - __split_huge_page_tail(folio, i, lruvec, list); >>>>> + for (i = nr - new_nr; i >= new_nr; i -= new_nr) { >>>>> + __split_huge_page_tail(folio, i, lruvec, list, new_order); >>>>> /* Some pages can be beyond EOF: drop them from page cache */ >>>>> if (head[i].index >= end) { >>>>> struct folio *tail = page_folio(head + i); >>>>> @@ -2910,29 +2919,41 @@ static void __split_huge_page(struct page *page, struct list_head *list, >>>>> __xa_store(&head->mapping->i_pages, head[i].index, >>>>> head + i, 0); >>>>> } else if (swap_cache) { >>>>> + /* >>>>> + * split anonymous THPs (including swapped out ones) to >>>>> + * non-zero order not supported >>>>> + */ >>>>> + VM_WARN_ONCE(new_order, >>>>> + "Split swap-cached anon folio to non-0 order not supported"); >>>> >>>> Why isn't it supported? Even if it's not supported, is this level the right >>>> place to enforce these kinds of policy decisions? I wonder if we should be >>>> leaving that to the higher level to decide? >>> >>> Is the swap-out small-size THP without splitting merged? This needs that patchset. >> >> No not yet. I have to respin it. Its on my todo list. >> >> I'm not sure I understand the dependency though? > > IIUC, swap cache only supports one cluster size, HPAGE_PMD_NR, so splitting > a PMD-size swapcached folio will need to split a cluster to smaller ones, which > needs your patchset support. Let me know if I get it wrong. Ahh yeah, sorry, obvious now that you've spelled it out - thanks! > >> >>> You are right that a warning here is not appropriate. I will fail the splitting >>> if the folio is swapcached and going to be split into >0 order. >>> >>>>> __xa_store(&swap_cache->i_pages, offset + i, >>>>> head + i, 0); >>>>> } >>>>> } >>>>> >>> >>> >>> -- >>> Best Regards, >>> Yan, Zi > > > -- > Best Regards, > Yan, Zi