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

[Balbir Singh]

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