All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mm/migrate_device: Clear stale mapping after freeing swapcache
@ 2026-07-24  8:27 Arvind Yadav
  2026-07-24 14:00 ` Zi Yan
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Arvind Yadav @ 2026-07-24  8:27 UTC (permalink / raw)
  To: linux-mm, linux-kernel
  Cc: akpm, david, matthew.brost, joshua.hahnjy, ziy, rakie.kim,
	byungchul, gourry, ying.huang, apopple

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

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.

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: Zi Yan <ziy@nvidia.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>
Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
---
 mm/migrate_device.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index 554754eb26ff..ae895758b72c 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -1204,6 +1204,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 related	[flat|nested] 10+ messages in thread

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

On Fri Jul 24, 2026 at 4:27 AM EDT, 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.
>
> 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.
>
> 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: Zi Yan <ziy@nvidia.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>
> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> ---
>  mm/migrate_device.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
LGTM. Thanks.

Reviewed-by: Zi Yan <ziy@nvidia.com>

-- 
Best Regards,
Yan, Zi



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

* Re: [PATCH v2] mm/migrate_device: Clear stale mapping after freeing swapcache
  2026-07-24  8:27 [PATCH v2] mm/migrate_device: Clear stale mapping after freeing swapcache Arvind Yadav
  2026-07-24 14:00 ` Zi Yan
@ 2026-07-25  4:43 ` Andrew Morton
  2026-07-25 19:36   ` David Hildenbrand (Arm)
  2026-07-27  3:06 ` Balbir Singh
  2 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2026-07-25  4:43 UTC (permalink / raw)
  To: Arvind Yadav
  Cc: linux-mm, linux-kernel, david, matthew.brost, joshua.hahnjy, ziy,
	rakie.kim, byungchul, gourry, ying.huang, apopple, Balbir Singh

On Fri, 24 Jul 2026 13:57:02 +0530 Arvind Yadav <arvind.yadav@intel.com> 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.
> 
> Refresh the saved mapping after folio_free_swap() so the current folio
> state is used during migration.
> 

Thanks.  AI review might have found an issue with this.  And one
possible pre-existing issue in the code which Alistair and Balbir
worked on.

	https://sashiko.dev/#/patchset/20260724082702.2531024-1-arvind.yadav@intel.com


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

* Re: [PATCH v2] mm/migrate_device: Clear stale mapping after freeing swapcache
  2026-07-25  4:43 ` Andrew Morton
@ 2026-07-25 19:36   ` David Hildenbrand (Arm)
  2026-07-25 21:05     ` Zi Yan
  0 siblings, 1 reply; 10+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-25 19:36 UTC (permalink / raw)
  To: Andrew Morton, Arvind Yadav
  Cc: linux-mm, linux-kernel, matthew.brost, joshua.hahnjy, ziy,
	rakie.kim, byungchul, gourry, ying.huang, apopple, Balbir Singh

On 7/25/26 06:43, Andrew Morton wrote:
> On Fri, 24 Jul 2026 13:57:02 +0530 Arvind Yadav <arvind.yadav@intel.com> 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.
>>
>> Refresh the saved mapping after folio_free_swap() so the current folio
>> state is used during migration.
>>
> 
> Thanks.  AI review might have found an issue with this.  And one
> possible pre-existing issue in the code which Alistair and Balbir
> worked on.
> 
> 	https://sashiko.dev/#/patchset/20260724082702.2531024-1-arvind.yadav@intel.com

Yeah, this might need another careful look.

-- 
Cheers,

David


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

