mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3] mm/migrate_device: Clear stale mapping after freeing swapcache
@ 2026-07-27  5:39 Arvind Yadav
  2026-07-27  8:00 ` Balbir Singh
  0 siblings, 1 reply; 4+ messages in thread
From: Arvind Yadav @ 2026-07-27  5:39 UTC (permalink / raw)
  To: linux-mm, linux-kernel
  Cc: akpm, david, matthew.brost, joshua.hahnjy, ziy, rakie.kim,
	byungchul, gourry, ying.huang, apopple, balbirs

__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@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 | 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;
 			} 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);
 			}
 		} else if (folio_is_zone_device(newfolio)) {
 			/*
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] mm/migrate_device: Clear stale mapping after freeing swapcache
  2026-07-27  5:39 [PATCH v3] mm/migrate_device: Clear stale mapping after freeing swapcache Arvind Yadav
@ 2026-07-27  8:00 ` Balbir Singh
  2026-07-27 10:58   ` Yadav, Arvind
  0 siblings, 1 reply; 4+ messages in thread
From: Balbir Singh @ 2026-07-27  8:00 UTC (permalink / raw)
  To: Arvind Yadav, linux-mm, linux-kernel
  Cc: akpm, david, matthew.brost, joshua.hahnjy, ziy, rakie.kim,
	byungchul, gourry, ying.huang, apopple

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@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 | 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))

>  			} 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);



>  		} else if (folio_is_zone_device(newfolio)) {
>  			/*

Thanks,
Balbir

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] mm/migrate_device: Clear stale mapping after freeing swapcache
  2026-07-27  8:00 ` Balbir Singh
@ 2026-07-27 10:58   ` Yadav, Arvind
  2026-07-27 22:43     ` Andrew Morton
  0 siblings, 1 reply; 4+ messages in thread
From: Yadav, Arvind @ 2026-07-27 10:58 UTC (permalink / raw)
  To: Balbir Singh, linux-mm, linux-kernel
  Cc: akpm, david, matthew.brost, joshua.hahnjy, ziy, rakie.kim,
	byungchul, gourry, ying.huang, apopple


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@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 | 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] mm/migrate_device: Clear stale mapping after freeing swapcache
  2026-07-27 10:58   ` Yadav, Arvind
@ 2026-07-27 22:43     ` Andrew Morton
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2026-07-27 22:43 UTC (permalink / raw)
  To: Yadav, Arvind
  Cc: Balbir Singh, linux-mm, linux-kernel, david, matthew.brost,
	joshua.hahnjy, ziy, rakie.kim, byungchul, gourry, ying.huang,
	apopple

On Mon, 27 Jul 2026 16:28:42 +0530 "Yadav, Arvind" <arvind.yadav@intel.com> wrote:

> >> --- 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

AI review suggest there might be another bug in here:
	https://sashiko.dev/#/patchset/20260727053920.3494211-1-arvind.yadav@intel.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-27 22:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-27  5:39 [PATCH v3] mm/migrate_device: Clear stale mapping after freeing swapcache Arvind Yadav
2026-07-27  8:00 ` Balbir Singh
2026-07-27 10:58   ` Yadav, Arvind
2026-07-27 22:43     ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®