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 17955C52D7B for ; Wed, 14 Aug 2024 11:20:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A14296B0085; Wed, 14 Aug 2024 07:20:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 975136B0088; Wed, 14 Aug 2024 07:20:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 815AB6B0089; Wed, 14 Aug 2024 07:20:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 568A66B0085 for ; Wed, 14 Aug 2024 07:20:34 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 06866120E50 for ; Wed, 14 Aug 2024 11:20:33 +0000 (UTC) X-FDA: 82450607988.10.731821D Received: from mail-vk1-f178.google.com (mail-vk1-f178.google.com [209.85.221.178]) by imf21.hostedemail.com (Postfix) with ESMTP id 11C8F1C0005 for ; Wed, 14 Aug 2024 11:20:31 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf21.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.178 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723634379; a=rsa-sha256; cv=none; b=cz7g2gz5zAyPbjhNJQ6XLnwJzJ7WT/fOyvpU6JLx4JQZbedCrCJl2sbPWnus/fdi10DTc5 bhdNWkNCfS7DyehRpxgQiXtOfX2CY36jIyQhNjwQMVYqfv8ssFCI6kQCnelEWznQQo0hAP 8E9ixqVgZfoAl7yKbIzfGdm8sdIBwN0= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf21.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.178 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723634379; 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=wT8/yxCSMEel7SFnMznHXl6VK4SxHxcbluAHjtHcqO8=; b=eHMEDf4+MAJBT0D4Ym2i9Wipdf/kg54DxUtiqnIFzwJxpzBQx2HRJBoOSFUX0Njo8e3y6/ dKK1kaKGt3WMTEl7UCbrtf/PK4DduZpCbNKRqs2Dss5+xG87ovE8Ho9cC6ZwqdwLb11XlS snV3k7YoaNVJ9QNay47Irvisk4fIMIg= Received: by mail-vk1-f178.google.com with SMTP id 71dfb90a1353d-4f50d8ce6e0so2375336e0c.0 for ; Wed, 14 Aug 2024 04:20:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723634431; x=1724239231; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=wT8/yxCSMEel7SFnMznHXl6VK4SxHxcbluAHjtHcqO8=; b=VmMx+k7dL/jmiT7UxGaFxxgrxCMvKHYB1bQsl/FTJMnno9xJ7UoJttxg7OwXAzYscV iCRUFioLLYKbHsDn1AtAhW4AtfhFf127FLUgWdwuDtB+bj8Rynvpc2tOLVnKOjnryJbW 7YeQonu3RF0t0O2ZeO6oVvP6rvRerpW03hv1lF17TPCWb2QdiEb9jOAAdB4iNGKWgV/r ciJCEcVlkh6gHnbar+TZ5t/Z2S/AH0Yh1v9TSJiEKB/xYmdrqwGMyjdcOUwSACWkeeoW 7tMqW/Pjo3USw90dICaAE4wBBOfIeN4E7wruRr1jQ0UUtdE+Yu2gob8onLah0oo6NKmO 87nw== X-Forwarded-Encrypted: i=1; AJvYcCXFO9iXSEUCfICl1Yh6xbaIs0eoxbud3WlwImm7MqJ3fjLt7qCbw8X9b2pyjcrM9HSk3CISocpjWefQ6isnEEKf+RM= X-Gm-Message-State: AOJu0YyUaCpIoAusr3dDXSWNBiuwWH15D/Y7qhro2XUfvF8U4xWPUIK1 SgCekNyxPkB3Wj7peIkN1DQkB+3EoEemhYk1g3WggyWyBbgDx7VEA+4Ez9rqld6zlg9Tyu6a7zu 56nRf4V9RJO3fMFgqbsEdby50Qe4= X-Google-Smtp-Source: AGHT+IHu4oujxvSrHv7gWoSezqByQGV6UEbbHSBKelGeoAa0pI/ooMvt8mHWIk+5v8opzsRKxJLHBnrWZalaZ5rnalE= X-Received: by 2002:a05:6122:130a:b0:4ed:12b:ec99 with SMTP id 71dfb90a1353d-4fad1b0fdafmr3105204e0c.3.1723634430926; Wed, 14 Aug 2024 04:20:30 -0700 (PDT) MIME-Version: 1.0 References: <20240813120328.1275952-1-usamaarif642@gmail.com> <20240813120328.1275952-5-usamaarif642@gmail.com> <925804e4-0b33-45eb-905d-e00f67192828@gmail.com> In-Reply-To: <925804e4-0b33-45eb-905d-e00f67192828@gmail.com> From: Barry Song Date: Wed, 14 Aug 2024 23:20:19 +1200 Message-ID: Subject: Re: [PATCH v3 4/6] mm: Introduce a pageflag for partially mapped folios To: Usama Arif Cc: akpm@linux-foundation.org, linux-mm@kvack.org, hannes@cmpxchg.org, riel@surriel.com, shakeel.butt@linux.dev, roman.gushchin@linux.dev, yuzhao@google.com, david@redhat.com, ryan.roberts@arm.com, rppt@kernel.org, willy@infradead.org, cerasuolodomenico@gmail.com, corbet@lwn.net, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, kernel-team@meta.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 11C8F1C0005 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: d5tkoeok4daix49izhs4nx7yimuzgds9 X-HE-Tag: 1723634431-860330 X-HE-Meta: U2FsdGVkX1/27WLM4mQO7J5vmITMzd8MBdfDmPbAuOcfyyohq5mUro3Ra6NazNxlV0Uoe4Y1rQXtigqMIhkkQhtbiKGVwt6ksa+RFFNDNe20Uq6k3uKPN0qTylhZWBdMh/VULlL35ZWFvF/xuakPl6XZSvDg4WR2xYE65xAfJbrEC7pg5Hqs8Bwd/WtwNET8YmSH12OZHkHyaoio+71zNRn/+qQHHbenenS1ntcJOXGKOiwZgz8VYoVdh02pEF/i5zeEc/5Lkdb/y+Ex+KqH2ooXTE1dV6YTnrkBfKEiGmbsbuL7ESPIcieMYmHXVbkiwptBclCqniXOPubus3iQchwCFid+yQGPQ6liDexwvgCVykL5AEU5GXz6fBK8Lq0sG0I6unbxzlP37gky2lPtXgRKNuqpCKJzrNw/USQEjkqzv8WYojg85U6Xp5zoc9yfwFemr6I5rijPNN27gHmmuoRSTSLJaJpu56fFOGouE7bvwt0dS/yAZhP4d9TD8U/HOi5d9njJwHhEp/a8xx8xrMV252Pd89G7IXE+A+neY2wmmHe6dn/8/HH+WbmR9LW8Wz4P4JDxZDL1rCZPJFcyCbvRbsBR65Uum9hKWcOCXEQYdU885Ty263dqAXed4qtq6Uw0iyGylSZKw/Kb8rN2VedlF6stWgeRGqPhWdwVya5PQOzHW/5IJIJB2ApHitpaziU1EDgURueP6Ku1Xlt6DDfgqFuSgw4wDl74EZWqiafzlGqifflukfpAyngq9nxiuKWOLYcSLkM2jdgCRfM1zNUqPyS30+HCw9E69jhQ2pni084WFP/wjAbw792u1YPbiPJ6E1J/3IZKHYyX9aK+84JoOHjnTkkredoC40DbcRvdmtaDQaEUEG8ziZ7FQZLw6Gjh/Y7RtPKi88sN0a9fJMMiSDGjHQtleHmjLrATIGG2BzXWHg6LdHhMKyo1WZMWp2Mzm2qysX7phv1vP9w T3X9/mvf TQz7w+EyOOhgqMrboFI4Va+wjIT23sKTxRKp4CbxQeWKVasb9MT9RSwfUiyE/TvcZuwDZ0rZNo5CMqi5th0dfMDOgyXOoL2iEhrZw7QEE7OTX+hnqJFTItJFdgxsaF5JCcSxr7vMa1Zwz4ue5nXZdK7dd/D/M8K0KN2puJbTcZDh+D7wkavPEu6J4zzPHWsuhfeR/xYGJ9RHLXrwM5k9zdGWapEzcSFPE6CZwy/GAws9NBYYl3kjyzW2b+oJodRbXZAWSWuMbKQe0gF5zCPHLswlLZsjoxuzf8ukANvp27AdHe6M= 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 Wed, Aug 14, 2024 at 11:11=E2=80=AFPM Usama Arif wrote: > > > > On 14/08/2024 11:44, Barry Song wrote: > > On Wed, Aug 14, 2024 at 12:03=E2=80=AFAM Usama Arif wrote: > >> > >> Currently folio->_deferred_list is used to keep track of > >> partially_mapped folios that are going to be split under memory > >> pressure. In the next patch, all THPs that are faulted in and collapse= d > >> by khugepaged are also going to be tracked using _deferred_list. > >> > >> This patch introduces a pageflag to be able to distinguish between > >> partially mapped folios and others in the deferred_list at split time = in > >> deferred_split_scan. Its needed as __folio_remove_rmap decrements > >> _mapcount, _large_mapcount and _entire_mapcount, hence it won't be > >> possible to distinguish between partially mapped folios and others in > >> deferred_split_scan. > >> > >> Eventhough it introduces an extra flag to track if the folio is > >> partially mapped, there is no functional change intended with this > >> patch and the flag is not useful in this patch itself, it will > >> become useful in the next patch when _deferred_list has non partially > >> mapped folios. > >> > >> Signed-off-by: Usama Arif > >> --- > >> include/linux/huge_mm.h | 4 ++-- > >> include/linux/page-flags.h | 3 +++ > >> mm/huge_memory.c | 21 +++++++++++++-------- > >> mm/hugetlb.c | 1 + > >> mm/internal.h | 4 +++- > >> mm/memcontrol.c | 3 ++- > >> mm/migrate.c | 3 ++- > >> mm/page_alloc.c | 5 +++-- > >> mm/rmap.c | 3 ++- > >> mm/vmscan.c | 3 ++- > >> 10 files changed, 33 insertions(+), 17 deletions(-) > >> > >> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > >> index 4c32058cacfe..969f11f360d2 100644 > >> --- a/include/linux/huge_mm.h > >> +++ b/include/linux/huge_mm.h > >> @@ -321,7 +321,7 @@ static inline int split_huge_page(struct page *pag= e) > >> { > >> return split_huge_page_to_list_to_order(page, NULL, 0); > >> } > >> -void deferred_split_folio(struct folio *folio); > >> +void deferred_split_folio(struct folio *folio, bool partially_mapped)= ; > >> > >> void __split_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd, > >> unsigned long address, bool freeze, struct folio *foli= o); > >> @@ -495,7 +495,7 @@ static inline int split_huge_page(struct page *pag= e) > >> { > >> return 0; > >> } > >> -static inline void deferred_split_folio(struct folio *folio) {} > >> +static inline void deferred_split_folio(struct folio *folio, bool par= tially_mapped) {} > >> #define split_huge_pmd(__vma, __pmd, __address) \ > >> do { } while (0) > >> > >> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > >> index a0a29bd092f8..cecc1bad7910 100644 > >> --- a/include/linux/page-flags.h > >> +++ b/include/linux/page-flags.h > >> @@ -182,6 +182,7 @@ enum pageflags { > >> /* At least one page in this folio has the hwpoison flag set *= / > >> PG_has_hwpoisoned =3D PG_active, > >> PG_large_rmappable =3D PG_workingset, /* anon or file-backed *= / > >> + PG_partially_mapped, /* was identified to be partially mapped = */ > >> }; > >> > >> #define PAGEFLAGS_MASK ((1UL << NR_PAGEFLAGS) - 1) > >> @@ -861,8 +862,10 @@ static inline void ClearPageCompound(struct page = *page) > >> ClearPageHead(page); > >> } > >> FOLIO_FLAG(large_rmappable, FOLIO_SECOND_PAGE) > >> +FOLIO_FLAG(partially_mapped, FOLIO_SECOND_PAGE) > >> #else > >> FOLIO_FLAG_FALSE(large_rmappable) > >> +FOLIO_FLAG_FALSE(partially_mapped) > >> #endif > >> > >> #define PG_head_mask ((1UL << PG_head)) > >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c > >> index 6df0e9f4f56c..c024ab0f745c 100644 > >> --- a/mm/huge_memory.c > >> +++ b/mm/huge_memory.c > >> @@ -3397,6 +3397,7 @@ int split_huge_page_to_list_to_order(struct page= *page, struct list_head *list, > >> * page_deferred_list. > >> */ > >> list_del_init(&folio->_deferred_list); > >> + folio_clear_partially_mapped(folio); > >> } > >> spin_unlock(&ds_queue->split_queue_lock); > >> if (mapping) { > >> @@ -3453,11 +3454,12 @@ void __folio_undo_large_rmappable(struct folio= *folio) > >> if (!list_empty(&folio->_deferred_list)) { > >> ds_queue->split_queue_len--; > >> list_del_init(&folio->_deferred_list); > >> + folio_clear_partially_mapped(folio); > >> } > >> spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags); > >> } > >> > >> -void deferred_split_folio(struct folio *folio) > >> +void deferred_split_folio(struct folio *folio, bool partially_mapped) > >> { > >> struct deferred_split *ds_queue =3D get_deferred_split_queue(f= olio); > >> #ifdef CONFIG_MEMCG > >> @@ -3485,14 +3487,17 @@ void deferred_split_folio(struct folio *folio) > >> if (folio_test_swapcache(folio)) > >> return; > >> > >> - if (!list_empty(&folio->_deferred_list)) > >> - return; > >> - > >> spin_lock_irqsave(&ds_queue->split_queue_lock, flags); > >> + if (partially_mapped) > >> + folio_set_partially_mapped(folio); > >> + else > >> + folio_clear_partially_mapped(folio); > > > > Hi Usama, > > > > Do we need this? When can a partially_mapped folio on deferred_list bec= ome > > non-partially-mapped and need a clear? I understand transferring from > > entirely_map > > to partially_mapped is a one way process? partially_mapped folios can't= go back > > to entirely_mapped? > > > Hi Barry, > > deferred_split_folio function is called in 3 places after this series, at= fault, collapse and partial mapping. partial mapping can only happen after= fault/collapse, and we have FOLIO_FLAG_FALSE(partially_mapped), i.e. flag = initialized to false, so technically its not needed. A partially_mapped fol= io on deferred list wont become non-partially mapped. > > I just did it as a precaution if someone ever changes the kernel and call= s deferred_split_folio with partially_mapped set to false after it had been= true. The function arguments of deferred_split_folio make it seem that pas= sing partially_mapped=3Dfalse as an argument would clear it, which is why I= cleared it as well. I could change the patch to something like below if it= makes things better? i.e. add a comment at the top of the function: > to me, it seems a BUG to call with false after a folio has been partially_mapped. So I'd rather VM_WARN_ON_FOLIO(folio_test_partially_mapped(folio), folio); The below should also fix the MTHP_STAT_SPLIT_DEFERRED counter this patch is breaking? @@ -3515,16 +3522,18 @@ void deferred_split_folio(struct folio *folio, bool partially_mapped) return; spin_lock_irqsave(&ds_queue->split_queue_lock, flags); - if (partially_mapped) - folio_set_partially_mapped(folio); - else - folio_clear_partially_mapped(folio); - if (list_empty(&folio->_deferred_list)) { - if (partially_mapped) { + if (partially_mapped) { + if (!folio_test_set_partially_mapped(folio)) { + mod_mthp_stat(folio_order(folio), + MTHP_STAT_NR_SPLIT_DEFERRED, 1); if (folio_test_pmd_mappable(folio)) count_vm_event(THP_DEFERRED_SPLIT_PAGE); count_mthp_stat(folio_order(folio), MTHP_STAT_SPLIT_DEFERRED); } + } + VM_WARN_ON_FOLIO(folio_test_partially_mapped(folio), folio); + + if (list_empty(&folio->_deferred_list)) { list_add_tail(&folio->_deferred_list, &ds_queue->split_queu= e); ds_queue->split_queue_len++; #ifdef CONFIG_MEMCG > > -void deferred_split_folio(struct folio *folio) > +/* partially_mapped=3Dfalse won't clear PG_partially_mapped folio flag *= / > +void deferred_split_folio(struct folio *folio, bool partially_mapped) > { > struct deferred_split *ds_queue =3D get_deferred_split_queue(foli= o); > #ifdef CONFIG_MEMCG > @@ -3485,14 +3488,15 @@ void deferred_split_folio(struct folio *folio) > if (folio_test_swapcache(folio)) > return; > > - if (!list_empty(&folio->_deferred_list)) > - return; > - > spin_lock_irqsave(&ds_queue->split_queue_lock, flags); > + if (partially_mapped) > + folio_set_partially_mapped(folio); > if (list_empty(&folio->_deferred_list)) { > - if (folio_test_pmd_mappable(folio)) > - count_vm_event(THP_DEFERRED_SPLIT_PAGE); > - count_mthp_stat(folio_order(folio), MTHP_STAT_SPLIT_DEFER= RED); > + if (partially_mapped) { > + if (folio_test_pmd_mappable(folio)) > + count_vm_event(THP_DEFERRED_SPLIT_PAGE); > + count_mthp_stat(folio_order(folio), MTHP_STAT_SPL= IT_DEFERRED); > + } > list_add_tail(&folio->_deferred_list, &ds_queue->split_qu= eue); > ds_queue->split_queue_len++; > #ifdef CONFIG_MEMCG > > > > I am trying to rebase my NR_SPLIT_DEFERRED counter on top of your > > work, but this "clear" makes that job quite tricky. as I am not sure > > if this patch > > is going to clear the partially_mapped flag for folios on deferred_list= . > > > > Otherwise, I can simply do the below whenever folio is leaving deferred= _list > > > > ds_queue->split_queue_len--; > > list_del_init(&folio->_deferred_list); > > if (folio_test_clear_partially_mapped(folio)) > > mod_mthp_stat(folio_order(folio), > > MTHP_STAT_NR_SPLIT_DEFERRED, -1); > > > Thanks Barry