Re: [PATCH v2 1/3] mm/huge_memory: Fix use of NULL folio in move_pages_huge_pmd()
From: Lorenzo Stoakes
Date: Mon Mar 02 2026 - 12:47:26 EST
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@xxxxxxxxxx/
>
> 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