* [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-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
* 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox