All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: Shuicheng Lin <shuicheng.lin@intel.com>, intel-xe@lists.freedesktop.org
Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>,
	Matthew Brost <matthew.brost@intel.com>
Subject: Re: [PATCH v6 2/2] drm/xe/shrinker: Take a runtime PM ref before shrinking non-system memory
Date: Mon, 07 Sep 2026 10:23:49 +0200	[thread overview]
Message-ID: <4fd1fca025af55962a5b58b1f5b7e970ac0c51f4.camel@linux.intel.com> (raw)
In-Reply-To: <20260904215049.543535-3-shuicheng.lin@intel.com>

On Fri, 2026-09-04 at 21:50 +0000, Shuicheng Lin wrote:
> __xe_shrinker_walk() walks the SYSTEM and TT LRUs without a runtime
> PM
> reference.  Shrinking a bo outside system memory invalidates its GPU
> mappings, which needs the device resumed, so while it is runtime
> suspended the page table zap trips an assert and the TLB invalidation
> returns -ENODEV:
> 
>   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
> 
> Take a reference before walking a memory type other than XE_PL_SYSTEM
> and stop there if it cannot be acquired.  Reuse the shrinker's
> existing
> acquire path, which resumes the device directly where reclaim allows
> that and otherwise queues the PM worker for a later scan.  Stop the
> walk
> once the scan target is met, so a satisfied scan does not wake the
> device.  System memory is still reclaimed while the device is
> suspended.
> 
> Gate this on xe_device_is_l2_flush_optimized(), the same condition
> under
> which xe_bo_trigger_rebind() issues the invalidation for a non-fault-
> mode
> vm, so reclaim is unaffected elsewhere.  The System CCS copy already
> has
> its own reference in xe_bo_shrink().
> 
> Only a non-fault-mode vm can reach this, since a fault-mode vm
> requires
> LR mode and that holds a runtime PM reference for the vm's lifetime.
> 
> Reproduced with igt@xe_madvise@dontneed-before-exec while the GPU is
> runtime suspended.
> 
> v2: simplify needs_rpm check. (Matt)
>     retarget Fixes tag since the issue occurs with the non-fault-mode
>     path added by 4e7ebff69aed.
> v3: handle this in xe_shrinker.c instead of xe_bo.c (Thomas)
> v4: stop the walk once the scan target is met. (Sashiko)
> v5: rebase on the freed page accounting fix. (Sashiko)
> v6: reuse the shrinker acquire path so runtime pm can be resumed
>     directly instead of always queueing a worker. (Thomas)
> 
> 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_shrinker.c | 67 ++++++++++++++++++++++--------
> --
>  1 file changed, 46 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_shrinker.c
> b/drivers/gpu/drm/xe/xe_shrinker.c
> index 89445cd20238..13f49ac873b4 100644
> --- a/drivers/gpu/drm/xe/xe_shrinker.c
> +++ b/drivers/gpu/drm/xe/xe_shrinker.c
> @@ -54,13 +54,33 @@ xe_shrinker_mod_pages(struct xe_shrinker
> *shrinker, long shrinkable, long purgea
>  	write_unlock(&shrinker->lock);
>  }
>  
> -static int __xe_shrinker_walk(struct xe_device *xe,
> +static bool __xe_shrinker_runtime_pm_get(struct xe_shrinker
> *shrinker)
> +{
> +	struct xe_device *xe = shrinker->xe;
> +
> +	if (xe_pm_runtime_get_if_active(xe))
> +		return true;
> +
> +	if (xe_rpm_reclaim_safe(xe) && !ttm_bo_shrink_avoid_wait())
> {
> +		xe_pm_runtime_get(xe);
> +		return true;
> +	}
> +
> +	queue_work(xe->unordered_wq, &shrinker->pm_worker);
> +
> +	return false;
> +}
> +
> +static int __xe_shrinker_walk(struct xe_shrinker *shrinker,
>  			      struct ttm_operation_ctx *ctx,
>  			      const struct xe_bo_shrink_flags flags,
>  			      unsigned long to_scan, unsigned long
> *scanned,
>  			      unsigned long *freed)
>  {
> +	struct xe_device *xe = shrinker->xe;
>  	unsigned int mem_type;
> +	bool rpm = false;
> +	int ret = 0;
>  	s64 lret;
>  
>  	for (mem_type = XE_PL_SYSTEM; mem_type <= XE_PL_TT;
> ++mem_type) {
> @@ -75,23 +95,36 @@ static int __xe_shrinker_walk(struct xe_device
> *xe,
>  		if (!man || !man->use_tt)
>  			continue;
>  
> +		if (mem_type != XE_PL_SYSTEM && !rpm &&
> +		    xe_device_is_l2_flush_optimized(xe)) {
> +			if (!__xe_shrinker_runtime_pm_get(shrinker))
> +				break;
> +			rpm = true;
> +		}
> +
>  		ttm_bo_lru_for_each_reserved_guarded(&curs, man,
> &arg, ttm_bo) {
>  			if (!ttm_bo_shrink_suitable(ttm_bo, ctx))
>  				continue;
>  
>  			lret = xe_bo_shrink(ctx, ttm_bo, flags,
> scanned);
> -			if (lret < 0)
> -				return lret;
> +			if (lret < 0) {
> +				ret = lret;
> +				goto out;
> +			}
>  
>  			*freed += lret;
>  			if (*scanned >= to_scan)
> -				break;
> +				goto out;
>  		}
>  		/* Trylocks should never error, just fail. */
>  		xe_assert(xe, !IS_ERR(ttm_bo));
>  	}
>  
> -	return 0;
> +out:
> +	if (rpm)
> +		xe_pm_runtime_put(xe);

xe_shrinker_runtime_pm_put()

With that
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>


> +
> +	return ret;
>  }
>  
>  /*
> @@ -100,7 +133,7 @@ static int __xe_shrinker_walk(struct xe_device
> *xe,
>   * add writeback. This avoids stalls and explicit writebacks with
> light or
>   * moderate memory pressure.
>   */
> -static int xe_shrinker_walk(struct xe_device *xe,
> +static int xe_shrinker_walk(struct xe_shrinker *shrinker,
>  			    struct ttm_operation_ctx *ctx,
>  			    const struct xe_bo_shrink_flags flags,
>  			    unsigned long to_scan, unsigned long
> *scanned,
> @@ -112,20 +145,21 @@ static int xe_shrinker_walk(struct xe_device
> *xe,
>  
>  	swap(no_wait_gpu, ctx->no_wait_gpu);
>  	save_flags.writeback = false;
> -	ret = __xe_shrinker_walk(xe, ctx, save_flags, to_scan,
> scanned, freed);
> +	ret = __xe_shrinker_walk(shrinker, ctx, save_flags, to_scan,
> scanned,
> +				 freed);
>  	swap(no_wait_gpu, ctx->no_wait_gpu);
>  	if (ret || *scanned >= to_scan)
>  		return ret;
>  
>  	if (!ctx->no_wait_gpu) {
> -		ret = __xe_shrinker_walk(xe, ctx, save_flags,
> to_scan, scanned,
> +		ret = __xe_shrinker_walk(shrinker, ctx, save_flags,
> to_scan, scanned,
>  					 freed);
>  		if (ret || *scanned >= to_scan)
>  			return ret;
>  	}
>  
>  	if (flags.writeback)
> -		ret = __xe_shrinker_walk(xe, ctx, flags, to_scan,
> scanned,
> +		ret = __xe_shrinker_walk(shrinker, ctx, flags,
> to_scan, scanned,
>  					 freed);
>  
>  	return ret;
> @@ -176,16 +210,7 @@ static bool xe_shrinker_runtime_pm_get(struct
> xe_shrinker *shrinker, bool force,
>  			return false;
>  	}
>  
> -	if (!xe_pm_runtime_get_if_active(xe)) {
> -		if (xe_rpm_reclaim_safe(xe) &&
> !ttm_bo_shrink_avoid_wait()) {
> -			xe_pm_runtime_get(xe);
> -			return true;
> -		}
> -		queue_work(xe->unordered_wq, &shrinker->pm_worker);
> -		return false;
> -	}
> -
> -	return true;
> +	return __xe_shrinker_runtime_pm_get(shrinker);
>  }
>  
>  static void xe_shrinker_runtime_pm_put(struct xe_shrinker *shrinker,
> bool runtime_pm)
> @@ -221,7 +246,7 @@ static unsigned long xe_shrinker_scan(struct
> shrinker *shrink, struct shrink_con
>  	runtime_pm = xe_shrinker_runtime_pm_get(shrinker, false,
> nr_to_scan, can_backup);
>  
>  	if (purgeable && nr_scanned < nr_to_scan)
> -		xe_shrinker_walk(shrinker->xe, &ctx, shrink_flags,
> +		xe_shrinker_walk(shrinker, &ctx, shrink_flags,
>  				 nr_to_scan, &nr_scanned, &freed);
>  
>  	sc->nr_scanned = nr_scanned;
> @@ -234,7 +259,7 @@ static unsigned long xe_shrinker_scan(struct
> shrinker *shrink, struct shrink_con
>  
>  	shrink_flags.purge = false;
>  
> -	xe_shrinker_walk(shrinker->xe, &ctx, shrink_flags,
> +	xe_shrinker_walk(shrinker, &ctx, shrink_flags,
>  			 nr_to_scan, &nr_scanned, &freed);
>  
>  	sc->nr_scanned = nr_scanned;

  parent reply	other threads:[~2026-09-07  8:23 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-04 21:50 [PATCH v6 0/2] drm/xe/shrinker: Runtime PM reference for non-system memory Shuicheng Lin
2026-09-04 21:50 ` [PATCH v6 1/2] drm/xe/shrinker: Return the freed page count through a parameter Shuicheng Lin
2026-09-04 21:59   ` sashiko-bot
2026-09-09 12:58   ` Thomas Hellström
2026-09-04 21:50 ` [PATCH v6 2/2] drm/xe/shrinker: Take a runtime PM ref before shrinking non-system memory Shuicheng Lin
2026-09-04 22:05   ` sashiko-bot
2026-09-07  8:23   ` Thomas Hellström [this message]
2026-09-04 22:10 ` ✓ CI.KUnit: success for drm/xe/shrinker: Runtime PM reference for " Patchwork
2026-09-04 23:05 ` ✓ Xe.CI.BAT: " Patchwork
2026-09-05  4:15 ` ✓ Xe.CI.FULL: " Patchwork

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=4fd1fca025af55962a5b58b1f5b7e970ac0c51f4.camel@linux.intel.com \
    --to=thomas.hellstrom@linux.intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.brost@intel.com \
    --cc=shuicheng.lin@intel.com \
    --cc=tejas.upadhyay@intel.com \
    /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.