Re: [PATCH v2 1/3] mm/huge_memory: Fix use of NULL folio in move_pages_huge_pmd()

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Mon, Mar 02, 2026 at 06:35:46PM +0100, David Hildenbrand (Arm) wrote:
>
> >>  mm/huge_memory.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> >> index 44ff8a648afd..fed57951a7cd 100644
> >> --- a/mm/huge_memory.c
> >> +++ b/mm/huge_memory.c
> >> @@ -2794,7 +2794,7 @@ int move_pages_huge_pmd(struct mm_struct *mm, pmd_t *dst_pmd, pmd_t *src_pmd, pm
> >>  		_dst_pmd = pmd_mkwrite(pmd_mkdirty(_dst_pmd), dst_vma);
> >>  	} else {
> >>  		src_pmdval = pmdp_huge_clear_flush(src_vma, src_addr, src_pmd);
> >> -		_dst_pmd = folio_mk_pmd(src_folio, dst_vma->vm_page_prot);
> >> +		_dst_pmd = folio_mk_pmd(page_folio(src_page), dst_vma->vm_page_prot);
> >
> > I prefer my version at [0].
> >
> > Cleaner to actually pull out the zero_folio into a local variable, and also we
> > should mark it special to be consistent with other codepaths.
>
> I argued in v1 that we should handle it similar to an ordinary move
> during mremap()->move_huge_pmd() and not split it over two patches.
>
> It's still split over two patches, which doesn't make sense.

Yes, let's not do that, I made the same comment.

>
> https://lore.kernel.org/linux-mm/0b653dcd-842b-4360-bc1c-8fe779efbc23@kernel.org/
>
> I don't think there is no need to get the folio involved at all if we
> know that we have a well-prepared PMD (zero folio, makred as special).
>
> The less code we have that has to deal with setting PMDs special (and
> possible messing it up), the better.

Yup I agree, I replied accordingly. That's a more elegant thing than duplicating
huge zero installation code.

I had just assumed that there was _some reason_ why we wouldn't want to do that
given the original patch from Suren didn't just do that, and for the sakes of a
backport no need to think too deep on it.

But you're right I don't think there's any reason we need to diverge from what
mremap() would do.

That does have:

		if (vma_has_uffd_without_event_remap(vma))
			pmd = clear_uffd_wp_pmd(pmd);

Though rather than unconditonally invoking clear_uffd_wp_pmd().

Is that correct?

(I hate the uffd wp stuff)

>
>
> @Chris, please make sure to CC all relevant maintainers (I didn't check)
> and send the patches as a proper thread (e.g., through git send-mail").
>
> --
> Cheers,
>
> David

Thanks, Lorenzo