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

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

[Arvind Yadav]

__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 use the
mapped-folio path for a folio that is no longer in swapcache. It can
then operate on swap_space.i_pages with invalid reference accounting,
eventually triggering 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.

v4:
- Re-read each source folio's mapping immediately before
  folio_migrate_mapping(), as suggested by Balbir Singh.
- Add a warning to validate the post-split order-0 invariant,
  as suggested by Balbir Singh.

Fixes: df263d9a7dff ("mm/migrate_device: try to handle swapcache pages")
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@kernel.org>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Joshua Hahn <joshua.hahnjy@gmail.com>
Cc: Rakie Kim <rakie.kim@sk.com>
Cc: Byungchul Park <byungchul@sk.com>
Cc: Gregory Price <gourry@gourry.net>
Cc: Ying Huang <ying.huang@linux.alibaba.com>
Cc: Alistair Popple <apopple@nvidia.com>
Reviewed-by: Zi Yan <ziy@nvidia.com>
Reviewed-by: Balbir Singh <balbirs@nvidia.com>
Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
---
 mm/migrate_device.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index 554754eb26ff..0e6f00190fd8 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -1182,6 +1182,13 @@ 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
+				 */
+				VM_WARN_ON_ONCE(folio_test_large(folio));
+				nr = 1;
 			} else if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
 				(dst_pfns[i] & MIGRATE_PFN_COMPOUND) &&
 				!(src_pfns[i] & MIGRATE_PFN_COMPOUND)) {
@@ -1221,6 +1228,12 @@ static void __migrate_device_pages(unsigned long *src_pfns,
 			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);
+
 			r = folio_migrate_mapping(mapping, newfolio, folio, extra_cnt);
 			if (r)
 				src_pfns[i+j] &= ~MIGRATE_PFN_MIGRATE;
-- 
2.43.0

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

[Balbir Singh]

On 7/28/26 4:28 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 use the
> mapped-folio path for a folio that is no longer in swapcache. It can
> then operate on swap_space.i_pages with invalid reference accounting,
> eventually triggering 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.
> 
> v4:
> - Re-read each source folio's mapping immediately before
>   folio_migrate_mapping(), as suggested by Balbir Singh.
> - Add a warning to validate the post-split order-0 invariant,
>   as suggested by Balbir Singh.
> 
> Fixes: df263d9a7dff ("mm/migrate_device: try to handle swapcache pages")
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@kernel.org>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Joshua Hahn <joshua.hahnjy@gmail.com>
> Cc: Rakie Kim <rakie.kim@sk.com>
> Cc: Byungchul Park <byungchul@sk.com>
> Cc: Gregory Price <gourry@gourry.net>
> Cc: Ying Huang <ying.huang@linux.alibaba.com>
> Cc: Alistair Popple <apopple@nvidia.com>
> Reviewed-by: Zi Yan <ziy@nvidia.com>
> Reviewed-by: Balbir Singh <balbirs@nvidia.com>
> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> ---
>  mm/migrate_device.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
Reviewed-by: Balbir Singh <balbirs@nvidia.com>

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

[David Hildenbrand (Arm)]

On 7/28/26 08:28, 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 use the
> mapped-folio path for a folio that is no longer in swapcache. It can
> then operate on swap_space.i_pages with invalid reference accounting,
> eventually triggering 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.
> 
> v4:
> - Re-read each source folio's mapping immediately before
>   folio_migrate_mapping(), as suggested by Balbir Singh.
> - Add a warning to validate the post-split order-0 invariant,
>   as suggested by Balbir Singh.
> 
> Fixes: df263d9a7dff ("mm/migrate_device: try to handle swapcache pages")
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@kernel.org>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Joshua Hahn <joshua.hahnjy@gmail.com>
> Cc: Rakie Kim <rakie.kim@sk.com>
> Cc: Byungchul Park <byungchul@sk.com>
> Cc: Gregory Price <gourry@gourry.net>
> Cc: Ying Huang <ying.huang@linux.alibaba.com>
> Cc: Alistair Popple <apopple@nvidia.com>
> Reviewed-by: Zi Yan <ziy@nvidia.com>
> Reviewed-by: Balbir Singh <balbirs@nvidia.com>
> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> ---
>  mm/migrate_device.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 554754eb26ff..0e6f00190fd8 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -1182,6 +1182,13 @@ 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
> +				 */
> +				VM_WARN_ON_ONCE(folio_test_large(folio));
> +				nr = 1;

This function is surely a beauty. (had to rephrase that sentence 3 times ;) )

migrate_vma_split_unmapped_folio() modifies the src_pfns() entries on success.
(and somehow assumes that it's always a THP, what? After a MIGRATE_PFN_COMPOUND
value is set? What? Why the "nr = 1 << folio_order(folio);" in the caller).

If we ended up modifying the current entry in such a way, shouldn't we just have
retry: label and restart at the very top of the function, where we just
naturally re-read the entry/page/folio and do the right thing?

>  			} else if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
>  				(dst_pfns[i] & MIGRATE_PFN_COMPOUND) &&
>  				!(src_pfns[i] & MIGRATE_PFN_COMPOUND)) {
> @@ -1221,6 +1228,12 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>  			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);
> +
>  			r = folio_migrate_mapping(mapping, newfolio, folio, extra_cnt);
>  			if (r)
>  				src_pfns[i+j] &= ~MIGRATE_PFN_MIGRATE;

While this looks good, I do wonder why do we have to supply the mapping here at all?

