Re: [PATCH v2] drm/xe/bo: Take a runtime PM ref when shrinking a non-system bo

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Fri, 2026-08-28 at 18:28 +0000, Shuicheng Lin wrote:
> xe_bo_shrink() only took a runtime PM reference for the System CCS
> backup case, and only after the purgeable branch had already
> returned.
> That branch calls xe_bo_move_notify(), which reaches
> xe_vm_invalidate_vma(): it zaps the page table entries and submits a
> TLB
> invalidation over GuC CT.  The non-purgeable branch reaches the same
> code
> through ttm_bo_shrink() -> xe_bo_move().
> 
> With the device runtime suspended the zap trips the assert in
> xe_map_memset(), and the CT is disabled so the invalidation returns
> -ENODEV:
> 
>   Assertion `!xe_pm_runtime_suspended(xe)` failed!
>    xe_pt_zap_ptes_entry+0xb8/0x120 [xe]
>    xe_vm_invalidate_vma_submit+0xb1/0x770 [xe]
> 
>   WARNING: drivers/gpu/drm/xe/xe_bo.c:770 at
> xe_bo_move_notify+0x1fc/0x450 [xe]
>    xe_bo_shrink+0x20f/0x2b0 [xe]
>    __xe_shrinker_walk+0x174/0x410 [xe]
>    xe_shrinker_scan+0x10c/0x1e0 [xe]
>    do_shrink_slab+0x176/0x7e0
>    drop_caches_sysctl_handler+0x9c/0xf0
> 
> Compute needs_rpm ahead of both branches so the reference is held
> across
> xe_bo_move_notify() in either path.  A bo already in XE_PL_SYSTEM
> needs
> neither the invalidation nor the CCS copy, so it stays reclaimable
> while
> the device is suspended.
> 
> A bo is skipped when the reference cannot be acquired, since resuming
> is
> not allowed from every reclaim context.  Queue the shrinker PM worker
> from there: xe_shrinker_runtime_pm_get() is gated on the needs of its
> own
> backup pass and returns early on DGFX, so otherwise a scan that skips
> every candidate returns SHRINK_STOP with nothing arranging a wake.
> 
> Only a non-fault-mode vm can reach this.  A fault-mode vm requires LR
> mode, which holds a runtime PM reference for the vm's lifetime, so
> the
> device cannot be suspended while one exists.
> 
> Reproduced with igt@xe_madvise@dontneed-after-exec while the GPU is
> runtime suspended.
> 
> v2: simplify needs_rpm check. (Matt)
>     retarget Fixes tag since issue occurs with non-fault-mode path
> added
>     by 4e7ebff69aed.
> 
> Fixes: 4e7ebff69aed ("drm/xe/xe3p_lpg: flush shrinker bo cachelines
> manually")
> Assisted-by: Claude:claude-opus-5
> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c       | 28 +++++++++++++++++++---------
>  drivers/gpu/drm/xe/xe_shrinker.c | 18 +++++++++++++++++-
>  drivers/gpu/drm/xe/xe_shrinker.h |  2 ++
>  3 files changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 2eb5d6aac523..95c168f6eb48 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1361,6 +1361,20 @@ long xe_bo_shrink(struct ttm_operation_ctx
> *ctx, struct ttm_buffer_object *bo,
>  	if (!xe_bo_is_xe_bo(bo) || !xe_bo_get_unless_zero(xe_bo))
>  		return xe_bo_shrink_purge(ctx, bo, scanned);
>  
> +	/* Both the invalidation and the System CCS copy need the
> device. */
> +	needs_rpm = bo->resource->mem_type != XE_PL_SYSTEM;
> +	if (needs_rpm && !xe_pm_runtime_get_if_active(xe)) {
> +		/*
> +		 * Resuming is not allowed from all reclaim
> contexts, so leave
> +		 * this bo alone and ask for the device to be woken
> up outside
> +		 * of reclaim. xe_shrinker_runtime_pm_get() is gated
> on the
> +		 * needs of its own backup pass rather than on ours,
> and on
> +		 * DGFX it returns before queueing anything, so ask
> here.
> +		 */
> +		xe_shrinker_queue_pm(xe->mem.shrinker);
> +		goto out_unref;
> +	}
> +

Rather than scheduling a waking work within the xe_bo code, I'd like to
keep all the waking logic within xe_shrinker.c. Could we look at
modifying / simplifying xe_shrinker_runtime_pm_get() instead? 

Also note that there is a possibility since we have a loop over memory
types in __xe_shrinker_walk() to defer wakeup until we actually hit a
non-system memory type.

Also please avoid in-function comments unless they are really
necessesary. Claude has a tendency to sprinkle these all over the code
so I typically ask it as a follow up pass to look at all added comments
focusing on "What rather than How" and remove all in-function comment
unless they are crucial for code clarity.

Thanks,
Thomas



>  	if (xe_tt->purgeable) {
>  		if (bo->resource->mem_type != XE_PL_SYSTEM)
>  			lret = xe_bo_move_notify(xe_bo, ctx);
> @@ -1369,28 +1383,24 @@ long xe_bo_shrink(struct ttm_operation_ctx
> *ctx, struct ttm_buffer_object *bo,
>  		if (lret > 0 && xe_bo_madv_is_dontneed(xe_bo))
>  			xe_bo_set_purgeable_state(xe_bo,
>  						 
> XE_MADV_PURGEABLE_PURGED);
> -		goto out_unref;
> +		goto out_put_rpm;
>  	}
>  
> -	/* System CCS needs gpu copy when moving PL_TT -> PL_SYSTEM
> */
> -	needs_rpm = (!IS_DGFX(xe) && bo->resource->mem_type !=
> XE_PL_SYSTEM &&
> -		     xe_bo_needs_ccs_pages(xe_bo));
> -	if (needs_rpm && !xe_pm_runtime_get_if_active(xe))
> -		goto out_unref;
> -
>  	*scanned += tt->num_pages;
>  	lret = ttm_bo_shrink(ctx, bo, (struct ttm_bo_shrink_flags)
>  			     {.purge = false,
>  			      .writeback = flags.writeback,
>  			      .allow_move = true});
> -	if (needs_rpm)
> -		xe_pm_runtime_put(xe);
>  
>  	if (lret > 0) {
>  		xe_ttm_tt_account_subtract(xe, tt);
>  		update_global_total_pages(bo->bdev, -(long)tt-
> >num_pages);
>  	}
>  
> +out_put_rpm:
> +	if (needs_rpm)
> +		xe_pm_runtime_put(xe);
> +
>  out_unref:
>  	xe_bo_put(xe_bo);
>  
> diff --git a/drivers/gpu/drm/xe/xe_shrinker.c
> b/drivers/gpu/drm/xe/xe_shrinker.c
> index 83374cd57660..fb6ec77972a5 100644
> --- a/drivers/gpu/drm/xe/xe_shrinker.c
> +++ b/drivers/gpu/drm/xe/xe_shrinker.c
> @@ -54,6 +54,22 @@ xe_shrinker_mod_pages(struct xe_shrinker
> *shrinker, long shrinkable, long purgea
>  	write_unlock(&shrinker->lock);
>  }
>  
> +/**
> + * xe_shrinker_queue_pm() - Ask for the device to be woken up for
> shrinking
> + * @shrinker: Pointer to the struct xe_shrinker.
> + *
> + * Queue a worker that takes and drops a runtime PM reference.
> Shrinking can
> + * be called from reclaim context, where resuming the device is not
> always
> + * allowed, so a caller that needs the device resumed but could not
> acquire a
> + * reference uses this to have it woken up outside of reclaim. The
> current
> + * scan makes no progress on the affected buffer objects, but a
> subsequent one
> + * can.
> + */
> +void xe_shrinker_queue_pm(struct xe_shrinker *shrinker)
> +{
> +	queue_work(shrinker->xe->unordered_wq, &shrinker-
> >pm_worker);
> +}
> +
>  static s64 __xe_shrinker_walk(struct xe_device *xe,
>  			      struct ttm_operation_ctx *ctx,
>  			      const struct xe_bo_shrink_flags flags,
> @@ -185,7 +201,7 @@ static bool xe_shrinker_runtime_pm_get(struct
> xe_shrinker *shrinker, bool force,
>  			xe_pm_runtime_get(xe);
>  			return true;
>  		}
> -		queue_work(xe->unordered_wq, &shrinker->pm_worker);
> +		xe_shrinker_queue_pm(shrinker);
>  		return false;
>  	}
>  
> diff --git a/drivers/gpu/drm/xe/xe_shrinker.h
> b/drivers/gpu/drm/xe/xe_shrinker.h
> index 5132ae5192e1..86d2a322cadd 100644
> --- a/drivers/gpu/drm/xe/xe_shrinker.h
> +++ b/drivers/gpu/drm/xe/xe_shrinker.h
> @@ -11,6 +11,8 @@ struct xe_device;
>  
>  void xe_shrinker_mod_pages(struct xe_shrinker *shrinker, long
> shrinkable, long purgeable);
>  
> +void xe_shrinker_queue_pm(struct xe_shrinker *shrinker);
> +
>  int xe_shrinker_create(struct xe_device *xe);
>  
>  #endif