Re: [PATCH 31/46] mm/mremap: fix moving PUD leaves

[David Hildenbrand (Arm)]

On 10/28/26 14:20, Pedro Falcato wrote:
> On Tue, Oct 27, 2026 at 12:17:51AM +0100, David Hildenbrand (Arm) wrote:
>> From: Josef Bacik <josef@xxxxxxxxxxxxxx>
>>
>> pud_none_or_clear_bad() only really works on page tables. PUD leaves
>> confuse the check and make us clear the page table entry and print a
>> warning.
>>
>> Since commit aed877c2b4257 ("device/dax: properly refcount device dax pages
>> when mapping"), this also leaks device-DAX folio references, mapcounts
>> and RSS accounting, resulting in "Bad rss-counter state" warnings on exit.
>>
>> Allow PUD leaves in get_old_pud(). If a PUD leaf cannot be moved as a
>> whole or the move fails, split it. This unmaps it with proper accounting
>> instead of treating the leaf entry as a PMD table.
>>
>> Keep using pud_trans_huge() for now, in the future this code should be
>> cleaned up to just check for PUD leaves.
>>
>> Fix generated by an LLM and sent by Josef Bacik in private.
>>
>> Fixes: a00cc7d9dd93 ("mm, x86: add support for PUD-sized transparent hugepages")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
>> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
>> [ david: fallthrough after splitting ]
>> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> 
> Reviewed-by: Pedro Falcato <pfalcato@xxxxxxx>

Thanks!

> 
>> ---
>>  mm/mremap.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/mremap.c b/mm/mremap.c
>> index 91f0865f6aeea..1237718848f5c 100644
>> --- a/mm/mremap.c
>> +++ b/mm/mremap.c
>> @@ -87,7 +87,7 @@ static pud_t *get_old_pud(struct mm_struct *mm, unsigned long addr)
>>  		return NULL;
>>  
>>  	pud = pud_offset(p4d, addr);
>> -	if (pud_none_or_clear_bad(pud))
>> +	if (!pud_trans_huge(*pud) && pud_none_or_clear_bad(pud))
>>  		return NULL;
>>  
>>  	return pud;
>> @@ -847,11 +847,10 @@ unsigned long move_page_tables(struct pagetable_move_control *pmc)
>>  		if (!new_pud)
>>  			break;
>>  		if (pud_trans_huge(*old_pud)) {
>> -			if (extent == HPAGE_PUD_SIZE) {
>> -				move_pgt_entry(pmc, HPAGE_PUD, old_pud, new_pud);
>> -				/* We ignore and continue on error? */
>> +			if (extent == HPAGE_PUD_SIZE &&
>> +			    move_pgt_entry(pmc, HPAGE_PUD, old_pud, new_pud))
>>  				continue;
>> -			}
>> +			split_huge_pud(pmc->old, old_pud, pmc->old_addr);
> 
> Note that, if the HPAGE_PUD move fails, we split but miss the opportunity to
> move at the PUD level (right below). Is this on purpose? Am I missing
> something?

I was mirroring the PMD code, really. There, we also don't seem to move a PTE
page table when moving the PMD leaf failed and we split.

I think part of the puzzle is: move_huge_pmd() / move_huge_pud() only really
fail if there was some race with concurrent splitting. So it doesn't really
happen all that often I guess.

> 
> ... I say, realizing I am missing something, because split_huge_pmd (and
> consequently split_huge_pud) just decide to zap if the VMA isn't anonymous,
> and split_huge_pud() just hardcoded it to "of course it's not anonymous, lol",
> and just zaps. So there's no point in moving what isn't there.

Right, we don't support anonymous PUDs and file PUDs/PMDs are split by
effectively zapping them (no PTE page table prealloc required).

Can I hear that you are interested in cleaning up this code? :)

-- 
Cheers,

David