Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()

[Zi Yan <ziy@nvidia.com>]

On 9 Feb 2026, at 11:20, David Hildenbrand (Arm) wrote:

> On 2/9/26 17:16, David Hildenbrand (Arm) wrote:
>>>> I recall that freeing pages with page->private set was allowed. Although
>>>> I once wondered whether we should actually change that.
>>>
>>> But if that is allowed, we can end up with tail page's private non zero,
>>> because that free page can merge with a lower PFN buddy and its ->private
>>> is not reset. See __free_one_page().
>>
>> Right. Or someone could use page->private on tail pages and free non- zero ->private that way.
>>
>> [...]
>>
>>>
>>> For the issue reported by Mikhail[2], the page comes from vmalloc(), so it will not be split.
>>> For other cases, a page/folio needs to be compound to be splittable and prep_compound_tail()
>>> sets all tail page's private to 0. So that check is not that useful.
>>
>> Thanks.
>>
>>>
>>> And the issue we are handling here is non compound high order page allocation. No one is
>>> clearing ->private for all pages right now.
>>
>> Right.
>>
>>>
>>> OK, I think we want to decide whether it is OK to have a page with set ->private at
>>> page free time.
>>
>> Right. And whether it is okay to have any tail->private be non-zero.
>>
>>> If no, we can get this patch in and add a VM_WARN_ON_ONCE(page->private)
>>> to catch all violators. If yes, we can use Mikhail's original patch, zeroing ->private
>>> in split_page() and add a comment on ->private:
>>>
>>> 1. for compound page allocation, prep_compound_tail() is responsible for resetting ->private;
>>> 2. for non compound high order page allocation, split_page() is responsible for resetting ->private.
>>
>> Ideally, I guess, we would minimize the clearing of the ->private fields.
>>
>> If we could guarantee that *any* pages in the buddy have ->private clear, maybe
>> prep_compound_tail() could stop clearing it (and check instead).
>>
>> So similar to what Vlasta said, maybe we want to (not check but actually clear):
>>
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index e4104973e22f..4960a36145fe 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -1410,6 +1410,7 @@ __always_inline bool free_pages_prepare(struct page *page,
>>                                  }
>>                          }
>>                          (page + i)->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>> +                       set_page_private(page + i, 0);
>>                  }
>>          }
>
> Thinking again, maybe it is indeed better to rework the code to not allow freeing pages with ->private on any page. Then, we only have to zero it out where we actually used it and could check here that all
> ->private is 0.
>
> I guess that's a bit more work, and any temporary fix would likely just do.

I agree. Silently fixing non zero ->private just moves the work/responsibility
from users to core mm. They could do better. :)

We can have a patch or multiple patches to fix users do not zero ->private
when freeing a page and add the patch below. The hassle would be that
catching all, especially non mm users might not be easy, but we could merge
the patch below (and obviously fixes) after next merge window is closed and
let rc tests tell us the remaining one. WDYT?


diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 24ac34199f95..0c5d117a251e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1411,6 +1411,7 @@ __always_inline bool free_pages_prepare(struct page *page,
 				}
 			}
 			(page + i)->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
+			VM_WARN_ON_ONCE((page + i)->private);
 		}
 	}
 	if (folio_test_anon(folio)) {
@@ -1430,6 +1431,7 @@ __always_inline bool free_pages_prepare(struct page *page,

 	page_cpupid_reset_last(page);
 	page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
+	VM_WARN_ON_ONCE(page->private);
 	page->private = 0;
 	reset_page_owner(page, order);
 	page_table_check_free(page, order);


Best Regards,
Yan, Zi