Re: [PATCH v4 05/16] mm/hugetlb: use direct assignment instead of folio_change_private()

["Zi Yan" <ziy@nvidia.com>]

On Wed Sep 16, 2026 at 2:12 AM EDT, David Hildenbrand (Arm) wrote:
> On 9/16/26 03:56, Zi Yan wrote:
>> On 13 Sep 2026, at 22:24, Zi Yan wrote:
>> 
>>> folio_change_private() should be used along with folio_attach_private() and
>>> folio_detach_private(), where adding and remove ->private content requires
>>> folio refcount change. add_hugetlb_folio() simply sets folio->private to
>>> NULL without refcount manipulation. Change it to direct assignment to avoid
>>> semantic confusion.
>>>
>>> It prepares for a future commit that remove PG_private.
>>>
>>> No functional change intended.
>>>
>>> Assisted-by: LLM
>>> To: Muchun Song <muchun.song@linux.dev>
>>> To: Oscar Salvador <osalvador@suse.de>
>>> To: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: David Hildenbrand <david@kernel.org>
>>> Cc: linux-mm@kvack.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Acked-by: Usama Arif <usama.arif@linux.dev>
>>> Reviewed-by: Gregory Price (Meta) <gourry@gourry.net>
>>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>>> ---
>>>  mm/hugetlb.c | 7 ++-----
>>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>>> index d3a0650ff6905..8d551cdaef9db 100644
>>> --- a/mm/hugetlb.c
>>> +++ b/mm/hugetlb.c
>>> @@ -1446,11 +1446,8 @@ void add_hugetlb_folio(struct hstate *h, struct folio *folio,
>>>  	}
>>>
>>>  	__folio_set_hugetlb(folio);
>>> -	folio_change_private(folio, NULL);
>>> -	/*
>>> -	 * We have to set hugetlb_vmemmap_optimized again as above
>>> -	 * folio_change_private(folio, NULL) cleared it.
>>> -	 */
>>> +	/* Clear all folio->private flags except hugetlb_vmemmap_optimized. */
>>> +	folio->private = NULL;
>>>  	folio_set_hugetlb_vmemmap_optimized(folio);
>>>
>> 
>> Sashiko[1] said when a CMA hugetlb folio is freed but fails vmemmap restore,
>> add_hugetlb_folio() clears HPG_cma. It either bypasses
>> hugetlb_cma_free_frozen_folio() due to the missing HPG_cma flag, or
>> skips cma_release() and causes a CMA memory leak if it is free directly
>> to buddy allocator. Sashiko also asked if HPG_temporary should be preserved
>> as well HPG_cma.
>> 
>> Answer:
>> 
>> Missing HPG_cma is a real issue. It can be fixed by checking
>> folio_test_hugetlb_cma(folio) at the beginning of add_hugetlb_folio() and
>> setting HPG_cma after folio->private is assigned to NULL.
>
>
> Maybe we should just simply only clear flags we actually want to clear ...

And it turns out to be a clean result (see below). add_hugetlb_folio()
adds a hugetlb folio back to freelist, so prior operations like
remove_hugetlb_folio() should clear hugetlb flags, like HPG_freed,
HPG_restore_reserve, HPG_migratible. HPG_temporary is only set in
one callsite in free_huge_folio() and is cleared before the folio is
passed to add_hugetlb_folio() via update_and_free_hugetlb_folio().
HPG_raw_hwp_unreliable probably can stay, since it preserves poison
information. The remaining HPG_cma and HPG_vmemmap_optimized are the two
we want to preserve.

Muchun, maybe you can chime in about the patch below. It can be a
follow-up patch after this series.

Thanks.


diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 8d551cdaef9db..bc772954e5685 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1446,9 +1446,6 @@ void add_hugetlb_folio(struct hstate *h, struct folio *folio,
 	}
 
 	__folio_set_hugetlb(folio);
-	/* Clear all folio->private flags except hugetlb_vmemmap_optimized. */
-	folio->private = NULL;
-	folio_set_hugetlb_vmemmap_optimized(folio);
 
 	arch_clear_hugetlb_flags(folio);
 	enqueue_hugetlb_folio(h, folio);
@@ -1736,6 +1733,7 @@ void free_huge_folio(struct folio *folio)
 		h->resv_huge_pages++;
 
 	if (folio_test_hugetlb_temporary(folio)) {
+		folio_clear_hugetlb_temporary(folio);
 		remove_hugetlb_folio(h, folio, false);
 		spin_unlock_irqrestore(&hugetlb_lock, flags);
 		update_and_free_hugetlb_folio(h, folio, true);


-- 
Best Regards,
Yan, Zi