* Re: [PATCH v2] mm/migrate_device: Clear stale mapping after freeing swapcache
  2026-07-25 19:36   ` David Hildenbrand (Arm)
@ 2026-07-25 21:05     ` Zi Yan
  2026-07-27  0:38       ` Balbir Singh
  0 siblings, 1 reply; 10+ messages in thread
From: Zi Yan @ 2026-07-25 21:05 UTC (permalink / raw)
  To: David Hildenbrand (Arm), Andrew Morton, Arvind Yadav
  Cc: linux-mm, linux-kernel, matthew.brost, joshua.hahnjy, rakie.kim,
	byungchul, gourry, ying.huang, apopple, Balbir Singh

On Sat Jul 25, 2026 at 3:36 PM EDT, David Hildenbrand (Arm) wrote:
> On 7/25/26 06:43, Andrew Morton wrote:
>> On Fri, 24 Jul 2026 13:57:02 +0530 Arvind Yadav <arvind.yadav@intel.com> 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.
>>>
>>> Refresh the saved mapping after folio_free_swap() so the current folio
>>> state is used during migration.
>>>
>> 
>> Thanks.  AI review might have found an issue with this.  And one
>> possible pre-existing issue in the code which Alistair and Balbir
>> worked on.
>> 
>> 	https://sashiko.dev/#/patchset/20260724082702.2531024-1-arvind.yadav@intel.com
>
> Yeah, this might need another careful look.

It seems that the pre-existing issue can be fixed by resetting nr to 1
after split is successful. It should also complete this patch. Something
like this:


diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index 18d097c388530..4a77b6c86ae4f 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -1193,6 +1193,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)) {


-- 
Best Regards,
Yan, Zi



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

* Re: [PATCH v2] mm/migrate_device: Clear stale mapping after freeing swapcache
  2026-07-25 21:05     ` Zi Yan
@ 2026-07-27  0:38       ` Balbir Singh
  2026-07-27  1:46         ` Zi Yan
  0 siblings, 1 reply; 10+ messages in thread
From: Balbir Singh @ 2026-07-27  0:38 UTC (permalink / raw)
  To: Zi Yan, David Hildenbrand (Arm), Andrew Morton, Arvind Yadav
  Cc: linux-mm, linux-kernel, matthew.brost, joshua.hahnjy, rakie.kim,
	byungchul, gourry, ying.huang, apopple

