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

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Mon, Aug 17, 2026 at 05:57:57PM +0200, David Hildenbrand (Arm) wrote:
> On 8/17/26 17:52, David Hildenbrand (Arm) wrote:
> > On 8/3/26 05:15, Hongfu Li wrote:
> >> From: Hongfu Li <lihongfu@kylinos.cn>
> >>
> >> Use the folio APIs in the device_private migration path of do_swap_page(),
> >> replacing four calls to compound_head() with one.
> >>
> >> Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
> >> Acked-by: David Hildenbrand (Arm) <david@kernel.org>
> >> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> >> ---
> >> v4:
> >> - Drop the do_non_swap_page() extraction (patch 2/2 from v2)
> >> - Added Reviewed-by: Lorenzo Stoakes
> >> - Link to v2: https://lore.kernel.org/all/20260723030752.16979-1-hongfu.li@linux.dev/
> >> v3:
> >> - Please ignore v3.
> >> v2:
> >> - Drop fault_folio, reuse the existing folio variable instead
> >> - Refresh commit messages
> >> - Update comment to use "folio" instead of "page" in the
> >>   device_private path
> >> - Add Acked-by: David Hildenbrand
> >> - Add patch 2/2 to extract do_non_swap_page() from do_swap_page()
> >> - Link to v1: https://lore.kernel.org/all/20260721013347.65698-1-hongfu.li@linux.dev/
> >> ---
> >>  mm/memory.c | 11 ++++++-----
> >>  1 file changed, 6 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/mm/memory.c b/mm/memory.c
> >> index 56b244552f13..bbb289ba09d3 100644
> >> --- a/mm/memory.c
> >> +++ b/mm/memory.c
> >> @@ -4873,18 +4873,19 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> >>  				goto unlock;
> >>
> >>  			/*
> >> -			 * Get a page reference while we know the page can't be
> >> +			 * Get a folio reference while we know the folio can't be
> >>  			 * freed.
> >>  			 */
> >> -			if (trylock_page(vmf->page)) {
> >> +			folio = page_folio(vmf->page);
> >> +			if (folio_trylock(folio)) {
> >>  				struct dev_pagemap *pgmap;
> >>
> >> -				get_page(vmf->page);
> >> +				folio_get(folio);
> >>  				pte_unmap_unlock(vmf->pte, vmf->ptl);
> >>  				pgmap = page_pgmap(vmf->page);
> >>  				ret = pgmap->ops->migrate_to_ram(vmf);
> >> -				unlock_page(vmf->page);
> >> -				put_page(vmf->page);
> >> +				folio_unlock(folio);
> >> +				folio_put(folio);
> >>  			} else {
> >>  				pte_unmap(vmf->pte);
> >>  				softleaf_entry_wait_on_locked(entry, vmf->ptl);
> >
> > Okay, so this breaks the HMM selftests:
> >
> > # ok 30 hmm.hmm_device_private.migrate_anon_huge_fault
> > # #  RUN           hmm.hmm_device_private.migrate_partial_unmap_fault ...
> > [   14.589125] Internal error: Oops - BUG: 00000000f2000800 [#1]  SMP
> > [   14.589178] Modules linked in: test_hmm
> > [   14.589203] CPU: 25 UID: 0 PID: 653 Comm: hmm-tests Not tainted 7.2.0-rc5+ #14 PREEMPT
> > [   14.589243] Hardware name: linux,dummy-virt (DT)
> > [   14.589268] pstate: 63400005 (nZCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
> > [   14.589300] pc : folio_unlock+0x78/0x88
> > [   14.589326] lr : folio_unlock+0x78/0x88
> > [   14.589349] sp : ffff80008284bb90
> > [   14.589368] x29: ffff80008284bb90 x28: ffff0000cc8ce180 x27: 0000000000000000
> > [   14.589406] x26: 0000000000000000 x25: ffff0000ca395dc0 x24: 0000000000000000
> > [   14.589447] x23: 0000000020100073 x22: ffff0000c23d35c0 x21: ffffffffbfff8000
> > [   14.589485] x20: ffffffffbfff8000 x19: ffff80008284bca0 x18: 0000000000000010
> > [   14.589523] x17: 3030303030303030 x16: 2066666666666666 x15: 6630303030303030
> > [   14.589562] x14: 0000000000000000 x13: 29296f696c6f6628 x12: 64656b636f6c5f74
> > [   14.589603] x11: fffffffffffc7100 x10: fffffffffffc70b0 x9 : ffffb0504ff88f70
> > [   14.589640] x8 : 3fffffffffffdfff x7 : ffff000c79b24bc0 x6 : 00000000000001a7
> > [   14.589680] x5 : ffff000c79b24bc0 x4 : 0000000000000000 x3 : 0000000000000000
> > [   14.589718] x2 : 0000000000000000 x1 : ffff0000cc8ce180 x0 : 000000000000003f
> > [   14.589756] Call trace:
> > [   14.589769]  folio_unlock+0x78/0x88 (P)
> > [   14.589792]  do_swap_page+0xf40/0x1580
> > [   14.589815]  __handle_mm_fault+0x648/0xe68
> > [   14.589838]  handle_mm_fault+0x9c/0x330
> > [   14.589858]  do_page_fault+0x268/0x7e0
> > [   14.589882]  do_translation_fault+0x5c/0x80
> > [   14.589904]  do_mem_abort+0x48/0xa0
> > [   14.589927]  el0_da+0x38/0xd0
> > [   14.589948]  el0t_64_sync_handler+0xd0/0xe8
> > [   14.589970]  el0t_64_sync+0x198/0x1a0
> > [   14.589990] Code: d65f03c0 d000eec1 913c4021 94015993 (d4210000)
> >
> > I can only speculate that we need another
> >
> > 	folio = page_folio(vmf->page);
> >
> > after the migrate_to_ram() call.
> >
> > Let me try that real quick.
> >
>
> Yeah, that fixes it:
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 4134ac607ee0..83d1a2be957c 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4937,6 +4937,7 @@ 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);
> +                               folio = page_folio(vmf->page);

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?

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.


>                                 folio_unlock(folio);
>                                 folio_put(folio);
>                         } else {
>
>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo