All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ryan Neph <ryanneph@google.com>
To: Matthew Brost <matthew.brost@intel.com>
Cc: intel-xe@lists.freedesktop.org, Carlos Santa <carlos.santa@intel.com>
Subject: Re: [PATCH 2/2] drm/xe: Set BO TTM priority based on VM exec queue priority band
Date: Tue, 14 Jul 2026 11:49:07 -0700	[thread overview]
Message-ID: <alaClrPwc_3hr8yK@google.com> (raw)
In-Reply-To: <20260713234606.3367215-2-matthew.brost@intel.com>

Testing now, but the corner-case you anticipated is showing itself; i.e.
a client that allocates before creating exec_queues has those BOs with
the default XE_BO_PRIORITY_NORMAL.

In vkDeviceCreate(), ANV creates the sole VM, allocs several MB worth of
internal BOs [1], then creates the exec_queues.

For an application that is expected to create its exec_queue(s) with
XE_EXEC_QUEUE_PRIORITY_LOW, when its BOs get shrunk I see a mixture of
LOW and NORMAL priority BOs.

ANV could reorder device creation to:
create VM -> create exec_queues -> alloc internal BOs

Or Xe can add the BO priority as a property of BO allocation or VM creation
ioctls.

[1]: https://gitlab.freedesktop.org/mesa/mesa/-/blob/ed807097ebdea0b490ce4d091619ca1c325e296c/src/intel/vulkan/anv_device.c#L851-1246

Best,
Ryan

