Re: [PATCH] drm/xe/bo: Take a runtime PM ref when shrinking a bo needing invalidation

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

On Tue, 2026-08-25 at 22:48 +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_bo_trigger_rebind() -> xe_vm_invalidate_vma() and submits a TLB
> invalidation over GuC CT.  The non-purgeable branch reaches the same
> code through ttm_bo_shrink(.allow_move = true) -> xe_bo_move().
> 
> When the device is runtime suspended the CT is disabled and the send
> returns -ENODEV, tripping the XE_WARN_ON() in xe_bo_trigger_rebind():
> 
>   WARNING: drivers/gpu/drm/xe/xe_bo.c:770 at
> xe_bo_move_notify+0x1fc/0x450 [xe], CPU#2: xe_madvise/21389
>    xe_bo_shrink+0x20f/0x2b0 [xe]
>    __xe_shrinker_walk+0x174/0x410 [xe]
>    xe_shrinker_walk+0x56/0xf0 [xe]
>    xe_shrinker_scan+0x10c/0x1e0 [xe]
>    do_shrink_slab+0x176/0x7e0
>    shrink_slab+0x137/0x990
>    drop_slab+0x7f/0x130
>    drop_caches_sysctl_handler+0x9c/0xf0
> 
> The invalidation is always issued for a fault-mode vm; since commit
> 4e7ebff69aed ("drm/xe/xe3p_lpg: flush shrinker bo cachelines
> manually")
> it is also issued for a non-fault-mode vm on hardware with an
> optimized
> L2 flush, which is how this surfaced.
> 
> Add bo_needs_invalidate(), mirroring that condition, and use it in
> xe_bo_shrink() to compute needs_rpm ahead of both branches.  The
> reference is then held across xe_bo_move_notify() in either path, and
> is
> not taken for a bo whose mappings would not have been invalidated.
> 
> Shrinking can run in reclaim contexts where the device may not be
> resumed, so a bo is skipped when the reference cannot be acquired. 
> Have
> that skip queue the shrinker PM worker: xe_shrinker_runtime_pm_get()
> is
> gated on the needs of its own backup pass rather than on this one,
> and
> on DGFX it returns before queueing anything, so without this a scan
> where every candidate is skipped makes no progress, reports nothing
> scanned and returns SHRINK_STOP with nothing arranging a wake.
> 
> Also gate the System CCS term on !xe_tt->purgeable, since
> xe_bo_shrink_purge() frees the pages without a GPU copy.
> 
> Reproduced with igt@xe_madvise@dontneed-before-exec while the GPU is
> runtime suspended.
> 
> Fixes: 00c8efc3180f ("drm/xe: Add a shrinker for xe bos")
> Assisted-by: Claude:claude-opus-5
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>

Nice catch.

I wonder, however, can we skip the TLB flush if runtime PM is not
available at TLB flush time (runtime_pm_get_if_active()?) Assuming that
if runtime PM is not available, no contexts can be active and they will
flush TLB implicitly when becoming active?

IIRC It's not totally clear whether this would work for
"has_ctx_tlb_inval" hardware, and if so we might need to add something
similar to this for that hardware.

Thanks,
Thomas

> ---
>  drivers/gpu/drm/xe/xe_bo.c       | 62 +++++++++++++++++++++++++++---
> --
>  drivers/gpu/drm/xe/xe_shrinker.c | 18 +++++++++-
>  drivers/gpu/drm/xe/xe_shrinker.h |  2 ++
>  3 files changed, 72 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 2eb5d6aac523..274ff97d8542 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -734,6 +734,32 @@ static int xe_ttm_io_mem_reserve(struct
> ttm_device *bdev,
>  	}
>  }
>  
> +/*
> + * Whether xe_bo_move_notify() will invalidate the GPU mappings of
> @bo, and
> + * therefore needs the device resumed to reach the GuC.
> + *
> + * This mirrors the condition under which xe_bo_trigger_rebind()
> below calls
> + * xe_vm_invalidate_vma(): always for a fault-mode vm, and for any
> bound vm on
> + * hardware where the L2 flush is optimized. Keep the two in sync.
> + *
> + * Context: Caller must hold the BO's dma-resv lock.
> + */
> +static bool bo_needs_invalidate(struct xe_bo *bo)
> +{
> +	struct drm_gpuvm_bo *vm_bo;
> +
> +	if (xe_device_is_l2_flush_optimized(xe_bo_device(bo)))
> +		return xe_bo_is_vm_bound(bo);
> +
> +	xe_bo_assert_held(bo);
> +
> +	drm_gem_for_each_gpuvm_bo(vm_bo, &bo->ttm.base)
> +		if (xe_vm_in_fault_mode(gpuvm_to_vm(vm_bo->vm)))
> +			return true;
> +
> +	return false;
> +}
> +
>  static int xe_bo_trigger_rebind(struct xe_device *xe, struct xe_bo
> *bo,
>  				const struct ttm_operation_ctx *ctx)
>  {
> @@ -1361,6 +1387,28 @@ 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);
>  
> +	/*
> +	 * Moving this bo out of a non-system placement makes
> +	 * xe_bo_move_notify() invalidate its GPU mappings over GuC
> CT, and
> +	 * System CCS needs a gpu copy when moving PL_TT ->
> PL_SYSTEM. Both
> +	 * need the device resumed.
> +	 */
> +	needs_rpm = bo->resource->mem_type != XE_PL_SYSTEM &&
> +		(bo_needs_invalidate(xe_bo) ||
> +		 (!xe_tt->purgeable && !IS_DGFX(xe) &&
> +		  xe_bo_needs_ccs_pages(xe_bo)));
> +	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;
> +	}
> +
>  	if (xe_tt->purgeable) {
>  		if (bo->resource->mem_type != XE_PL_SYSTEM)
>  			lret = xe_bo_move_notify(xe_bo, ctx);
> @@ -1369,28 +1417,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