Re: [PATCH v3] mm/migrate_device: Clear stale mapping after freeing swapcache

[Yadav, Arvind]

On 27-07-2026 13:30, Balbir Singh wrote:
> On 7/27/26 3:39 PM, Arvind Yadav wrote:
> > __migrate_device_pages() reads the folio mapping before calling
> > folio_free_swap(). When folio_free_swap() succeeds, the folio is removed
> > from the swap cache, but the saved mapping still points to swap_space.
> >
> > Passing the stale mapping to folio_migrate_mapping() makes it take the
> > mapped-folio path after the swapcache reference has been dropped. This can
> > cause an invalid swap_space lock access followed by a folio reference
> > count BUG.
> >
> > After a successful split, nr still contains the number of pages in the
> > original large folio, although each resulting page is now a separate
> > order-0 folio. Reset nr to 1 so each split folio is processed separately,
> > including its own swapcache removal and mapping lookup.
> >
> > Refresh the saved mapping after folio_free_swap() so the current folio
> > state is used during migration.
> >
> > v2:
> > - Refresh the mapping using folio_mapping(), as suggested by Zi Yan.
> >
> > v3:
> > - Reset nr to 1 after a successful split so each resulting folio is
> >    processed independently, as suggested by Zi Yan.
> >
> > Fixes: df263d9a7dff ("mm/migrate_device: try to handle swapcache pages")
> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > Cc: David Hildenbrand <david@xxxxxxxxxx>
> > Cc: Matthew Brost <matthew.brost@xxxxxxxxx>
> > Cc: Joshua Hahn <joshua.hahnjy@xxxxxxxxx>
> > Cc: Rakie Kim <rakie.kim@xxxxxx>
> > Cc: Byungchul Park <byungchul@xxxxxx>
> > Cc: Gregory Price <gourry@xxxxxxxxxx>
> > Cc: Ying Huang <ying.huang@xxxxxxxxxxxxxxxxx>
> > Cc: Alistair Popple <apopple@xxxxxxxxxx>
> > Reviewed-by: Zi Yan <ziy@xxxxxxxxxx>
> > Reviewed-by: Balbir Singh <balbirs@xxxxxxxxxx>
> > Signed-off-by: Arvind Yadav <arvind.yadav@xxxxxxxxx>
> > ---
> >   mm/migrate_device.c | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
> >
> > diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> > index 554754eb26ff..74ac6651d4b3 100644
> > --- a/mm/migrate_device.c
> > +++ b/mm/migrate_device.c
> > @@ -1182,6 +1182,11 @@ static void __migrate_device_pages(unsigned long *src_pfns,
> >   							 MIGRATE_PFN_COMPOUND);
> >   					goto next;
> >   				}
> > +				/*
> > +				 * reset nr so that only first after-split folio
> > +				 * is processed below
> > +				 */
> > +				nr = 1;
> We should also add a VM_WARN_ON_ONCE(folio_test_swapcache(folio))


Agreed , but folio_test_swapcache()  is not valid immediately after the 
split, as the order-0 folios may still be in swapcache.
I’ll use just before nr = 1

> >   			} else if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
> >   				(dst_pfns[i] & MIGRATE_PFN_COMPOUND) &&
> >   				!(src_pfns[i] & MIGRATE_PFN_COMPOUND)) {
> > @@ -1204,6 +1209,12 @@ static void __migrate_device_pages(unsigned long *src_pfns,
> >   					src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
> >   					goto next;
> >   				}
> > +
> > +				/*
> > +				 * folio_free_swap() removed the folio from the swap
> > +				 * cache. Refresh the saved mapping before migration.
> > +				 */
> > +				mapping = folio_mapping(folio);
> >   			}
> I wonder if we should change this instead to (sorry for the wrap around, I'll
> need to fix my mailer)
>
> @@ -1242,6 +1232,11 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>                  for (j = 0; j < nr && i + j < npages; j++) {
>                          folio = page_folio(migrate_pfn_to_page(src_pfns[i+j]));
>                          newfolio = page_folio(migrate_pfn_to_page(dst_pfns[i+j]));
> +                       /*
> +                        * folio_free_swap() removed the folio from the swap
> +                        * cache. Refresh the saved mapping before migration.
> +                        */
> +                       mapping = folio_mapping(folio);

Agreed, Moving the mapping lookup into the per-folio loop looks better; 
I’ll update it


Thanks,
Arvind

> >   		} else if (folio_is_zone_device(newfolio)) {
> >   			/*
> Thanks,
> Balbir