Re: A potential refcount issue during __folio_split

[Wei Yang <richard.weiyang@gmail.com>]

On Mon, Feb 23, 2026 at 11:00:01PM -0500, Zi Yan wrote:
>On 23 Feb 2026, at 6:59, Wei Yang wrote:
>
>> On Mon, Feb 23, 2026 at 10:23:11AM +0100, David Hildenbrand (Arm) wrote:
>>>> BTW, in the folio world, I do not think it is possible to perform the aforementioned
>>>> split_huge_page_to_list_to_order() pattern any more, since you always work on folio,
>>>> the head. Unless there is a need of getting hold of a tail after-split folio after
>>>> a folio split, the pattern would be:
>>>>
>>>> tail_page = folio_page(folio, N);
>>>>
>>>> folio_get(folio);
>>>> folio_lock(folio);
>>>> folio_split(folio, ..., /* new parameter: lock_at = */ tail_page, ...);
>>>> tail_folio = page_folio(tail_page);
>>>> folio_unlock(tail_folio);
>>>> folio_put(tail_folio);
>>>
>>
>> Missed this. Agree.
>>
>>> Agreed. Maybe it would be even nicer if the split function could return the
>>> new folio directly.
>>>
>>> folio_get(folio);
>>> folio_lock(folio);
>>> split_folio = folio_split_XXX(folio, ..., tail_page, ...);
>>> if (IS_ERR_VALUE(split_folio)) {
>>> 	...
>>> }
>>> folio_unlock(split_folio);
>>> folio_put(split__folio);
>>>
>>
>> I am afraid it would be complicated?
>>
>> Well, we don't have this usecase now, could decide it when we do need it.
>
>The patch below should work, but for now, since we do not have any user,
>it is better to update the comment and add a check to make sure @lock_at
>always points to the head page if @list is not NULL.
>

Agree.

