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

[Pedro Falcato]

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>

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

Lord help us.

-- 
Pedro