From: "Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>
To: Matthew Brost <matthew.brost@intel.com>,
<intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH v6 09/24] drm/xe: Add struct xe_pt_job_ops
Date: Mon, 21 Sep 2026 16:37:18 +0530 [thread overview]
Message-ID: <87f6232f-1652-41f7-9f38-ad14ee650db1@intel.com> (raw)
In-Reply-To: <20260904211613.3934307-10-matthew.brost@intel.com>
On 05-09-2026 02:45, Matthew Brost wrote:
> Add struct xe_pt_job_ops, a dynamically refcounted object that contains
> the information required to issue a CPU bind via a job after the initial
> bind IOCTL returns.
>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> Link: https://patch.msgid.link/20260228013501.106680-10-matthew.brost@intel.com
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> ---
> drivers/gpu/drm/xe/xe_migrate.c | 10 +--
> drivers/gpu/drm/xe/xe_pt.c | 136 +++++++++++++++++++++++++++----
> drivers/gpu/drm/xe/xe_pt.h | 4 +
> drivers/gpu/drm/xe/xe_pt_types.h | 27 ++++--
> drivers/gpu/drm/xe/xe_vm.c | 10 +--
> 5 files changed, 152 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 68a0b05f56ec..217e526526e0 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
> @@ -1875,7 +1875,7 @@ xe_migrate_update_pgtables_cpu(struct xe_migrate *m,
> }
>
> xe_migrate_update_pgtables_cpu_execute(vm, m->tile, ops,
> - pt_update_ops->ops,
> + pt_update_ops->pt_job_ops->ops,
> pt_update_ops->num_ops);
>
> return dma_fence_get_stub();
> @@ -1902,7 +1902,7 @@ __xe_migrate_update_pgtables(struct xe_migrate *m,
> bool usm = is_migrate && xe->info.has_usm;
>
> for (i = 0; i < pt_update_ops->num_ops; ++i) {
> - struct xe_vm_pgtable_update_op *pt_op = &pt_update_ops->ops[i];
> + struct xe_vm_pgtable_update_op *pt_op = &pt_update_ops->pt_job_ops->ops[i];
> struct xe_vm_pgtable_update *updates = pt_op->entries;
>
> num_updates += pt_op->num_entries;
> @@ -1971,7 +1971,7 @@ __xe_migrate_update_pgtables(struct xe_migrate *m,
>
> for (; i < pt_update_ops->num_ops; ++i) {
> struct xe_vm_pgtable_update_op *pt_op =
> - &pt_update_ops->ops[i];
> + &pt_update_ops->pt_job_ops->ops[i];
> struct xe_vm_pgtable_update *updates = pt_op->entries;
>
> for (; j < pt_op->num_entries; ++j, ++current_update, ++idx) {
> @@ -2008,7 +2008,7 @@ __xe_migrate_update_pgtables(struct xe_migrate *m,
> (page_ofs / sizeof(u64)) * XE_PAGE_SIZE;
> for (i = 0; i < pt_update_ops->num_ops; ++i) {
> struct xe_vm_pgtable_update_op *pt_op =
> - &pt_update_ops->ops[i];
> + &pt_update_ops->pt_job_ops->ops[i];
> struct xe_vm_pgtable_update *updates = pt_op->entries;
>
> for (j = 0; j < pt_op->num_entries; ++j) {
> @@ -2026,7 +2026,7 @@ __xe_migrate_update_pgtables(struct xe_migrate *m,
>
> for (i = 0; i < pt_update_ops->num_ops; ++i) {
> struct xe_vm_pgtable_update_op *pt_op =
> - &pt_update_ops->ops[i];
> + &pt_update_ops->pt_job_ops->ops[i];
> struct xe_vm_pgtable_update *updates = pt_op->entries;
>
> for (j = 0; j < pt_op->num_entries; ++j)
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index e560f167fdf1..30127ebf1b60 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -206,6 +206,7 @@ unsigned int xe_pt_shift(unsigned int level)
> */
> void xe_pt_destroy(struct xe_pt *pt, u32 flags, struct llist_head *deferred)
> {
> + bool added = false;
> int i;
>
> if (!pt)
> @@ -213,7 +214,19 @@ void xe_pt_destroy(struct xe_pt *pt, u32 flags, struct llist_head *deferred)
>
> XE_WARN_ON(!list_empty(&pt->bo->ttm.base.gpuva.list));
> xe_bo_unpin(pt->bo);
> - xe_bo_put_deferred(pt->bo, deferred, NULL);
> + xe_bo_put_deferred(pt->bo, deferred, &added);
> + if (added) {
> + xe_assert(pt->bo->vm->xe, !kref_read(&pt->bo->ttm.base.refcount));
> +
> + /*
> + * We need the VM present until the BO is destroyed as it shares
> + * a dma-resv and BO destroy is async. Reinit BO refcount so
> + * xe_bo_put_async can be used when the PT job ops refcount goes
> + * to zero.
> + */
Seems BO refcount logic is from previous version and might be the reason
prompting Sashiko comments.
> + xe_vm_get(pt->bo->vm);
> + pt->bo->flags |= XE_BO_FLAG_PUT_VM_ASYNC;
> + }
>
> if (pt->level > 0 && pt->num_live) {
> struct xe_pt_dir *pt_dir = as_xe_pt_dir(pt);
> @@ -2081,13 +2094,13 @@ xe_pt_commit_prepare_unbind(struct xe_vma *vma,
> static struct xe_vm_pgtable_update_op *
> to_pt_op(struct xe_vm_pgtable_update_ops *pt_update_ops, u32 op_idx)
> {
> - return &pt_update_ops->ops[op_idx];
> + return &pt_update_ops->pt_job_ops->ops[op_idx];
> }
>
> static u32
> get_current_op(struct xe_vm_pgtable_update_ops *pt_update_ops)
> {
> - return pt_update_ops->current_op;
> + return pt_update_ops->pt_job_ops->current_op;
> }
>
> static struct xe_vm_pgtable_update_op *
> @@ -2099,7 +2112,7 @@ to_current_pt_op(struct xe_vm_pgtable_update_ops *pt_update_ops)
> static void
> incr_current_op(struct xe_vm_pgtable_update_ops *pt_update_ops)
> {
> - ++pt_update_ops->current_op;
> + ++pt_update_ops->pt_job_ops->current_op;
> }
>
> static void
> @@ -2473,8 +2486,7 @@ static int op_prepare(struct xe_vm *vm,
> static void
> xe_pt_update_ops_init(struct xe_vm_pgtable_update_ops *pt_update_ops)
> {
> - init_llist_head(&pt_update_ops->deferred);
> - pt_update_ops->current_op = 0;
> + pt_update_ops->pt_job_ops->current_op = 0;
> pt_update_ops->start = ~0x0ull;
> pt_update_ops->last = 0x0ull;
> pt_update_ops->needs_svm_lock = false;
> @@ -2824,7 +2836,8 @@ xe_pt_update_ops_run(struct xe_tile *tile, struct xe_vma_ops *vops)
> to_pt_op(pt_update_ops, i);
>
> xe_pt_commit(pt_op->vma, pt_op->entries,
> - pt_op->num_entries, &pt_update_ops->deferred);
> + pt_op->num_entries,
> + &pt_update_ops->pt_job_ops->deferred);
> pt_op->vma = NULL; /* skip in xe_pt_update_ops_abort */
> }
>
> @@ -2912,19 +2925,8 @@ void xe_pt_update_ops_fini(struct xe_tile *tile, struct xe_vma_ops *vops)
> {
> struct xe_vm_pgtable_update_ops *pt_update_ops =
> &vops->pt_update_ops[tile->id];
> - int i;
>
> xe_page_reclaim_entries_put(pt_update_ops->prl.entries);
> -
> - lockdep_assert_held(&vops->vm->lock);
> - xe_vm_assert_held(vops->vm);
> -
> - for (i = 0; i < pt_update_ops->current_op; ++i) {
> - struct xe_vm_pgtable_update_op *pt_op = &pt_update_ops->ops[i];
> -
> - xe_pt_free_bind(pt_op->entries, pt_op->num_entries);
> - }
> - xe_bo_put_commit(&vops->pt_update_ops[tile->id].deferred);
> }
>
> /**
> @@ -2961,3 +2963,101 @@ void xe_pt_update_ops_abort(struct xe_tile *tile, struct xe_vma_ops *vops)
>
> xe_pt_update_ops_fini(tile, vops);
> }
> +
> +/**
> + * xe_pt_job_ops_alloc() - Allocate PT job ops
> + * @num_ops: Number of VM PT update ops
> + *
> + * Allocate PT job ops and internal array of VM PT update ops.
> + *
> + * Return: Pointer to PT job ops or NULL
> + */
> +struct xe_pt_job_ops *xe_pt_job_ops_alloc(u32 num_ops)
> +{
> + struct xe_pt_job_ops *pt_job_ops;
> +
> + pt_job_ops = kmalloc_obj(*pt_job_ops);
> + if (!pt_job_ops)
> + return NULL;
> +
> + pt_job_ops->ops = kvmalloc_array(num_ops, sizeof(*pt_job_ops->ops),
> + GFP_KERNEL);
> + if (!pt_job_ops->ops) {
> + kfree(pt_job_ops);
> + return NULL;
> + }
> +
> + pt_job_ops->current_op = 0;
> + kref_init(&pt_job_ops->refcount);
> + init_llist_head(&pt_job_ops->deferred);
> +
> + return pt_job_ops;
> +}
> +
> +/**
> + * xe_pt_job_ops_get() - Get PT job ops
> + * @pt_job_ops: PT job ops to get
> + *
> + * Take a reference to PT job ops
> + *
> + * Return: Pointer to PT job ops or NULL
> + */
> +struct xe_pt_job_ops *xe_pt_job_ops_get(struct xe_pt_job_ops *pt_job_ops)
> +{
> + if (pt_job_ops)
> + kref_get(&pt_job_ops->refcount);
> +
> + return pt_job_ops;
> +}
Although unused in this patch, but seems correct place to introduce it.
> +
> +static void xe_pt_update_ops_free(struct xe_vm_pgtable_update_op *pt_op,
> + u32 num_ops)
> +{
> + u32 i;
> +
> + for (i = 0; i < num_ops; ++i, ++pt_op)
> + xe_pt_free_bind(pt_op->entries, pt_op->num_entries);
> +}
> +
> +static void xe_pt_job_ops_destroy(struct kref *ref)
> +{
> + struct xe_pt_job_ops *pt_job_ops =
> + container_of(ref, struct xe_pt_job_ops, refcount);
> + struct llist_node *freed;
> + struct xe_bo *bo, *next;
> +
> + xe_pt_update_ops_free(pt_job_ops->ops,
> + pt_job_ops->current_op);
> +
> + freed = llist_del_all(&pt_job_ops->deferred);
> + if (freed) {
> + llist_for_each_entry_safe(bo, next, freed, freed) {
> + struct xe_bo_dev *bo_device =
> + &xe_bo_device(bo)->bo_device;
> + /*
> + * If called from run_job, we are in the dma-fencing
> + * path and cannot take dma-resv locks so use an async
> + * put.
> + */
> + if (llist_add(&bo->freed, &bo_device->async_list))
> + schedule_work(&bo_device->async_free);
> + }
> + }
> +
> + kvfree(pt_job_ops->ops);
> + kfree(pt_job_ops);
> +}
> +
> +/**
> + * xe_pt_job_ops_put() - Put PT job ops
> + * @pt_job_ops: PT job ops to put
> + *
> + * Drop a reference to PT job ops
> + */
> +void xe_pt_job_ops_put(struct xe_pt_job_ops *pt_job_ops)
> +{
> + if (!pt_job_ops)
> + return;
> +
> + kref_put(&pt_job_ops->refcount, xe_pt_job_ops_destroy);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_pt.h b/drivers/gpu/drm/xe/xe_pt.h
> index 4daeebaab5a1..5faddb8e700c 100644
> --- a/drivers/gpu/drm/xe/xe_pt.h
> +++ b/drivers/gpu/drm/xe/xe_pt.h
> @@ -49,4 +49,8 @@ bool xe_pt_zap_ptes(struct xe_tile *tile, struct xe_vma *vma);
> bool xe_pt_zap_ptes_range(struct xe_tile *tile, struct xe_vm *vm,
> struct xe_svm_range *range);
>
> +struct xe_pt_job_ops *xe_pt_job_ops_alloc(u32 num_ops);
> +struct xe_pt_job_ops *xe_pt_job_ops_get(struct xe_pt_job_ops *pt_job_ops);
> +void xe_pt_job_ops_put(struct xe_pt_job_ops *pt_job_ops);
> +
> #endif
> diff --git a/drivers/gpu/drm/xe/xe_pt_types.h b/drivers/gpu/drm/xe/xe_pt_types.h
> index a7d1bb708b69..39c5b89ce9b7 100644
> --- a/drivers/gpu/drm/xe/xe_pt_types.h
> +++ b/drivers/gpu/drm/xe/xe_pt_types.h
> @@ -91,12 +91,29 @@ struct xe_vm_pgtable_update_op {
> bool rebind;
> };
>
> +/**
> + * struct xe_pt_job_ops - Page-table update operations (dynamically allocated)
> + *
> + * This is the portion of &struct xe_vma_ops and
> + * &struct xe_vm_pgtable_update_ops that is dynamically allocated, as it
> + * must remain valid until the associated bind job completes. A reference
> + * count controls its lifetime.
> + */
> +struct xe_pt_job_ops {
> + /** @current_op: current page-table update operation */
> + u32 current_op;
> + /** @refcount: reference count */
> + struct kref refcount;
> + /** @deferred: list of deferred PT entries to destroy */
> + struct llist_head deferred;
> + /** @ops: page-table update operations */
> + struct xe_vm_pgtable_update_op *ops;
> +};
> +
> /** struct xe_vm_pgtable_update_ops: page table update operations */
> struct xe_vm_pgtable_update_ops {
> - /** @ops: operations */
> - struct xe_vm_pgtable_update_op *ops;
> - /** @deferred: deferred list to destroy PT entries */
> - struct llist_head deferred;
> + /** @pt_job_ops: PT update operations dynamic allocation*/
> + struct xe_pt_job_ops *pt_job_ops;
> /** @q: exec queue for PT operations */
> struct xe_exec_queue *q;
> /** @prl: embedded page reclaim list */
> @@ -107,8 +124,6 @@ struct xe_vm_pgtable_update_ops {
> u64 last;
> /** @num_ops: number of operations */
> u32 num_ops;
> - /** @current_op: current operations */
> - u32 current_op;
> /** @needs_svm_lock: Needs SVM lock */
> bool needs_svm_lock;
> /** @needs_invalidation: Needs invalidation */
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 753a5fc55baa..2737bd25f39a 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -681,11 +681,9 @@ static int xe_vma_ops_alloc(struct xe_vma_ops *vops, bool array_of_binds)
> if (!vops->pt_update_ops[i].num_ops)
> continue;
>
> - vops->pt_update_ops[i].ops =
> - kmalloc_objs(*vops->pt_update_ops[i].ops,
> - vops->pt_update_ops[i].num_ops,
> - GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> - if (!vops->pt_update_ops[i].ops)
> + vops->pt_update_ops[i].pt_job_ops =
> + xe_pt_job_ops_alloc(vops->pt_update_ops[i].num_ops);
> + if (!vops->pt_update_ops[i].pt_job_ops)
> return array_of_binds ? -ENOBUFS : -ENOMEM;
> }
>
> @@ -732,7 +730,7 @@ static void xe_vma_ops_fini(struct xe_vma_ops *vops)
> xe_vma_svm_prefetch_ops_fini(vops);
>
> for (i = 0; i < XE_MAX_TILES_PER_DEVICE; ++i)
> - kfree(vops->pt_update_ops[i].ops);
> + xe_pt_job_ops_put(vops->pt_update_ops[i].pt_job_ops);
With stale comment removal. Patch LGTM.
Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> }
>
> static void xe_vma_ops_incr_pt_update_ops(struct xe_vma_ops *vops, u8 tile_mask, int inc_val)
next prev parent reply other threads:[~2026-09-21 11:07 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-04 21:15 [PATCH v6 00/24] CPU binds and ULLS on migration queue Matthew Brost
2026-09-04 21:15 ` [PATCH v6 01/24] drm/xe: Drop struct xe_migrate_pt_update argument from populate/clear vfuns Matthew Brost
2026-09-04 21:15 ` [PATCH v6 02/24] drm/xe: Add xe_migrate_update_pgtables_cpu_execute helper Matthew Brost
2026-09-04 21:28 ` sashiko-bot
2026-09-04 21:15 ` [PATCH v6 03/24] drm/xe: Decouple exec queue idle check from LRC Matthew Brost
2026-09-04 21:15 ` [PATCH v6 04/24] drm/xe: Add job count to GuC exec queue snapshot Matthew Brost
2026-09-04 21:23 ` sashiko-bot
2026-09-04 21:15 ` [PATCH v6 05/24] drm/xe: Update xe_bo_put_deferred arguments to include writeback flag Matthew Brost
2026-09-04 21:15 ` [PATCH v6 06/24] drm/xe: Add XE_BO_FLAG_PUT_VM_ASYNC Matthew Brost
2026-09-04 21:33 ` sashiko-bot
2026-09-11 13:10 ` Francois Dugast
2026-09-11 19:54 ` Matthew Brost
2026-09-12 0:27 ` Matthew Brost
2026-09-04 21:15 ` [PATCH v6 07/24] drm/xe: Update scheduler job layer to support PT jobs Matthew Brost
2026-09-04 21:37 ` sashiko-bot
2026-09-11 15:24 ` Francois Dugast
2026-09-11 19:25 ` Matthew Brost
2026-09-04 21:15 ` [PATCH v6 08/24] drm/xe: Add helpers to access PT ops Matthew Brost
2026-09-04 21:15 ` [PATCH v6 09/24] drm/xe: Add struct xe_pt_job_ops Matthew Brost
2026-09-04 21:40 ` sashiko-bot
2026-09-21 11:07 ` Ghimiray, Himal Prasad [this message]
2026-09-04 21:15 ` [PATCH v6 10/24] drm/xe: Update GuC submission backend to run PT jobs Matthew Brost
2026-09-04 21:39 ` sashiko-bot
2026-09-21 11:55 ` Ghimiray, Himal Prasad
2026-09-21 17:50 ` Matthew Brost
2026-09-04 21:16 ` [PATCH v6 11/24] drm/xe: Store level in struct xe_vm_pgtable_update Matthew Brost
2026-09-21 12:53 ` Ghimiray, Himal Prasad
2026-09-04 21:16 ` [PATCH v6 12/24] drm/xe: Don't use migrate exec queue for page fault binds Matthew Brost
2026-09-21 13:12 ` Ghimiray, Himal Prasad
2026-09-04 21:16 ` [PATCH v6 13/24] drm/xe: Enable CPU binds for jobs Matthew Brost
2026-09-04 21:44 ` sashiko-bot
2026-09-21 13:41 ` Ghimiray, Himal Prasad
2026-09-24 22:48 ` Matthew Brost
2026-09-04 21:16 ` [PATCH v6 14/24] drm/xe: Remove unused arguments from xe_migrate_pt_update_ops Matthew Brost
2026-09-19 0:13 ` Lin, Shuicheng
2026-09-04 21:16 ` [PATCH v6 15/24] drm/xe: Make bind queues operate cross-tile Matthew Brost
2026-09-19 3:06 ` Lin, Shuicheng
2026-09-04 21:16 ` [PATCH v6 16/24] drm/xe: Add CPU bind layer Matthew Brost
2026-09-04 21:50 ` sashiko-bot
2026-09-21 16:44 ` Ghimiray, Himal Prasad
2026-09-21 20:20 ` Lin, Shuicheng
2026-09-04 21:16 ` [PATCH v6 17/24] drm/xe: Add device flag to enable PT mirroring across tiles Matthew Brost
2026-09-04 21:40 ` sashiko-bot
2026-09-21 17:03 ` Ghimiray, Himal Prasad
2026-09-25 3:11 ` Matthew Brost
2026-09-21 17:02 ` Ghimiray, Himal Prasad
2026-09-04 21:16 ` [PATCH v6 18/24] drm/xe: Add ULLS support to LRC Matthew Brost
2026-09-23 7:01 ` Ghimiray, Himal Prasad
2026-09-04 21:16 ` [PATCH v6 19/24] drm/xe: Add ULLS migration job support to migration layer Matthew Brost
2026-09-04 21:40 ` sashiko-bot
2026-09-04 21:16 ` [PATCH v6 20/24] drm/xe: Add ULLS migration job support to ring ops Matthew Brost
2026-09-22 23:07 ` Lin, Shuicheng
2026-09-25 3:21 ` Matthew Brost
2026-09-23 8:47 ` Ghimiray, Himal Prasad
2026-09-04 21:16 ` [PATCH v6 21/24] drm/xe: Add ULLS migration job support to GuC submission Matthew Brost
2026-09-22 20:46 ` Lin, Shuicheng
2026-09-23 8:49 ` Ghimiray, Himal Prasad
2026-09-04 21:16 ` [PATCH v6 22/24] drm/xe: Enter ULLS for migration jobs upon page fault or SVM prefetch Matthew Brost
2026-09-22 16:39 ` Lin, Shuicheng
2026-09-23 8:52 ` Ghimiray, Himal Prasad
2026-09-04 21:16 ` [PATCH v6 23/24] drm/xe: Add modparam to enable / disable ULLS on migrate queue Matthew Brost
2026-09-09 8:03 ` Thomas Hellström
2026-09-09 18:11 ` Matthew Brost
2026-09-19 2:13 ` Lin, Shuicheng
2026-09-04 21:16 ` [PATCH v6 24/24] drm/xe: Document ULLS for migration jobs Matthew Brost
2026-09-09 9:01 ` Thomas Hellström
2026-09-09 17:53 ` Matthew Brost
2026-09-04 21:24 ` ✗ CI.checkpatch: warning for CPU binds and ULLS on migration queue (rev8) Patchwork
2026-09-04 21:26 ` ✓ CI.KUnit: success " Patchwork
2026-09-04 22:16 ` ✓ Xe.CI.BAT: " Patchwork
2026-09-05 3:34 ` ✗ 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=87f6232f-1652-41f7-9f38-ad14ee650db1@intel.com \
--to=himal.prasad.ghimiray@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox