All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Kiryl Shutsemau <kirill@shutemov.name>
Cc: akpm@linux-foundation.org, david@kernel.org, ziy@nvidia.com,
	 hannes@cmpxchg.org, usama.arif@linux.dev, lance.yang@linux.dev,
	 baolin.wang@linux.alibaba.com, liam@infradead.org,
	nico.pache@linux.dev, ryan.roberts@arm.com,  dev.jain@arm.com,
	baohua@kernel.org, kasong@tencent.com, hughd@google.com,
	 balbirs@nvidia.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,  kernel-team@meta.com,
	"Kiryl Shutsemau (Meta)" <kas@kernel.org>
Subject: Re: [PATCH] mm/huge_memory: add folio_reset_partially_mapped()
Date: Tue, 8 Sep 2026 10:25:07 +0100	[thread overview]
Message-ID: <ap_Tjt5vxal-B8DP@gremlin> (raw)
In-Reply-To: <20260907183340.1446132-1-kirill@shutemov.name>

On Mon, Sep 07, 2026 at 07:33:40PM +0100, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
>
> __folio_unqueue_deferred_split() and __folio_freeze_and_split_unmapped()
> both clear PG_partially_mapped and take the folio out of
> MTHP_STAT_NR_ANON_PARTIALLY_MAPPED with the same five lines.
>
> Move the block into folio_reset_partially_mapped() and call it from both
> places.
>
> The helper asserts what both callers rely on: the folio is frozen, so
> deferred_split_folio() cannot set the flag again under it, and the folio
> is already off the deferred split queue. The list check sits behind the
> flag test because order-1 folios have no _deferred_list.
>
> folio_order() is safe to use at this point in the split process: it
> still shows the pre-split order.
>
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Assisted-by: Claude-Code:claude-fable-5-1

Note that policy has changed on Assisted-by tags so this should be:

Assisted-by: LLM

https://docs.kernel.org/process/coding-assistants.html

> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>

One nit above and below, with those addressed LGTM, so:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  mm/huge_memory.c | 31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index dd66c6ad5af1..23ef22c2b5db 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3972,6 +3972,24 @@ static unsigned int folio_cache_ref_count(const struct folio *folio)
>  	return folio_nr_pages(folio);
>  }
>
> +static void folio_reset_partially_mapped(struct folio *folio)
> +{
> +	VM_WARN_ON_FOLIO(folio_ref_count(folio), folio);

NIT: I wonder if we could have a folio_is_frozen() that'd make this
self-documenting? As trivial as it'd be :)

If not, then a comment like:

	/* Folio must be frozen. */

Would be helpful.

> +
> +	if (!folio_test_partially_mapped(folio))
> +		return;
> +
> +	/*
> +	 * Order-1 folios have no _deferred_list. The flag is only ever set
> +	 * on folios that do, so the list can be checked after the flag.
> +	 */
> +	VM_WARN_ON_FOLIO(!list_empty(&folio->_deferred_list), folio);
> +
> +	folio_clear_partially_mapped(folio);
> +	mod_mthp_stat(folio_order(folio),
> +		      MTHP_STAT_NR_ANON_PARTIALLY_MAPPED, -1);
> +}
> +
>  static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int new_order,
>  					     struct page *split_at, struct xa_state *xas,
>  					     struct address_space *mapping, bool do_lru,
> @@ -3980,7 +3998,6 @@ static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int n
>  {
>  	struct folio *end_folio = folio_next(folio);
>  	struct folio *new_folio, *next;
> -	int old_order = folio_order(folio);
>  	int ret = 0;
>
>  	VM_WARN_ON_ONCE(!mapping && end);
> @@ -3998,11 +4015,7 @@ static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int n
>  		 * leaves PG_partially_mapped set.
>  		 * Clear it here: the flag does not survive the split.
>  		 */
> -		if (folio_test_partially_mapped(folio)) {
> -			folio_clear_partially_mapped(folio);
> -			mod_mthp_stat(old_order,
> -				      MTHP_STAT_NR_ANON_PARTIALLY_MAPPED, -1);
> -		}
> +		folio_reset_partially_mapped(folio);
>
>  		if (mapping) {
>  			int nr = folio_nr_pages(folio);
> @@ -4516,11 +4529,7 @@ bool __folio_unqueue_deferred_split(struct folio *folio)
>  	memcg = folio_memcg(folio);
>  	lru = list_lru_lock_irqsave(&deferred_split_lru, nid, &memcg, &flags);
>  	if (__list_lru_del(&deferred_split_lru, lru, &folio->_deferred_list, nid)) {
> -		if (folio_test_partially_mapped(folio)) {
> -			folio_clear_partially_mapped(folio);
> -			mod_mthp_stat(folio_order(folio),
> -				      MTHP_STAT_NR_ANON_PARTIALLY_MAPPED, -1);
> -		}
> +		folio_reset_partially_mapped(folio);

Nice bit of red :)

>  		unqueued = true;
>  	}
>  	list_lru_unlock_irqrestore(lru, &flags);
>
> base-commit: e3fc12b08aadde9cec7b3799ac0e0c9a1aa245c4
> --
> 2.54.0
>

--
Cheers, Lorenzo


  parent reply	other threads:[~2026-09-08  9:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-07 18:33 [PATCH] mm/huge_memory: add folio_reset_partially_mapped() Kiryl Shutsemau
2026-09-07 19:46 ` Zi Yan
2026-09-07 19:57 ` David Hildenbrand (Arm)
2026-09-08  6:06 ` Baolin Wang
2026-09-08  9:25 ` Lorenzo Stoakes (ARM) [this message]
2026-09-08 13:09   ` Kiryl Shutsemau
2026-09-08 13:33     ` Ilya Gladyshev
2026-09-08 11:12 ` Balbir Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ap_Tjt5vxal-B8DP@gremlin \
    --to=ljs@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=balbirs@nvidia.com \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=kas@kernel.org \
    --cc=kasong@tencent.com \
    --cc=kernel-team@meta.com \
    --cc=kirill@shutemov.name \
    --cc=lance.yang@linux.dev \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nico.pache@linux.dev \
    --cc=ryan.roberts@arm.com \
    --cc=usama.arif@linux.dev \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.