On 7/26/26 7:05 AM, Zi Yan wrote:
> On Sat Jul 25, 2026 at 3:36 PM EDT, David Hildenbrand (Arm) wrote:
>> On 7/25/26 06:43, Andrew Morton wrote:
>>> On Fri, 24 Jul 2026 13:57:02 +0530 Arvind Yadav <arvind.yadav@intel.com> 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.
>>>>
>>>> Refresh the saved mapping after folio_free_swap() so the current folio
>>>> state is used during migration.
>>>>
>>>
>>> Thanks.  AI review might have found an issue with this.  And one
>>> possible pre-existing issue in the code which Alistair and Balbir
>>> worked on.
>>>
>>> 	https://sashiko.dev/#/patchset/20260724082702.2531024-1-arvind.yadav@intel.com
>>
>> Yeah, this might need another careful look.
> 
> It seems that the pre-existing issue can be fixed by resetting nr to 1
> after split is successful. It should also complete this patch. Something
> like this:
> 
> 
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 18d097c388530..4a77b6c86ae4f 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -1193,6 +1193,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)) {
> 
> 

Hmm.. I don't this error condition possible, migrate_vma_split_unmapped_folio()
will VM_WARN_ON non anonymous folios, but the design contract is for anonymous
folios only. The enforcement comes from the callers of migrate_vma_pages() and
migrate_device_pages(). Also __folio_freeze_and_split_unmapped() checks if the
folio has a swapcache and mapping associated with it, prior to split. I think
this is a false positive

Balbir


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

* Re: [PATCH v2] mm/migrate_device: Clear stale mapping after freeing swapcache
  2026-07-27  0:38       ` Balbir Singh
@ 2026-07-27  1:46         ` Zi Yan
  2026-07-27  2:24           ` Balbir Singh
  2026-07-27  5:06           ` Yadav, Arvind
  0 siblings, 2 replies; 10+ messages in thread
From: Zi Yan @ 2026-07-27  1:46 UTC (permalink / raw)
  To: Balbir Singh, David Hildenbrand (Arm), Andrew Morton,
	Arvind Yadav
  Cc: linux-mm, linux-kernel, matthew.brost, joshua.hahnjy, rakie.kim,
	byungchul, gourry, ying.huang, apopple

On Sun Jul 26, 2026 at 8:38 PM EDT, Balbir Singh wrote:
> On 7/26/26 7:05 AM, Zi Yan wrote:
>> On Sat Jul 25, 2026 at 3:36 PM EDT, David Hildenbrand (Arm) wrote:
>>> On 7/25/26 06:43, Andrew Morton wrote:
>>>> On Fri, 24 Jul 2026 13:57:02 +0530 Arvind Yadav <arvind.yadav@intel.com> 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.
>>>>>
>>>>> Refresh the saved mapping after folio_free_swap() so the current folio
>>>>> state is used during migration.
>>>>>
>>>>
>>>> Thanks.  AI review might have found an issue with this.  And one
>>>> possible pre-existing issue in the code which Alistair and Balbir
>>>> worked on.
>>>>
>>>> 	https://sashiko.dev/#/patchset/20260724082702.2531024-1-arvind.yadav@intel.com
>>>
>>> Yeah, this might need another careful look.
>> 
>> It seems that the pre-existing issue can be fixed by resetting nr to 1
>> after split is successful. It should also complete this patch. Something
>> like this:
>> 
>> 
>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>> index 18d097c388530..4a77b6c86ae4f 100644
>> --- a/mm/migrate_device.c
>> +++ b/mm/migrate_device.c
>> @@ -1193,6 +1193,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)) {
>> 
>> 
>
> Hmm.. I don't this error condition possible, migrate_vma_split_unmapped_folio()
> will VM_WARN_ON non anonymous folios, but the design contract is for anonymous
> folios only. The enforcement comes from the callers of migrate_vma_pages() and

folio_test_anon() returns true for an anon folio in swapcache. So
migrate_vma_split_unmapped_folio()'s VM_WARN_ON() does not prevent anon
folios in swapcache.

> migrate_device_pages(). Also __folio_freeze_and_split_unmapped() checks if the
> folio has a swapcache and mapping associated with it, prior to split. I think
> this is a false positive
>

NULL is passed as mapping to __folio_freeze_and_split_unmapped() by
migrate_vma_split_unmapped_folio(), so the VM_WARN_ON_ONCE() there will
not warn this.

None of the above arguments is valid.

One thing prevents large anon folios in swapcache from reaching to
__migrate_device_pages() is migrate_vma_check_page(). When
folio_mapping() is not NULL, extra pin count is only 1 +
folio_has_private(), which happens to exclude large anon folios in
swapcache.

Regardless, adding nr = 1 here still makes sense, since why should the
code below process the old nr pages after split is successful? It might
be an optimization for current large anon folio only case, but it is
more like an issue in the future.



-- 
Best Regards,
Yan, Zi


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

* Re: [PATCH v2] mm/migrate_device: Clear stale mapping after freeing swapcache
  2026-07-27  1:46         ` Zi Yan
@ 2026-07-27  2:24           ` Balbir Singh
  2026-07-27  5:06           ` Yadav, Arvind
  1 sibling, 0 replies; 10+ messages in thread
From: Balbir Singh @ 2026-07-27  2:24 UTC (permalink / raw)
  To: Zi Yan, David Hildenbrand (Arm), Andrew Morton, Arvind Yadav
  Cc: linux-mm, linux-kernel, matthew.brost, joshua.hahnjy, rakie.kim,
	byungchul, gourry, ying.huang, apopple

On 7/27/26 11:46 AM, Zi Yan wrote:
> On Sun Jul 26, 2026 at 8:38 PM EDT, Balbir Singh wrote:
>> On 7/26/26 7:05 AM, Zi Yan wrote:
>>> On Sat Jul 25, 2026 at 3:36 PM EDT, David Hildenbrand (Arm) wrote:
>>>> On 7/25/26 06:43, Andrew Morton wrote:
>>>>> On Fri, 24 Jul 2026 13:57:02 +0530 Arvind Yadav <arvind.yadav@intel.com> 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.
>>>>>>
>>>>>> Refresh the saved mapping after folio_free_swap() so the current folio
>>>>>> state is used during migration.
>>>>>>
>>>>>
>>>>> Thanks.  AI review might have found an issue with this.  And one
>>>>> possible pre-existing issue in the code which Alistair and Balbir
>>>>> worked on.
>>>>>
>>>>> 	https://sashiko.dev/#/patchset/20260724082702.2531024-1-arvind.yadav@intel.com
>>>>
>>>> Yeah, this might need another careful look.
>>>
>>> It seems that the pre-existing issue can be fixed by resetting nr to 1
>>> after split is successful. It should also complete this patch. Something
>>> like this:
>>>
>>>
>>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>>> index 18d097c388530..4a77b6c86ae4f 100644
>>> --- a/mm/migrate_device.c
>>> +++ b/mm/migrate_device.c
>>> @@ -1193,6 +1193,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)) {
>>>
>>>
>>
>> Hmm.. I don't this error condition possible, migrate_vma_split_unmapped_folio()
>> will VM_WARN_ON non anonymous folios, but the design contract is for anonymous
>> folios only. The enforcement comes from the callers of migrate_vma_pages() and
> 
> folio_test_anon() returns true for an anon folio in swapcache. So
> migrate_vma_split_unmapped_folio()'s VM_WARN_ON() does not prevent anon
> folios in swapcache.
> 

Agreed. Rhe warning is in folio_split_unmapped() it only works fornon-anon
folios. So it doesn't exclude the swapcache case.

And the !folio_test_anon(folio) || !folio_free_swap(folio) actually
handles a swapcache anon folio by dropping the swap, which is precisely
where the stale mapping originates. So you're right, my argument there
doesn't hold.

>> migrate_device_pages(). Also __folio_freeze_and_split_unmapped() checks if the
>> folio has a swapcache and mapping associated with it, prior to split. I think
>> this is a false positive
>>
> 
> NULL is passed as mapping to __folio_freeze_and_split_unmapped() by
> migrate_vma_split_unmapped_folio(), so the VM_WARN_ON_ONCE() there will
> not warn this.
> 

> None of the above arguments is valid.
> 

Agreed, mapping isn't passed all the way. The split still resolves the
swapcache correctly from the current folio rather than mapping - in
__folio_freeze_and_split_unmapped(), ci = swap_cluster_get_and_lock(folio)
guarded by folio_test_swapcache(folio), and each after-split folio is
fixed up via __swap_cache_replace_folio(ci, ...)

> One thing prevents large anon folios in swapcache from reaching to
> __migrate_device_pages() is migrate_vma_check_page(). When
> folio_mapping() is not NULL, extra pin count is only 1 +
> folio_has_private(), which happens to exclude large anon folios in
> swapcache.
> 


Yes, agreed - that's the reason why it's safe, and it kicks in during
migrate_vma_unmap() before any of the above. So the condition is not
reachable today for large folios; it's a false positive in that sense.

> Regardless, adding nr = 1 here still makes sense, since why should the
> code below process the old nr pages after split is successful? It might
> be an optimization for current large anon folio only case, but it is
> more like an issue in the future.
Agreed, and we could add it in as a correctness check. After a successful
split the inner loop still runs folio_migrate_mapping(mapping, ...) over
all nr after-split folios, but folio_free_swap() was only called on the
first one - so folios [i+1 .. i+nr) would be migrated as swapcache folios
against a single cached mapping.

Thanks for the correction!
Balbir



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

* Re: [PATCH v2] mm/migrate_device: Clear stale mapping after freeing swapcache
  2026-07-24  8:27 [PATCH v2] mm/migrate_device: Clear stale mapping after freeing swapcache Arvind Yadav
  2026-07-24 14:00 ` Zi Yan
  2026-07-25  4:43 ` Andrew Morton
@ 2026-07-27  3:06 ` Balbir Singh
  2 siblings, 0 replies; 10+ messages in thread
From: Balbir Singh @ 2026-07-27  3:06 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/24/26 6:27 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.
> 
> 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.
> 
> 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: Zi Yan <ziy@nvidia.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>
> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> ---
>  mm/migrate_device.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 554754eb26ff..ae895758b72c 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -1204,6 +1204,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)) {
>  			/*

LGTM

Reviewed-by: Balbir Singh <balbirs@nvidia.com>


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

* Re: [PATCH v2] mm/migrate_device: Clear stale mapping after freeing swapcache
  2026-07-27  1:46         ` Zi Yan
  2026-07-27  2:24           ` Balbir Singh
@ 2026-07-27  5:06           ` Yadav, Arvind
  1 sibling, 0 replies; 10+ messages in thread
From: Yadav, Arvind @ 2026-07-27  5:06 UTC (permalink / raw)
  To: Zi Yan, Balbir Singh, David Hildenbrand (Arm), Andrew Morton
  Cc: linux-mm, linux-kernel, matthew.brost, joshua.hahnjy, rakie.kim,
	byungchul, gourry, ying.huang, apopple


On 27-07-2026 07:16, Zi Yan wrote:
> On Sun Jul 26, 2026 at 8:38 PM EDT, Balbir Singh wrote:
>> On 7/26/26 7:05 AM, Zi Yan wrote:
>>> On Sat Jul 25, 2026 at 3:36 PM EDT, David Hildenbrand (Arm) wrote:
>>>> On 7/25/26 06:43, Andrew Morton wrote:
>>>>> On Fri, 24 Jul 2026 13:57:02 +0530 Arvind Yadav <arvind.yadav@intel.com> 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.
>>>>>>
>>>>>> Refresh the saved mapping after folio_free_swap() so the current folio
>>>>>> state is used during migration.
>>>>>>
>>>>> Thanks.  AI review might have found an issue with this.  And one
>>>>> possible pre-existing issue in the code which Alistair and Balbir
>>>>> worked on.
>>>>>
>>>>> 	https://sashiko.dev/#/patchset/20260724082702.2531024-1-arvind.yadav@intel.com
>>>> Yeah, this might need another careful look.
>>> It seems that the pre-existing issue can be fixed by resetting nr to 1
>>> after split is successful. It should also complete this patch. Something
>>> like this:
>>>
>>>
>>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>>> index 18d097c388530..4a77b6c86ae4f 100644
>>> --- a/mm/migrate_device.c
>>> +++ b/mm/migrate_device.c
>>> @@ -1193,6 +1193,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)) {
>>>
>>>
>> Hmm.. I don't this error condition possible, migrate_vma_split_unmapped_folio()
>> will VM_WARN_ON non anonymous folios, but the design contract is for anonymous
>> folios only. The enforcement comes from the callers of migrate_vma_pages() and
> folio_test_anon() returns true for an anon folio in swapcache. So
> migrate_vma_split_unmapped_folio()'s VM_WARN_ON() does not prevent anon
> folios in swapcache.
>
>> migrate_device_pages(). Also __folio_freeze_and_split_unmapped() checks if the
>> folio has a swapcache and mapping associated with it, prior to split. I think
>> this is a false positive
>>
> NULL is passed as mapping to __folio_freeze_and_split_unmapped() by
> migrate_vma_split_unmapped_folio(), so the VM_WARN_ON_ONCE() there will
> not warn this.
>
> None of the above arguments is valid.
>
> One thing prevents large anon folios in swapcache from reaching to
> __migrate_device_pages() is migrate_vma_check_page(). When
> folio_mapping() is not NULL, extra pin count is only 1 +
> folio_has_private(), which happens to exclude large anon folios in
> swapcache.
>
> Regardless, adding nr = 1 here still makes sense, since why should the
> code below process the old nr pages after split is successful? It might
> be an optimization for current large anon folio only case, but it is
> more like an issue in the future.



Thanks Zi Yan, this makes sense.
After the split, each page is a separate order-0 folio so setting nr = 1 
lets each folio go through folio_free_swap() and mapping lookup 
independently. I will fold this into v3.

Regards,
Arvind

>
>


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

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

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-24  8:27 [PATCH v2] mm/migrate_device: Clear stale mapping after freeing swapcache Arvind Yadav
2026-07-24 14:00 ` Zi Yan
2026-07-25  4:43 ` Andrew Morton
2026-07-25 19:36   ` David Hildenbrand (Arm)
2026-07-25 21:05     ` Zi Yan
2026-07-27  0:38       ` Balbir Singh
2026-07-27  1:46         ` Zi Yan
2026-07-27  2:24           ` Balbir Singh
2026-07-27  5:06           ` Yadav, Arvind
2026-07-27  3:06 ` Balbir Singh

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.