Is there a path where we call folio_migrate_mapping() and the old folio (folio)
does no longer have the right mapping attached?

It would be a lot less error prone if the function would just obtain the mapping
from the old folio.

-- 
Cheers,

David

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

[Yadav, Arvind]

On 28-07-2026 18:20, David Hildenbrand (Arm) wrote:
> On 7/28/26 08:28, 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 use the
>> mapped-folio path for a folio that is no longer in swapcache. It can
>> then operate on swap_space.i_pages with invalid reference accounting,
>> eventually triggering 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.
>>
>> v4:
>> - Re-read each source folio's mapping immediately before
>>    folio_migrate_mapping(), as suggested by Balbir Singh.
>> - Add a warning to validate the post-split order-0 invariant,
>>    as suggested by Balbir Singh.
>>
>> Fixes: df263d9a7dff ("mm/migrate_device: try to handle swapcache pages")
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: David Hildenbrand <david@kernel.org>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: Joshua Hahn <joshua.hahnjy@gmail.com>
>> Cc: Rakie Kim <rakie.kim@sk.com>
>> Cc: Byungchul Park <byungchul@sk.com>
>> Cc: Gregory Price <gourry@gourry.net>
>> Cc: Ying Huang <ying.huang@linux.alibaba.com>
>> Cc: Alistair Popple <apopple@nvidia.com>
>> Reviewed-by: Zi Yan <ziy@nvidia.com>
>> Reviewed-by: Balbir Singh <balbirs@nvidia.com>
>> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
>> ---
>>   mm/migrate_device.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>> index 554754eb26ff..0e6f00190fd8 100644
>> --- a/mm/migrate_device.c
>> +++ b/mm/migrate_device.c
>> @@ -1182,6 +1182,13 @@ 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
>> +				 */
>> +				VM_WARN_ON_ONCE(folio_test_large(folio));
>> +				nr = 1;
> This function is surely a beauty. (had to rephrase that sentence 3 times ;) )
>
> migrate_vma_split_unmapped_folio() modifies the src_pfns() entries on success.
> (and somehow assumes that it's always a THP, what? After a MIGRATE_PFN_COMPOUND
> value is set? What? Why the "nr = 1 << folio_order(folio);" in the caller).
>
> If we ended up modifying the current entry in such a way, shouldn't we just have
> retry: label and restart at the very top of the function, where we just
> naturally re-read the entry/page/folio and do the right thing?


Agreed. Since migrate_vma_split_unmapped_folio() rewrites src_pfns[], I 
will retry the current entry so all state is re-read.

I will keep nr set to the original folio size for the split-failure 
path, where goto next must skip the complete folio range.

>
>>   			} else if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
>>   				(dst_pfns[i] & MIGRATE_PFN_COMPOUND) &&
>>   				!(src_pfns[i] & MIGRATE_PFN_COMPOUND)) {
>> @@ -1221,6 +1228,12 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>>   			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);
>> +
>>   			r = folio_migrate_mapping(mapping, newfolio, folio, extra_cnt);
>>   			if (r)
>>   				src_pfns[i+j] &= ~MIGRATE_PFN_MIGRATE;
> While this looks good, I do wonder why do we have to supply the mapping here at all?
>
> Is there a path where we call folio_migrate_mapping() and the old folio (folio)
> does no longer have the right mapping attached?
>
> It would be a lot less error prone if the function would just obtain the mapping
> from the old folio.


Agreed. I will make folio_migrate_mapping() obtain the mapping from the 
source folio and update its in-tree callers. Since this changes an 
exported helper, I will send it as a separate patch in v5.

Please correct me if I misunderstood your suggestion.

Thanks,
Arvind

>

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

[David Hildenbrand (Arm)]

>> While this looks good, I do wonder why do we have to supply the mapping here
>> at all?
>>
>> Is there a path where we call folio_migrate_mapping() and the old folio (folio)
>> does no longer have the right mapping attached?
>>
>> It would be a lot less error prone if the function would just obtain the mapping
>> from the old folio.
> 
> 
> Agreed. I will make folio_migrate_mapping() obtain the mapping from the source
> folio and update its in-tree callers. Since this changes an exported helper, I
> will send it as a separate patch in v5.

I think it would be good to understand why we currently pass in the folio: was
there ever a reason to do so? Or why do we pass in something that can just
easily be had from the source folio?

But note that that should be a follow-up cleanup to the fix.

-- 
Cheers,

David

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

[Yadav, Arvind]

On 30-07-2026 18:17, David Hildenbrand (Arm) wrote:
>>> While this looks good, I do wonder why do we have to supply the mapping here
>>> at all?
>>>
>>> Is there a path where we call folio_migrate_mapping() and the old folio (folio)
>>> does no longer have the right mapping attached?
>>>
>>> It would be a lot less error prone if the function would just obtain the mapping
>>> from the old folio.
>>
>> Agreed. I will make folio_migrate_mapping() obtain the mapping from the source
>> folio and update its in-tree callers. Since this changes an exported helper, I
>> will send it as a separate patch in v5.
> I think it would be good to understand why we currently pass in the folio: was
> there ever a reason to do so? Or why do we pass in something that can just
> easily be had from the source folio?
>
> But note that that should be a follow-up cleanup to the fix.


Thanks for the suggestion. Once this is merged, I will review the 
history and callers to confirm whether any path intentionally uses a 
mapping other than folio_mapping(src) and follow up with a separate 
cleanup patch if needed.

Thanks,
Arvind