>From 66e24e6cc4397caa134f5600d22d77fdb9b58049 Mon Sep 17 00:00:00 2001
>From: Zi Yan <ziy@nvidia.com>
>Date: Mon, 23 Feb 2026 21:59:18 -0500
>Subject: [PATCH] mm/huge_memory: allow caller to unlock any subpage of a folio
> after split
>
>Transfer to-be-split folio's reference to an after-split folio that caller
>wants to unlock and put.
>
>Also let __folio_split() return the folio containing @lock_at for caller to
>use.
>
>Signed-off-by: Zi Yan <ziy@nvidia.com>
>---
> mm/huge_memory.c | 65 ++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 47 insertions(+), 18 deletions(-)
>
>diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>index 0d487649e4de..d051d611c6e5 100644
>--- a/mm/huge_memory.c
>+++ b/mm/huge_memory.c
>@@ -3768,10 +3768,9 @@ static unsigned int folio_cache_ref_count(const struct folio *folio)
> }
>
> 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,
>-					     struct list_head *list, enum split_type split_type,
>-					     pgoff_t end, int *nr_shmem_dropped)
>+		struct page *split_at, struct page *lock_at, struct xa_state *xas,
>+		struct address_space *mapping, bool do_lru, struct list_head *list,
>+		enum split_type split_type, pgoff_t end, int *nr_shmem_dropped)
> {
> 	struct folio *end_folio = folio_next(folio);
> 	struct folio *new_folio, *next;
>@@ -3855,7 +3854,11 @@ static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int n
> 			folio_ref_unfreeze(new_folio,
> 					   folio_cache_ref_count(new_folio) + 1);
>
>-			if (do_lru)
>+			/*
>+			 * skip @lock_at since caller wants to unlock and put it
>+			 * after split
>+			 */
>+			if (do_lru && new_folio != page_folio(lock_at))
> 				lru_add_split_folio(folio, new_folio, lruvec, list);

This makes me thing whether we need to always grab lru lock.

If the folio has already been removed from lru, it looks not necessary?

Well this is another thing.

>
> 			/*
>@@ -3898,8 +3901,17 @@ static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int n
> 		 */
> 		folio_ref_unfreeze(folio, folio_cache_ref_count(folio) + 1);
>
>-		if (do_lru)
>+		if (do_lru) {
>+			/*
>+			 * caller wants to unlock and put @lock_at instead of
>+			 * @folio, treat @folio as other after-split folios
>+			 * by either elevating its refcount and putting it in
>+			 * @list or putting it back to lru if @list is NULL.
>+			 */
>+			if (folio != page_folio(lock_at))
>+				lru_add_split_folio(folio, folio, lruvec, list);
> 			unlock_page_lruvec(lruvec);
>+		}
>
> 		if (ci)
> 			swap_cluster_unlock(ci);
>@@ -3925,14 +3937,13 @@ static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int n
>  * preparing @folio for __split_unmapped_folio().
>  *
>  * After splitting, the after-split folio containing @lock_at remains locked
>- * and others are unlocked:
>- * 1. for uniform split, @lock_at points to one of @folio's subpages;
>- * 2. for buddy allocator like (non-uniform) split, @lock_at points to @folio.
>+ * and others are unlocked and the caller's folio reference is transferred to
>+ * @lock_at's folio. @lock_at can point to anyone of @folio's subpages.
>  *
>  * Return: 0 - successful, <0 - failed (if -ENOMEM is returned, @folio might be
>  * split but not to @new_order, the caller needs to check)
>  */
>-static int __folio_split(struct folio *folio, unsigned int new_order,
>+static struct folio* __folio_split(struct folio *folio, unsigned int new_order,
> 		struct page *split_at, struct page *lock_at,
> 		struct list_head *list, enum split_type split_type)
> {
>@@ -4052,8 +4063,10 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
> 		}
> 	}
>
>-	ret = __folio_freeze_and_split_unmapped(folio, new_order, split_at, &xas, mapping,
>-						true, list, split_type, end, &nr_shmem_dropped);
>+	ret = __folio_freeze_and_split_unmapped(folio, new_order, split_at,
>+						lock_at, &xas, mapping, true,
>+						list, split_type, end,
>+						&nr_shmem_dropped);
> fail:
> 	if (mapping)
> 		xas_unlock(&xas);
>@@ -4100,7 +4113,10 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
> 	if (old_order == HPAGE_PMD_ORDER)
> 		count_vm_event(!ret ? THP_SPLIT_PAGE : THP_SPLIT_PAGE_FAILED);
> 	count_mthp_stat(old_order, !ret ? MTHP_STAT_SPLIT : MTHP_STAT_SPLIT_FAILED);
>-	return ret;
>+
>+	if (!ret)
>+		return page_folio(lock_at);
>+	return (struct folio*)ERR_PTR(ret);
> }
>
> /**
>@@ -4138,9 +4154,10 @@ int folio_split_unmapped(struct folio *folio, unsigned int new_order)
> 		return -EAGAIN;
>
> 	local_irq_disable();
>-	ret = __folio_freeze_and_split_unmapped(folio, new_order, &folio->page, NULL,
>-						NULL, false, NULL, SPLIT_TYPE_UNIFORM,
>-						0, NULL);
>+	ret = __folio_freeze_and_split_unmapped(folio, new_order, &folio->page,
>+						&folio->page, NULL, NULL, false,
>+						NULL, SPLIT_TYPE_UNIFORM, 0,
>+						NULL);
> 	local_irq_enable();
> 	return ret;
> }
>@@ -4196,9 +4213,14 @@ int __split_huge_page_to_list_to_order(struct page *page, struct list_head *list
> 				     unsigned int new_order)
> {
> 	struct folio *folio = page_folio(page);
>+	struct folio *ret;
>
>-	return __folio_split(folio, new_order, &folio->page, page, list,
>+	ret = __folio_split(folio, new_order, &folio->page, page, list,
> 			     SPLIT_TYPE_UNIFORM);
>+	if (IS_ERR_VALUE(ret))
>+		return PTR_ERR(ret);
>+
>+	return 0;
> }
>
> /**
>@@ -4228,8 +4250,15 @@ int __split_huge_page_to_list_to_order(struct page *page, struct list_head *list
> int folio_split(struct folio *folio, unsigned int new_order,
> 		struct page *split_at, struct list_head *list)
> {
>-	return __folio_split(folio, new_order, split_at, &folio->page, list,
>+	struct folio *ret;
>+
>+	ret = __folio_split(folio, new_order, split_at, &folio->page, list,
> 			     SPLIT_TYPE_NON_UNIFORM);
>+
>+	if (IS_ERR_VALUE(ret))
>+		return PTR_ERR(ret);
>+
>+	return 0;
> }
>
> /**
>-- 
>2.51.0
>
>
>Best Regards,
>Yan, Zi

-- 
Wei Yang
Help you, Help me