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

[Muchun Song]

> On Sep 18, 2026, at 09:41, Zi Yan <ziy@xxxxxxxxxx> wrote:
> 
> 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@xxxxxxxxx>
>>>> To: Oscar Salvador <osalvador@xxxxxxx>
>>>> To: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>>>> Cc: David Hildenbrand <david@xxxxxxxxxx>
>>>> Cc: linux-mm@xxxxxxxxx
>>>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>>>> Acked-by: Usama Arif <usama.arif@xxxxxxxxx>
>>>> Reviewed-by: Gregory Price (Meta) <gourry@xxxxxxxxxx>
>>>> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
>>>> ---
>>>> 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);

This looks good to me. Since add_hugetlb_folio() will no longer clear
folio->private, would it be worth adding a whitelist-based
VM_WARN_ON_ONCE_FOLIO() here to catch unexpected flags? This would document
which flags may survive the transition and help detect future callers that
fail to clear transient state before returning a folio to the pool.

Thanks.

> 
> 	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