Re: [PATCH v4] mm: Use a folio in the softleaf_is_device_private path

[Hongfu Li]

On 8/18/26 4:56 PM, Lorenzo Stoakes (ARM) wrote:
> On Tue, Aug 18, 2026 at 09:30:40AM +0200, David Hildenbrand (Arm) wrote:
> > On 8/18/26 04:13, Hongfu Li wrote:
> > > > > So after migrate_to_ram() the folio might have been split or otherwise somehow
> > > > > the page doesn't belong to the same locked, refcount-incremented folio it did
> > > > > before?
> > > > I suspect a split.
> > > >
> > > > > That's kinda a footgun... but this documents it at least.
> > > > >
> > > > > I think a comment explaining how this can happen would be helpful though as this
> > > > > doesn't seem intuitive.
> > > > I think, conceptually, calling into something that consumes a page (vmf->page)
> > > > always needs care when operating on folios.
> > > >
> > > > Passing the vmf to some callback might be the odd thing here, because the
> > > > vmf->page contract is not really clear.
> > > I'm very sorry for introducing this regression.
> > > Thank you for identifying and testing this issue.
> > >
> > > I will double-check this code path and apply your fix to run further
> > > tests locally.
> > >
> > > Would it make sense to add a comment here like the below, to make this
> > > subtle behavior clearer for future readers?
> > >
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index 4134ac607ee0..2b6d6c863ecb 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -4937,6 +4937,12 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> > >                                  pte_unmap_unlock(vmf->pte, vmf->ptl);
> > >                                  pgmap = page_pgmap(vmf->page);
> > >                                  ret = pgmap->ops->migrate_to_ram(vmf);
> > > +                               /*
> > > +                                * migrate_to_ram() can split a large folio, updating
> > > +                                * vmf->page to a different folio. Re-fetch folio for
> > > +                                * correct unlock/put.
> > > +                                */
> > "migrate_to_ram() might have split the folio."
> >
> > Should be sufficient I guess.
> Agreed, no need for a schloppy long comment here!
>
> 	/* migrate_to_ram() might have split the folio. */
> 	folio = page_folio(vmf->page);
>
> Should be fine.
>
> The commit message should be updated to reflect it too and mention that we are
> going from 4 -> 2 compound_head()'s now.

Thanks a lot for the suggestion. A shorter comment is indeed better.

Will use your suggested one‑line comment, add the folio re‑fetch, and
update commit message for v5.

> > --
> > Cheers,
> >
> > David
> --
> Cheers, Lorenzo

--
Best regards,
Hongfu