On Mon, Jul 13, 2026 at 04:46:06PM -0700, Matthew Brost wrote:
> Add XE_BO_PRIORITY_LOW/HIGH/HIGHEST defines alongside the existing
> XE_BO_PRIORITY_NORMAL, and use them in xe_bo_init_locked() to select
> the TTM LRU priority for a newly created bo.
> 
> Kernel internal buffers (non ttm_bo_type_device) and buffers that can
> be shared cross-process, i.e. not private to a single VM (no VM
> pointer available), should not be evicted ahead of user buffers, so
> they are always given the highest priority. For buffers private to a
> user VM, prioritize the buffer based on the highest priority band of
> any exec queue userspace has created against that VM, since that
> reflects the importance userspace has assigned to work using the VM:
> 
>  - ttm_bo_type != ttm_bo_type_device, or no VM: XE_BO_PRIORITY_HIGHEST
>  - VM with a HIGH priority exec queue: XE_BO_PRIORITY_HIGH
>  - VM with a NORMAL priority exec queue: XE_BO_PRIORITY_NORMAL
>  - VM with a LOW priority exec queue: XE_BO_PRIORITY_LOW
>  - VM with no exec queues at all: XE_BO_PRIORITY_NORMAL
> 
> This relies on the per-priority-band exec queue counts tracked in
> vm->exec_queues.priority_count, read under vm->exec_queues.lock in
> read mode. xe_bo_init_locked() now takes a struct xe_vm * parameter;
> all callers that create ttm_bo_type_kernel/ttm_bo_type_sg bos pass
> NULL since they always get XE_BO_PRIORITY_HIGHEST regardless.
> 
> Cc: Carlos Santa <carlos.santa@intel.com>
> Cc: Ryan Neph <ryanneph@google.com>
> Assisted-by: GitHub_Copilot:claude-sonnet-5
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c       | 28 +++++++++++++++++++++++-----
>  drivers/gpu/drm/xe/xe_bo.h       |  3 ++-
>  drivers/gpu/drm/xe/xe_bo_types.h |  3 +++
>  drivers/gpu/drm/xe/xe_dma_buf.c  |  2 +-
>  4 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 85e6d9a0f575..9da75ade8eaf 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1346,7 +1346,8 @@ int xe_bo_notifier_prepare_pinned(struct xe_bo *bo)
>  		if (bo->flags & XE_BO_FLAG_PINNED_NORESTORE)
>  			break;
>  
> -		backup = xe_bo_init_locked(xe, NULL, NULL, bo->ttm.base.resv, NULL, xe_bo_size(bo),
> +		backup = xe_bo_init_locked(xe, NULL, NULL, NULL, bo->ttm.base.resv, NULL,
> +					   xe_bo_size(bo),
>  					   DRM_XE_GEM_CPU_CACHING_WB, ttm_bo_type_kernel,
>  					   XE_BO_FLAG_SYSTEM | XE_BO_FLAG_NEEDS_CPU_ACCESS |
>  					   XE_BO_FLAG_PINNED, &exec);
> @@ -1486,7 +1487,7 @@ int xe_bo_evict_pinned(struct xe_bo *bo)
>  			break;
>  
>  		if (!backup) {
> -			backup = xe_bo_init_locked(xe, NULL, NULL, bo->ttm.base.resv, NULL,
> +			backup = xe_bo_init_locked(xe, NULL, NULL, NULL, bo->ttm.base.resv, NULL,
>  						   xe_bo_size(bo),
>  						   DRM_XE_GEM_CPU_CACHING_WB, ttm_bo_type_kernel,
>  						   XE_BO_FLAG_SYSTEM | XE_BO_FLAG_NEEDS_CPU_ACCESS |
> @@ -2276,6 +2277,9 @@ void xe_bo_free(struct xe_bo *bo)
>   * if the function should allocate a new one.
>   * @tile: The tile to select for migration of this bo, and the tile used for
>   * GGTT binding if any. Only to be non-NULL for ttm_bo_type_kernel bos.
> + * @vm: The VM this bo will be associated with, used to determine the bo's
> + * TTM LRU priority based on the priority band of the VM's exec queues, or
> + * NULL if the bo is not associated with a user VM.
>   * @resv: Pointer to a locked shared reservation object to use for this bo,
>   * or NULL for the xe_bo to use its own.
>   * @bulk: The bulk move to use for LRU bumping, or NULL for external bos.
> @@ -2291,7 +2295,8 @@ void xe_bo_free(struct xe_bo *bo)
>   * Return: The buffer object on success. Negative error pointer on failure.
>   */
>  struct xe_bo *xe_bo_init_locked(struct xe_device *xe, struct xe_bo *bo,
> -				struct xe_tile *tile, struct dma_resv *resv,
> +				struct xe_tile *tile, struct xe_vm *vm,
> +				struct dma_resv *resv,
>  				struct ttm_lru_bulk_move *bulk, size_t size,
>  				u16 cpu_caching, enum ttm_bo_type type,
>  				u32 flags, struct drm_exec *exec)
> @@ -2353,7 +2358,20 @@ struct xe_bo *xe_bo_init_locked(struct xe_device *xe, struct xe_bo *bo,
>  	bo->flags = flags;
>  	bo->cpu_caching = cpu_caching;
>  	bo->ttm.base.funcs = &xe_gem_object_funcs;
> -	bo->ttm.priority = XE_BO_PRIORITY_NORMAL;
> +	if (type != ttm_bo_type_device || !vm) {
> +		bo->ttm.priority = XE_BO_PRIORITY_HIGHEST;
> +	} else {
> +		down_read(&vm->exec_queues.lock);
> +		if (vm->exec_queues.priority_count[XE_EXEC_QUEUE_PRIORITY_HIGH])
> +			bo->ttm.priority = XE_BO_PRIORITY_HIGH;
> +		else if (vm->exec_queues.priority_count[XE_EXEC_QUEUE_PRIORITY_NORMAL])
> +			bo->ttm.priority = XE_BO_PRIORITY_NORMAL;
> +		else if (vm->exec_queues.priority_count[XE_EXEC_QUEUE_PRIORITY_LOW])
> +			bo->ttm.priority = XE_BO_PRIORITY_LOW;
> +		else
> +			bo->ttm.priority = XE_BO_PRIORITY_NORMAL;
> +		up_read(&vm->exec_queues.lock);
> +	}
>  	INIT_LIST_HEAD(&bo->pinned_link);
>  #ifdef CONFIG_PROC_FS
>  	INIT_LIST_HEAD(&bo->client_link);
> @@ -2496,7 +2514,7 @@ __xe_bo_create_locked(struct xe_device *xe,
>  		}
>  	}
>  
> -	bo = xe_bo_init_locked(xe, bo, tile, vm ? xe_vm_resv(vm) : NULL,
> +	bo = xe_bo_init_locked(xe, bo, tile, vm, vm ? xe_vm_resv(vm) : NULL,
>  			       vm && !xe_vm_in_fault_mode(vm) &&
>  			       flags & XE_BO_FLAG_USER ?
>  			       &vm->lru_bulk_move : NULL, size,
> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> index 6340317f7d2e..12625740a47a 100644
> --- a/drivers/gpu/drm/xe/xe_bo.h
> +++ b/drivers/gpu/drm/xe/xe_bo.h
> @@ -115,7 +115,8 @@ struct xe_bo *xe_bo_alloc(void);
>  void xe_bo_free(struct xe_bo *bo);
>  
>  struct xe_bo *xe_bo_init_locked(struct xe_device *xe, struct xe_bo *bo,
> -				struct xe_tile *tile, struct dma_resv *resv,
> +				struct xe_tile *tile, struct xe_vm *vm,
> +				struct dma_resv *resv,
>  				struct ttm_lru_bulk_move *bulk, size_t size,
>  				u16 cpu_caching, enum ttm_bo_type type,
>  				u32 flags, struct drm_exec *exec);
> diff --git a/drivers/gpu/drm/xe/xe_bo_types.h b/drivers/gpu/drm/xe/xe_bo_types.h
> index fcc63ae3f455..95904b03c71f 100644
> --- a/drivers/gpu/drm/xe/xe_bo_types.h
> +++ b/drivers/gpu/drm/xe/xe_bo_types.h
> @@ -24,7 +24,10 @@ struct xe_vm;
>  #define XE_BO_MAX_PLACEMENTS	3
>  
>  /* TODO: To be selected with VM_MADVISE */
> +#define	XE_BO_PRIORITY_LOW	0
>  #define	XE_BO_PRIORITY_NORMAL	1
> +#define	XE_BO_PRIORITY_HIGH	2
> +#define	XE_BO_PRIORITY_HIGHEST	3
>  
>  /**
>   * struct xe_bo - Xe buffer object
> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c
> index 8a920e58245c..a80831fabd1d 100644
> --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> @@ -300,7 +300,7 @@ xe_dma_buf_create_obj(struct drm_device *dev, struct dma_buf *dma_buf)
>  		if (ret)
>  			break;
>  
> -		bo = xe_bo_init_locked(xe, NULL, NULL, resv, NULL, dma_buf->size,
> +		bo = xe_bo_init_locked(xe, NULL, NULL, NULL, resv, NULL, dma_buf->size,
>  				       0, /* Will require 1way or 2way for vm_bind */
>  				       ttm_bo_type_sg, XE_BO_FLAG_SYSTEM, &exec);
>  		drm_exec_retry_on_contention(&exec);
> -- 
> 2.34.1

-- 

Best,
Ryan Neph

  reply	other threads:[~2026-07-14 18:49 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-13 23:46 [PATCH 1/2] drm/xe: Track exec queue priority band counts for user VMs Matthew Brost
2026-07-13 23:46 ` [PATCH 2/2] drm/xe: Set BO TTM priority based on VM exec queue priority band Matthew Brost
2026-07-14 18:49   ` Ryan Neph [this message]
2026-07-15  2:32     ` Matthew Brost
2026-07-14  1:08 ` ✓ CI.KUnit: success for series starting with [1/2] drm/xe: Track exec queue priority band counts for user VMs Patchwork
2026-07-14  1:56 ` ✓ Xe.CI.BAT: " Patchwork
2026-07-14  7:21 ` ✗ Xe.CI.FULL: failure " 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=alaClrPwc_3hr8yK@google.com \
    --to=ryanneph@google.com \
    --cc=carlos.santa@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.brost@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.