Re: [PATCH 2/4] mm/huge_memory: add page->private check back in __split_folio_to_order()

["David Hildenbrand (Arm)" <david@kernel.org>]

On 6/29/26 16:39, Vlastimil Babka (SUSE) wrote:
> On 6/29/26 04:56, Zi Yan wrote:

s/add/readd/

>> page->private should not be set in tail pages. Commit 4265d67e405a
>> ("mm/migrate_device: add THP splitting during migration") removed it
>> without a proper reason. Add it back.

You can link to the discussion where we clarified that this was not intentional.

>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>>  mm/huge_memory.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 2bccb0a53a0a..037d67fbec6e 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3594,6 +3594,16 @@ static void __split_folio_to_order(struct folio *folio, int old_order,
>>  		new_folio->mapping = folio->mapping;
>>  		new_folio->index = folio->index + i;
>>  
>> +		/*
>> +		 * page->private should not be set in tail pages. Fix up and warn once
>> +		 * if private is unexpectedly set. Do it before swap.val assignment
>> +		 * since private overlaps with swap.val.
>> +		 */
>> +		if (unlikely(new_folio->private)) {
>> +			VM_WARN_ON_ONCE_PAGE(true, new_head);
>> +			new_folio->private = NULL;
>> +		}
> 
> The unconditional warning means this is not expected to happen. In that case
> it's odd to check and fixup always, but only warn with CONFIG_DEBUG_VM.
> 
> If we are reasonably sure the current code is OK, and only want to catch new
> mistakes in development, we could just VM_WARN_ON_ONCE_PAGE() without fixup.
> 
> If we are paranoid, leave it as it is, but drop the "VM_" ?

Agreed, either "if (WARN_ON_ONCE(new_folio->private))" + fixup, or no fixup.

I'd prefer VM_WARN_ON_ONCE_PAGE().

(I was only able to trigger this once while testing my own patches)

-- 
Cheers,

David