All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Arvind Yadav <arvind.yadav@intel.com>
Cc: <dri-devel@lists.freedesktop.org>,
	<intel-xe@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
	<thomas.hellstrom@linux.intel.com>,
	<maarten.lankhorst@linux.intel.com>, <mripard@kernel.org>,
	<tzimmermann@suse.de>, <airlied@gmail.com>, <simona@ffwll.ch>,
	<himal.prasad.ghimiray@intel.com>
Subject: Re: [PATCH v6] drm/pagemap: Prevent double migration of device pages
Date: Mon, 10 Aug 2026 12:16:27 -0700	[thread overview]
Message-ID: <anoji9hN/ifeoMwl@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <20260810092845.2776097-1-arvind.yadav@intel.com>

On Mon, Aug 10, 2026 at 02:58:45PM +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-private 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-private folios using a low bit of
> their zone_device_data before migration finalization. Make both CPU-fault
> and raw-PFN migration paths skip device-private folios carrying this
> flag.
> 
> Mask the flag when retrieving the drm_pagemap_zdd pointer and preserve
> it when a device-private 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)
> 
> v5:
> - Simplify ZDD flag updates and folio iteration. (Matthew Brost)
> - Skip retired device-private folios in the CPU-fault path. (Matthew Brost)
> - Preserve flag bits while taking a new ZDD reference for split folios.
> 
> v6:
> - Restore MIGRATE_PFN_COMPOUND-aware stepping so non-compound migration
>   entries are processed one at a time. (Sashiko)
> - Drop the pre-existing fixes already covered by Matthew Brost's series:
>   https://patchwork.freedesktop.org/series/171651/
> 
> 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 <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Assisted-by: Claude:claude-opus-4-8
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> ---
>  drivers/gpu/drm/drm_pagemap.c | 127 ++++++++++++++++++++++++++++++++--
>  include/drm/drm_pagemap.h     |   8 ++-
>  2 files changed, 129 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> index 7a056592ac66..097a900cf55d 100644
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c
> @@ -1102,12 +1102,117 @@ void drm_pagemap_put(struct drm_pagemap *dpagemap)
>  }
>  EXPORT_SYMBOL(drm_pagemap_put);
>  
> +/**
> + * drm_pagemap_page_get_flags() - Read flags from a device-private folio
> + * @page: Pointer to a page of the device-private 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 flags on a device-private folio
> + * @page: Pointer to a page of the device-private 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);
> +	unsigned long old;
> +
> +	if (WARN_ON_ONCE(flags & ~DRM_PAGEMAP_ZDD_FLAG_MASK))
> +		return;
> +
> +	old = (unsigned long)folio_zone_device_data(folio);
> +	folio_set_zone_device_data(folio, (void *)(old | flags));
> +}
> +
> +/**
> + * drm_pagemap_retire_migrated_pages() - Record migrated device-private folios
> + * @src_pfns: source array after migrate_vma_pages() or migrate_device_pages()
> + * @npages: number of entries in @src_pfns
> + *
> + * Flag device-private folios 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 subsequent migrations 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));
> +
> +		if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
> +		    is_device_private_page(page))
> +			drm_pagemap_page_set_flags(page,
> +						   DRM_PAGEMAP_ZDD_FLAG_MIGRATED);
> +
> +		i += nr;
> +	}
> +}
> +
> +/**
> + * drm_pagemap_skip_retired_pages() - Skip retired device-private folios
> + * @src_pfns: MIGRATE_PFN-encoded source array
> + * @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));
> +
> +		if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
> +		    is_device_private_page(page) &&
> +		    (drm_pagemap_page_get_flags(page) &
> +		     DRM_PAGEMAP_ZDD_FLAG_MIGRATED))
> +			src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
> +
> +		i += nr;
> +	}
> +}
> +
>  /**
>   * 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-private PFNs already migrated to RAM by either path are skipped.
>   *
>   * Return: 0 on success, negative error code on failure.
>   */
> @@ -1149,6 +1254,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)
> @@ -1179,6 +1286,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);
>  	migrate_device_finalize(src, dst, npages);
>  	drm_pagemap_migrate_unmap_pages(devmem_allocation->dev, pagemap_addr, dst, npages,
>  					DMA_FROM_DEVICE, &state);
> @@ -1276,13 +1384,15 @@ static int __drm_pagemap_migrate_to_ram(struct vm_area_struct *vas,
>  	if (!migrate.cpages)
>  		goto err_free;
>  
> +	drm_pagemap_skip_retired_pages(migrate.src, npages);
> +
>  	ops = zdd->devmem_allocation->ops;
>  	dev = zdd->devmem_allocation->dev;
>  
>  	err = drm_pagemap_migrate_populate_ram_pfn(vas, page, npages, &mpages,
>  						   migrate.src, migrate.dst,
>  						   start);
> -	if (err)
> +	if (err || !mpages)
>  		goto err_finalize;
>  
>  	err = drm_pagemap_migrate_map_system_pages(dev, pagemap_addr,
> @@ -1309,6 +1419,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,
> @@ -1361,13 +1472,19 @@ 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;
> +	unsigned long orig_data, new_data;
>  
>  	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));
> +
> +	orig_data = (unsigned long)folio_zone_device_data(orig_folio);
> +	zdd = (struct drm_pagemap_zdd *)(orig_data & ~DRM_PAGEMAP_ZDD_FLAG_MASK);
> +
> +	new_data = (unsigned long)drm_pagemap_zdd_get(zdd);
> +	new_data |= orig_data & DRM_PAGEMAP_ZDD_FLAG_MASK;
> +	folio_set_zone_device_data(new_folio, (void *)new_data);
>  }
>  
>  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
> 

      parent reply	other threads:[~2026-08-10 19:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-10  9:28 [PATCH v6] drm/pagemap: Prevent double migration of device pages Arvind Yadav
2026-08-10  9:35 ` ✓ CI.KUnit: success for drm/pagemap: Prevent double migration of device pages (rev7) Patchwork
2026-08-10  9:51 ` [PATCH v6] drm/pagemap: Prevent double migration of device pages sashiko-bot
2026-08-10 10:16 ` ✓ Xe.CI.BAT: success for drm/pagemap: Prevent double migration of device pages (rev7) Patchwork
2026-08-10 11:28 ` ✗ Xe.CI.FULL: failure " Patchwork
2026-08-10 19:16 ` Matthew Brost [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=anoji9hN/ifeoMwl@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=airlied@gmail.com \
    --cc=arvind.yadav@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=thomas.hellstrom@linux.intel.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.