Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: <intel-xe@lists.freedesktop.org>,
	Matthew Auld <matthew.auld@intel.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Subject: Re: [PATCH v4 3/4] drm/xe: Split the bo shrinker into purgeable and non-purgeable shrinkers
Date: Fri, 16 Oct 2026 11:16:23 -0700	[thread overview]
Message-ID: <atJp90kXhJceIa/i@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <20261016133523.3622-4-thomas.hellstrom@linux.intel.com>

On Fri, Oct 16, 2026 at 03:35:22PM +0200, Thomas Hellström wrote:
> Purgeable objects are reclaimed by simply discarding their pages, with
> no writeback required. They are therefore much cheaper to shrink than
> regular shrinkable (non-purgeable) objects, whose reclaim may need to
> wait for the GPU or write dirty compressed contents back to a backup
> store, and whose contents need to be read back on next use.
> 
> Split the single xe_shrinker into two independent struct shrinker
> instances registered against the shrinker core, sharing the same
> underlying accounting (shrinkable_pages, purgeable_pages,
> populated_tts) and locking:
> 
>  - A purgeable shrinker, whose count_objects() reports only
>    purgeable_pages, and whose scan_objects() only purges objects.
> 
>  - A non-purgeable (shrinkable) shrinker, whose count_objects()
>    reports the shrinkable page count (capped by backup availability)
>    and whose scan_objects() shrinks non-purgeable objects, and also
>    purges any purgeable objects it encounters, waking the device via
>    runtime PM as needed.
> 
> Express the cost difference to the shrinker core through the seeks
> value: the purgeable shrinker keeps DEFAULT_SEEKS, while the
> non-purgeable shrinker uses a higher value,
> XE_SHRINKER_NON_PURGEABLE_SEEKS (8). The core scales each shrinker's
> scan target inversely with seeks, so purgeable objects receive
> proportionally more reclaim pressure than non-purgeable ones, without
> one shrinker's behaviour depending on the other's state.
> 
> The standalone purge walk relies on the LRU walk skipping
> non-purgeable objects, for which xe_bo_shrink() returns -EBUSY,
> rather than aborting.
> 
> v3:
> - Patch introduced. (Matt Brost)
> 
> v4:
> - The purge walk aborting at the first non-purgeable object is now
>   fixed by a separate, preceding patch. (Sashiko)
> 
> Assisted-by: LLM
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>  drivers/gpu/drm/xe/xe_shrinker.c | 109 +++++++++++++++++++------------
>  1 file changed, 68 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_shrinker.c b/drivers/gpu/drm/xe/xe_shrinker.c
> index 378022c37a57..8e3fe675e0f3 100644
> --- a/drivers/gpu/drm/xe/xe_shrinker.c
> +++ b/drivers/gpu/drm/xe/xe_shrinker.c
> @@ -14,6 +14,15 @@
>  #include "xe_pm.h"
>  #include "xe_shrinker.h"
>  
> +/*
> + * Reclaiming non-purgeable objects requires GPU idling and writeback to a
> + * backup store, and restoring them requires reading the contents back.
> + * Tell the shrinker core they're costlier to recreate than purgeable
> + * objects, which use DEFAULT_SEEKS, so that purgeable objects are
> + * preferentially reclaimed.
> + */
> +#define XE_SHRINKER_NON_PURGEABLE_SEEKS 8

I suggest for full tuning this tied to a Kconfig + plus syfs entry like
I have here [1] so customers can play around with various configs.

[1] https://patchwork.freedesktop.org/series/176251/

Most everything every LGTM, but I think the Sashiko comment it somewhat
valid, I'll reply there.

Matt

> +
>  /**
>   * struct xe_shrinker - per-device shrinker
>   * @xe: Back pointer to the device.
> @@ -21,7 +30,8 @@
>   * @shrinkable_pages: Number of pages that are currently shrinkable.
>   * @purgeable_pages: Number of pages that are currently purgeable.
>   * @populated_tts: Number of populated ttm_tts currently shrinkable or purgeable.
> - * @shrink: Pointer to the mm shrinker.
> + * @shrink: Pointer to the mm shrinker handling non-purgeable (shrinkable) objects.
> + * @purge_shrink: Pointer to the mm shrinker handling purgeable objects.
>   * @pm_worker: Worker to wake up the device if required.
>   */
>  struct xe_shrinker {
> @@ -31,6 +41,7 @@ struct xe_shrinker {
>  	long purgeable_pages;
>  	long populated_tts;
>  	struct shrinker *shrink;
> +	struct shrinker *purge_shrink;
>  	struct work_struct pm_worker;
>  };
>  
> @@ -179,6 +190,19 @@ static int xe_shrinker_walk(struct xe_shrinker *shrinker,
>  	return ret;
>  }
>  
> +static unsigned long
> +xe_shrinker_count_purgeable(struct shrinker *shrink, struct shrink_control *sc)
> +{
> +	struct xe_shrinker *shrinker = to_xe_shrinker(shrink);
> +	unsigned long num_pages;
> +
> +	read_lock(&shrinker->lock);
> +	num_pages = shrinker->purgeable_pages;
> +	read_unlock(&shrinker->lock);
> +
> +	return num_pages ? num_pages : SHRINK_EMPTY;
> +}
> +
>  static unsigned long
>  xe_shrinker_count(struct shrinker *shrink, struct shrink_control *sc)
>  {
> @@ -194,7 +218,6 @@ xe_shrinker_count(struct shrinker *shrink, struct shrink_control *sc)
>  	else
>  		num_pages = 0;
>  
> -	num_pages += shrinker->purgeable_pages;
>  	read_unlock(&shrinker->lock);
>  
>  	return num_pages ? num_pages : SHRINK_EMPTY;
> @@ -207,8 +230,7 @@ xe_shrinker_count(struct shrinker *shrink, struct shrink_control *sc)
>   * If bo shrinking needs an rpm reference and we don't have it (yet),
>   * that bo will be skipped anyway.
>   */
> -static bool xe_shrinker_runtime_pm_get(struct xe_shrinker *shrinker, bool force,
> -				       unsigned long nr_to_scan, bool can_backup)
> +static bool xe_shrinker_runtime_pm_get(struct xe_shrinker *shrinker)
>  {
>  	struct xe_device *xe = shrinker->xe;
>  
> @@ -216,63 +238,52 @@ static bool xe_shrinker_runtime_pm_get(struct xe_shrinker *shrinker, bool force,
>  	    !ttm_backup_bytes_avail())
>  		return false;
>  
> -	if (!force) {
> -		read_lock(&shrinker->lock);
> -		force = (nr_to_scan > shrinker->purgeable_pages && can_backup);
> -		read_unlock(&shrinker->lock);
> -		if (!force)
> -			return false;
> -	}
> -
>  	return __xe_shrinker_runtime_pm_get(shrinker);
>  }
>  
> -static unsigned long xe_shrinker_scan(struct shrinker *shrink, struct shrink_control *sc)
> +static unsigned long xe_shrinker_scan_purgeable(struct shrinker *shrink,
> +						struct shrink_control *sc)
>  {
>  	struct xe_shrinker *shrinker = to_xe_shrinker(shrink);
>  	struct ttm_operation_ctx ctx = {
>  		.interruptible = false,
>  		.no_wait_gpu = ttm_bo_shrink_avoid_wait(),
>  	};
> -	unsigned long nr_to_scan, nr_scanned = 0, freed = 0;
> +	unsigned long nr_to_scan = sc->nr_to_scan, nr_scanned = 0, freed = 0;
>  	struct xe_bo_shrink_flags shrink_flags = {
>  		.purge = true,
> -		/* Don't request writeback without __GFP_IO. */
> -		.writeback = !ctx.no_wait_gpu && (sc->gfp_mask & __GFP_IO),
>  	};
> -	bool runtime_pm;
> -	bool purgeable;
> -	bool can_backup = !!(sc->gfp_mask & __GFP_FS);
> -
> -	nr_to_scan = sc->nr_to_scan;
> -
> -	read_lock(&shrinker->lock);
> -	purgeable = !!shrinker->purgeable_pages;
> -	read_unlock(&shrinker->lock);
>  
> -	/* Might need runtime PM. Try to wake early if it looks like it. */
> -	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, &ctx, shrink_flags,
> -				 nr_to_scan, &nr_scanned, &freed);
> +	xe_shrinker_walk(shrinker, &ctx, shrink_flags, nr_to_scan, &nr_scanned, &freed);
>  
>  	sc->nr_scanned = nr_scanned;
> -	if (nr_scanned >= nr_to_scan || !can_backup)
> -		goto out;
>  
> -	/* If we didn't wake before, try to do it now if needed. */
> -	if (!runtime_pm)
> -		runtime_pm = xe_shrinker_runtime_pm_get(shrinker, true, 0, can_backup);
> +	return nr_scanned ? freed : SHRINK_STOP;
> +}
>  
> -	shrink_flags.purge = false;
> +static unsigned long xe_shrinker_scan(struct shrinker *shrink, struct shrink_control *sc)
> +{
> +	struct xe_shrinker *shrinker = to_xe_shrinker(shrink);
> +	struct ttm_operation_ctx ctx = {
> +		.interruptible = false,
> +		.no_wait_gpu = ttm_bo_shrink_avoid_wait(),
> +	};
> +	unsigned long nr_to_scan = sc->nr_to_scan, nr_scanned = 0, freed = 0;
> +	struct xe_bo_shrink_flags shrink_flags = {
> +		.purge = false,
> +		/* Don't request writeback without __GFP_IO. */
> +		.writeback = !ctx.no_wait_gpu && (sc->gfp_mask & __GFP_IO),
> +	};
> +	bool runtime_pm;
> +
> +	/* Might need runtime PM to access backed-up or CCS-compressed data. */
> +	runtime_pm = xe_shrinker_runtime_pm_get(shrinker);
>  
> -	xe_shrinker_walk(shrinker, &ctx, shrink_flags,
> -			 nr_to_scan, &nr_scanned, &freed);
> +	xe_shrinker_walk(shrinker, &ctx, shrink_flags, nr_to_scan, &nr_scanned, &freed);
>  
>  	sc->nr_scanned = nr_scanned;
> -out:
>  	xe_shrinker_runtime_pm_put(shrinker, runtime_pm);
> +
>  	return nr_scanned ? freed : SHRINK_STOP;
>  }
>  
> @@ -294,6 +305,7 @@ static void xe_shrinker_fini(struct drm_device *drm, void *arg)
>  	xe_assert(shrinker->xe, !shrinker->purgeable_pages);
>  	xe_assert(shrinker->xe, !shrinker->populated_tts);
>  	shrinker_free(shrinker->shrink);
> +	shrinker_free(shrinker->purge_shrink);
>  	flush_work(&shrinker->pm_worker);
>  	kfree(shrinker);
>  }
> @@ -311,8 +323,15 @@ int xe_shrinker_create(struct xe_device *xe)
>  	if (!shrinker)
>  		return -ENOMEM;
>  
> -	shrinker->shrink = shrinker_alloc(0, "drm-xe_gem:%s", xe->drm.unique);
> +	shrinker->purge_shrink = shrinker_alloc(0, "drm-xe_gem-purgeable:%s", xe->drm.unique);
> +	if (!shrinker->purge_shrink) {
> +		kfree(shrinker);
> +		return -ENOMEM;
> +	}
> +
> +	shrinker->shrink = shrinker_alloc(0, "drm-xe_gem-shrinkable:%s", xe->drm.unique);
>  	if (!shrinker->shrink) {
> +		shrinker_free(shrinker->purge_shrink);
>  		kfree(shrinker);
>  		return -ENOMEM;
>  	}
> @@ -320,10 +339,18 @@ int xe_shrinker_create(struct xe_device *xe)
>  	INIT_WORK(&shrinker->pm_worker, xe_shrinker_pm);
>  	shrinker->xe = xe;
>  	rwlock_init(&shrinker->lock);
> +
> +	shrinker->purge_shrink->count_objects = xe_shrinker_count_purgeable;
> +	shrinker->purge_shrink->scan_objects = xe_shrinker_scan_purgeable;
> +	shrinker->purge_shrink->private_data = shrinker;
> +	shrinker_register(shrinker->purge_shrink);
> +
>  	shrinker->shrink->count_objects = xe_shrinker_count;
>  	shrinker->shrink->scan_objects = xe_shrinker_scan;
>  	shrinker->shrink->private_data = shrinker;
> +	shrinker->shrink->seeks = XE_SHRINKER_NON_PURGEABLE_SEEKS;
>  	shrinker_register(shrinker->shrink);
> +
>  	xe->mem.shrinker = shrinker;
>  
>  	return drmm_add_action_or_reset(&xe->drm, xe_shrinker_fini, shrinker);
> -- 
> 2.56.0
> 

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

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-16 13:35 [PATCH v4 0/4] drm/xe: Adjust the bo shrinker batch size Thomas Hellström
2026-10-16 13:35 ` [PATCH v4 1/4] drm/xe: Don't abort the shrinker walk on -EBUSY Thomas Hellström
2026-10-16 13:44   ` sashiko-bot
2026-10-16 13:55   ` Matthew Brost
2026-10-16 13:35 ` [PATCH v4 2/4] drm/xe: Track number of populated ttm_tts in the shrinker Thomas Hellström
2026-10-16 14:26   ` Matthew Brost
2026-10-16 13:35 ` [PATCH v4 3/4] drm/xe: Split the bo shrinker into purgeable and non-purgeable shrinkers Thomas Hellström
2026-10-16 13:53   ` sashiko-bot
2026-10-16 14:09     ` Thomas Hellström
2026-10-16 18:41       ` Matthew Brost
2026-10-16 18:16   ` Matthew Brost [this message]
2026-10-16 13:35 ` [PATCH v4 4/4] drm/xe: Update non-purgeable shrinker batch size based on average BO size Thomas Hellström
2026-10-16 13:45   ` sashiko-bot
2026-10-16 14:06     ` Thomas Hellström
2026-10-16 18:42       ` Matthew Brost
2026-10-16 18:44   ` Matthew Brost
2026-10-16 13:45 ` ✓ CI.KUnit: success for drm/xe: Adjust the bo shrinker batch size (rev4) Patchwork
2026-10-16 14:37 ` ✓ Xe.CI.BAT: " Patchwork
2026-10-16 22:14 ` ✓ 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=atJp90kXhJceIa/i@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=matthew.auld@intel.com \
    --cc=thomas.hellstrom@linux.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox