Re: [PATCH v4] drm/pagemap: Prevent double migration of device pages
"Yadav, Arvind" <[email protected]>
| Newsgroups | org.freedesktop.lists.intel-xe,org.freedesktop.lists.dri-devel |
|---|---|
| Message-ID | <[email protected]> |
On 08-08-2026 04:37, Matthew Brost wrote:
> On Fri, Aug 07, 2026 at 01:42:44PM +0530, Arvind Yadav wrote:
>> A device-private folio migrated to system memory by a CPU fault can
>> remain reachable through the raw-PFN eviction path until migration
>> finalization drops the source reference.
>>
>> If eviction selects the same device folio during this window, it can
>> attempt to migrate the folio again. The second migration can leave an
>> uncharged folio on an LRU list, causing folio_lruvec_lock_irqsave() to
>> retry indefinitely and resulting in a soft lockup and RCU stall.
>>
>> Mark successfully migrated device folios using a low bit of their
>> zone_device_data before migration finalization. Make raw-PFN eviction
>> skip folios carrying this flag.
>>
>> Mask the flag when retrieving the drm_pagemap_zdd pointer and preserve
>> it when a device folio is split. Keeping the state on the physical folio
>> also avoids depending on a virtual address that may change before a
>> fault occurs.
>>
>> v2:
>> - Replace the retired-PFN XArray with an embedded bitmap.(Matthew Brost)
>> - Mark every base page covered by a migrated folio so retirement remains
>> valid if the folio is later split.
>>
>> v3:
>> - Store the migrated state in a low bit of zone_device_data instead of
>> adding virtual-range and bitmap tracking to the ZDD. (Matthew Brost)
>> - Mask the flag when retrieving the ZDD and preserve it when splitting
>> a folio.
>> - Drop the pre-existing fixes already covered by Matthew Brost's series:
>> https://patchwork.freedesktop.org/series/171651/
>>
>> v4:
>> - Advance by the folio size only for migration entries marked with
>> MIGRATE_PFN_COMPOUND. (Sashiko)
>>
>> The lockup was observed as:
>> [10109.860465] watchdog: BUG: soft lockup - CPU#9 stuck for 26s! [kworker/u65:5:6557]
>> [10109.860524] Tainted: [S]=CPU_OUT_OF_SPEC, [O]=OOT_MODULE
>> [10109.860524] Hardware name: ASUS System Product Name/PRIME Z790-P WIFI, BIOS 0812 02/24/2023
>> [10109.860525] Workqueue: xe_page_fault_work_queue xe_pagefault_queue_work [xe]
>> [10109.860644] RIP: 0010:_raw_spin_unlock_irqrestore+0x57/0x80
>> [10109.860655] Call Trace:
>> [10109.860655] <TASK>
>> [10109.860657] folio_lruvec_lock_irqsave+0x216/0x220
>> [10109.860661] ? __pfx_lru_add+0x10/0x10
>> [10109.860665] folio_batch_move_lru+0xc8/0x450
>> [10109.860670] ? lock_acquire+0xc4/0x2d0
>> [10109.860674] ? __folio_batch_add_and_move+0x60/0x2e0
>> [10109.860677] ? folio_migrate_mapping+0xa6/0x110
>> [10109.860679] ? folio_migrate_flags+0x13b/0x1b0
>> [10109.860681] ? __pfx_lru_add+0x10/0x10
>> [10109.860683] __folio_batch_add_and_move+0xe7/0x2e0
>> [10109.860685] ? dma_iova_try_alloc+0xb0/0x140
>> [10109.860689] folio_add_lru+0x64/0x80
>> [10109.860691] __migrate_device_finalize+0x12c/0x270
>> [10109.860695] migrate_device_finalize+0x10/0x20
>> [10109.860698] drm_pagemap_evict_to_ram+0x185/0x370 [drm_gpusvm_helper]
>> [10109.860704] ? drm_pagemap_evict_to_ram+0x96/0x370 [drm_gpusvm_helper]
>> [10109.860709] xe_svm_bo_evict+0x15/0x20 [xe]
>> [10109.860819] ? xe_svm_bo_evict+0x15/0x20 [xe]
>> [10109.860921] xe_bo_move+0x107e/0x1570 [xe]
>> [10109.860992] ? xe_ttm_tt_create+0x168/0x340 [xe]
>> [10109.861059] ? __up_read+0x98/0x2b0
>> [10109.861061] ? lock_is_held_type+0xa3/0x130
>> [10109.861067] ttm_bo_handle_move_mem+0xe8/0x1e0 [ttm]
>> [10109.861075] ttm_bo_evict+0x141/0x1c0 [ttm]
>> [10109.861081] ttm_bo_evict_cb+0x9f/0x100 [ttm]
>> [10109.861086] ttm_lru_walk_for_evict+0x84/0x190 [ttm]
>> [10109.861091] ? xe_ttm_vram_mgr_new+0x258/0x3a0 [xe]
>> [10109.861198] ttm_bo_alloc_resource+0x219/0x750 [ttm]
>> [10109.861203] ? ttm_bo_alloc_resource+0xa9/0x750 [ttm]
>> [10109.861208] ? lock_acquire+0xc4/0x2d0
>> [10109.861214] ttm_bo_validate+0x94/0x1c0 [ttm]
>> [10109.861218] ? ww_mutex_trylock+0x19d/0x3d0
>> [10109.861219] ? _raw_write_unlock+0x22/0x50
>> [10109.861223] ttm_bo_init_reserved+0x17d/0x1f0 [ttm]
>> [10109.861228] xe_bo_init_locked+0x20a/0x620 [xe]
>> [10109.861294] ? __pfx_xe_ttm_bo_destroy+0x10/0x10 [xe]
>> [10109.861359] ? mark_held_locks+0x46/0x90
>> [10109.861361] ? __create_object+0x68/0xc0
>> [10109.861366] __xe_bo_create_locked+0x384/0xa20 [xe]
>> [10109.861432] ? lock_acquire+0xc4/0x2d0
>> [10109.861434] ? xe_drm_pagemap_populate_mm+0xd3/0x340 [xe]
>> [10109.861542] xe_bo_create_locked+0x23/0x40 [xe]
>> [10109.861609] xe_drm_pagemap_populate_mm+0x12e/0x340 [xe]
>> [10109.861707] ? __lock_acquire+0x43e/0x2930
>> [10109.861716] drm_pagemap_populate_mm+0x74/0xe0 [drm_gpusvm_helper]
>> [10109.861720] xe_svm_alloc_vram+0xb5/0x2c0 [xe]
>> [10109.861817] ? seqcount_lockdep_reader_access.constprop.0+0x9f/0xc0
>> [10109.861819] ? ktime_get+0x23/0x130
>> [10109.861821] ? trace_hardirqs_on+0x22/0xe0
>> [10109.861823] ? seqcount_lockdep_reader_access.constprop.0+0x9f/0xc0
>> [10109.861826] __xe_svm_handle_pagefault+0x77d/0xbf0 [xe]
>> [10109.861924] ? rwsem_down_write_slowpath+0x43a/0x9a0
>> [10109.861926] ? _raw_spin_unlock_irq+0x27/0x70
>> [10109.861928] ? rwsem_down_write_slowpath+0x43a/0x9a0
>> [10109.861929] ? trace_hardirqs_on+0x22/0xe0
>> [10109.861931] ? _raw_spin_unlock_irq+0x27/0x70
>> [10109.861933] ? rwsem_down_write_slowpath+0x459/0x9a0
>> [10109.861937] xe_svm_handle_pagefault+0x3d/0xb0 [xe]
>> [10109.862030] xe_pagefault_queue_work+0x1a9/0x520 [xe]
>> [10109.862122] process_one_work+0x239/0x730
>> [10109.862127] worker_thread+0x200/0x3f0
>> [10109.862130] ? __pfx_worker_thread+0x10/0x10
>> [10109.862132] kthread+0x10d/0x150
>> [10109.862133] ? __pfx_kthread+0x10/0x10
>> [10109.862135] ret_from_fork+0x3bd/0x470
>> [10109.862138] ? __pfx_kthread+0x10/0x10
>> [10109.862140] ret_from_fork_asm+0x1a/0x30
>> [10109.862146] </TASK>
>>
>> Fixes: 99624bdff867 ("drm/gpusvm: Add support for GPU Shared Virtual Memory")
>> Cc: Maarten Lankhorst<[email protected]>
>> Cc: Maxime Ripard<[email protected]>
>> Cc: Matthew Brost<[email protected]>
> Mostly nits.
>
>> Cc: Thomas Zimmermann<[email protected]>
>> Cc: David Airlie<[email protected]>
>> Cc: Simona Vetter<[email protected]>
>> Cc: Thomas Hellström<[email protected]>
>> Cc: Himal Prasad Ghimiray<[email protected]>
>> Assisted-by: Claude:claude-opus-4-8
>> Suggested-by: Matthew Brost<[email protected]>
>> Signed-off-by: Arvind Yadav<[email protected]>
>> ---
>> drivers/gpu/drm/drm_pagemap.c | 124 ++++++++++++++++++++++++++++++++--
>> include/drm/drm_pagemap.h | 8 ++-
>> 2 files changed, 127 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
>> index 892b325fa99b..c1112d492d64 100644
>> --- a/drivers/gpu/drm/drm_pagemap.c
>> +++ b/drivers/gpu/drm/drm_pagemap.c
>> @@ -1102,12 +1102,119 @@ void drm_pagemap_put(struct drm_pagemap *dpagemap)
>> }
>> EXPORT_SYMBOL(drm_pagemap_put);
>>
>> +/**
>> + * drm_pagemap_page_get_flags() - Read the drm_pagemap flags of a device folio
>> + * @page: Pointer to a page of the device folio
>> + *
>> + * Return: The DRM_PAGEMAP_ZDD_FLAG_* bits encoded in zone_device_data.
>> + */
>> +static unsigned long drm_pagemap_page_get_flags(struct page *page)
>> +{
>> + struct folio *folio = page_folio(page);
>> +
>> + return (unsigned long)folio_zone_device_data(folio) &
>> + DRM_PAGEMAP_ZDD_FLAG_MASK;
>> +}
>> +
>> +/**
>> + * drm_pagemap_page_set_flags() - Set the drm_pagemap flags of a device folio
>> + * @page: Pointer to a page of the device folio
>> + * @flags: DRM_PAGEMAP_ZDD_FLAG_* bits to set
>> + *
>> + * Preserve any flags already encoded alongside the ZDD pointer.
>> + */
>> +static void drm_pagemap_page_set_flags(struct page *page,
>> + unsigned long flags)
>> +{
>> + struct folio *folio = page_folio(page);
>> + struct drm_pagemap_zdd *zdd;
>> +
>> + if (WARN_ON_ONCE(flags & ~DRM_PAGEMAP_ZDD_FLAG_MASK))
>> + return;
>> +
>> + zdd = drm_pagemap_page_zone_device_data(page);
>> + flags |= drm_pagemap_page_get_flags(page);
>> + folio_set_zone_device_data(folio,
>> + (void *)((unsigned long)zdd | flags));
> One could write this like:
>
> unsigned long old;
>
> old = (unsigned long)folio_zone_device_data(folio);
> folio_set_zone_device_data(folio, (void *)(old | flags));
Noted,
>
>> +}
>> +
>> +/**
>> + * drm_pagemap_retire_migrated_pages() - Record migrated device folios
>> + * @src_pfns: source array after migrate_vma_pages() or migrate_device_pages()
>> + * @npages: number of entries in @src_pfns
>> + *
>> + * Flag device folios that were successfully migrated to RAM before finalize
>> + * unlocks the sources. The migrated state is stored in the physical folio, so
>> + * it survives later folio splits and a subsequent raw-PFN eviction can skip it.
>> + */
>> +static void drm_pagemap_retire_migrated_pages(unsigned long *src_pfns,
>> + unsigned long npages)
>> +{
>> + unsigned long i = 0;
>> +
>> + while (i < npages) {
>> + struct page *page = migrate_pfn_to_page(src_pfns[i]);
>> + unsigned long nr = 1;
>> +
>> + if (!page) {
>> + i++;
>> + continue;
>> + }
>> +
>> + if (src_pfns[i] & MIGRATE_PFN_COMPOUND)
>> + nr = folio_nr_pages(page_folio(page));
> You could omit the 'src_pfns[i] & MIGRATE_PFN_COMPOUND' check as
> folio_nr_pages returns 1 for non-compound pages.
Agreed. Populated migration entries refer to the folio head, and
|folio_nr_pages()| returns 1 for an order-0 folio. I’ll drop the
compound check.
>
>> +
>> + if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
>> + is_device_private_page(page))
> When this called from drm_pagemap_evict_to_ram this could be a coherrent
> page. I'd just drop the page type check.
I think this check should remain. The zone-device-data helpers warn for
non-private pages, and this fix only applies to device-private pages.
>
>> + drm_pagemap_page_set_flags(page,
>> + DRM_PAGEMAP_ZDD_FLAG_MIGRATED);
>> +
>> + i += nr;
> You could drop nr and just make this:
>
> i += folio_nr_pages(page_folio(page));
Noted,
>
>> + }
>> +}
>> +
>> +/**
>> + * drm_pagemap_skip_retired_pages() - Drop retired PFNs from a raw-PFN eviction
>> + * @src_pfns: source array after migrate_device_pfns() (MIGRATE_PFN encoded)
>> + * @npages: number of entries in @src_pfns
>> + *
>> + * Skip source folios already migrated to RAM, identified by the migrated flag
>> + * stored in the physical folio's zone_device_data.
>> + */
>> +static void drm_pagemap_skip_retired_pages(unsigned long *src_pfns,
>> + unsigned long npages)
>> +{
>> + unsigned long i = 0;
>> +
>> + while (i < npages) {
>> + struct page *page = migrate_pfn_to_page(src_pfns[i]);
>> + unsigned long nr = 1;
>> +
>> + if (!page) {
>> + i++;
>> + continue;
>> + }
>> +
>> + if (src_pfns[i] & MIGRATE_PFN_COMPOUND)
>> + nr = folio_nr_pages(page_folio(page));
> Same as above wrt MIGRATE_PFN_COMPOUND check not being needed.
Agreed, same change here.
>
>> +
>> + if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
>> + is_device_private_page(page) &&
> Same as above wrt device_private_page.
Agreed, same change here.
>
>> + (drm_pagemap_page_get_flags(page) &
>> + DRM_PAGEMAP_ZDD_FLAG_MIGRATED))
>> + src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
>> +
>> + i += nr;
> Same as above about dropping NR.
Agreed, same change here.
>
>> + }
>> +}
>> +
>> /**
>> * drm_pagemap_evict_to_ram() - Evict GPU SVM range to RAM
>> * @devmem_allocation: Pointer to the device memory allocation
>> *
>> - * Similar to __drm_pagemap_migrate_to_ram but does not require mmap lock and
>> - * migration done via migrate_device_* functions.
>> + * Similar to __drm_pagemap_migrate_to_ram(), but uses the
>> + * migrate_device_* helpers and does not require the mmap lock. Device
>> + * PFNs already migrated to RAM by either migration path are skipped.
>> *
>> * Return: 0 on success, negative error code on failure.
>> */
>> @@ -1148,6 +1255,8 @@ int drm_pagemap_evict_to_ram(struct drm_pagemap_devmem *devmem_allocation)
>> if (err)
>> goto err_free;
>>
>> + drm_pagemap_skip_retired_pages(src, npages);
>> +
>> err = drm_pagemap_migrate_populate_ram_pfn(NULL, NULL, npages, &mpages,
>> src, dst, 0);
>> if (err || !mpages)
>> @@ -1178,6 +1287,7 @@ int drm_pagemap_evict_to_ram(struct drm_pagemap_devmem *devmem_allocation)
>> if (err)
>> drm_pagemap_migration_unlock_put_pages(npages, dst);
>> migrate_device_pages(src, dst, npages);
>> + drm_pagemap_retire_migrated_pages(src, npages);
> Since we retire the pages here too, should the
> __drm_pagemap_migrate_to_ram also call drm_pagemap_skip_retired_pages?
Agreed. I will call |drm_pagemap_skip_retired_pages()| after
|migrate_vma_setup()| and before destination allocation.
>
>> migrate_device_finalize(src, dst, npages);
>> drm_pagemap_migrate_unmap_pages(devmem_allocation->dev, pagemap_addr, dst, npages,
>> DMA_FROM_DEVICE, &state);
>> @@ -1308,6 +1418,7 @@ static int __drm_pagemap_migrate_to_ram(struct vm_area_struct *vas,
>> if (err)
>> drm_pagemap_migration_unlock_put_pages(npages, migrate.dst);
>> migrate_vma_pages(&migrate);
>> + drm_pagemap_retire_migrated_pages(migrate.src, npages);
>> migrate_vma_finalize(&migrate);
>> if (dev)
>> drm_pagemap_migrate_unmap_pages(dev, pagemap_addr, migrate.dst,
>> @@ -1360,13 +1471,18 @@ static vm_fault_t drm_pagemap_migrate_to_ram(struct vm_fault *vmf)
>> static void drm_pagemap_folio_split(struct folio *orig_folio, struct folio *new_folio)
>> {
>> struct drm_pagemap_zdd *zdd;
>> + struct page *page;
>> + unsigned long flags;
>>
>> if (!new_folio)
>> return;
>>
>> new_folio->pgmap = orig_folio->pgmap;
>> - zdd = folio_zone_device_data(orig_folio);
>> - folio_set_zone_device_data(new_folio, drm_pagemap_zdd_get(zdd));
>> + page = folio_page(orig_folio, 0);
>> + zdd = drm_pagemap_page_zone_device_data(page);
>> + flags = drm_pagemap_page_get_flags(page);
> folio_set_zone_device_data(new_folio, folio_zone_device_data(orig_folio));
>
> ?
We cannot copy it directly because the new folio needs its own ZDD
reference. I will keep drm_pagemap_zdd_get() and preserve the flag bits.
Thanks,
Arvind
>
> Matt
>
>> + folio_set_zone_device_data(new_folio,
>> + (void *)((unsigned long)drm_pagemap_zdd_get(zdd) | flags));
>> }
>>
>> static const struct dev_pagemap_ops drm_pagemap_pagemap_ops = {
>> diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
>> index 95eb4b66b057..ebbd3b0ddf36 100644
>> --- a/include/drm/drm_pagemap.h
>> +++ b/include/drm/drm_pagemap.h
>> @@ -2,6 +2,7 @@
>> #ifndef _DRM_PAGEMAP_H_
>> #define _DRM_PAGEMAP_H_
>>
>> +#include <linux/bits.h>
>> #include <linux/dma-direction.h>
>> #include <linux/hmm.h>
>> #include <linux/memremap.h>
>> @@ -339,6 +340,9 @@ struct drm_pagemap_migrate_details {
>>
>> #if IS_ENABLED(CONFIG_ZONE_DEVICE)
>>
>> +#define DRM_PAGEMAP_ZDD_FLAG_MIGRATED BIT(0)
>> +#define DRM_PAGEMAP_ZDD_FLAG_MASK DRM_PAGEMAP_ZDD_FLAG_MIGRATED
>> +
>> int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
>> struct mm_struct *mm,
>> unsigned long start, unsigned long end,
>> @@ -373,7 +377,9 @@ static inline struct drm_pagemap_zdd *drm_pagemap_page_zone_device_data(struct p
>> {
>> struct folio *folio = page_folio(page);
>>
>> - return folio_zone_device_data(folio);
>> + return (struct drm_pagemap_zdd *)
>> + ((unsigned long)folio_zone_device_data(folio) &
>> + ~DRM_PAGEMAP_ZDD_FLAG_MASK);
>> }
>>
>> #else
>> --
>> 2.43.0
>>