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

[Hongfu Li <hongfu.li@linux.dev>]

> > 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.
+                                */
+                               folio = page_folio(vmf->page);
                                folio_unlock(folio);
                                folio_put(folio);
                        } else {