Replace the open-coded user fence implementation in xe_sync.c with the new common drm_user_fence helper.
struct xe_user_fence now embeds struct drm_user_fence as its base. XE-specific fields (xe_device pointer for the ufence_wq wake-up, userspace VA, expected value, signalled flag) remain in the wrapper. The local user_fence_destroy/get/put/worker/kick_ufence/user_fence_cb functions are removed. Their logic moves to xe_ufence_ops.writeback and xe_ufence_ops.destroy, which are called by the drm_user_fence helper. Cc: Matthew Brost <[email protected]> Cc: Mika Kuoppala <[email protected]> Cc: Thomas Hellström <[email protected]> Cc: Maarten Lankhorst <[email protected]> Cc: Christian König <[email protected]> Cc: [email protected] Cc: [email protected] Signed-off-by: Srinivasan Shanmugam <[email protected]> --- v8: - Fix xe_ufence_worker ordering: WRITE_ONCE(signalled, 1) before copy_to_user(), add smp_wmb() before wake_up_all() (Matt, mirrors upstream fix 8ae04fe9ffc93) - Rename .worker → .writeback in xe_ufence_ops (Matt) - Update commit message: xe_ufence_ops.writeback (not .worker) drivers/gpu/drm/xe/xe_sync.c | 154 ++++++++++++++++------------- drivers/gpu/drm/xe/xe_sync.h | 2 + drivers/gpu/drm/xe/xe_sync_types.h | 1 - drivers/gpu/drm/xe/xe_vm.c | 1 + 4 files changed, 89 insertions(+), 69 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c index 37866768d64c..c0e720cf579a 100644 --- a/drivers/gpu/drm/xe/xe_sync.c +++ b/drivers/gpu/drm/xe/xe_sync.c @@ -6,12 +6,11 @@ #include "xe_sync.h" #include <linux/dma-fence-array.h> -#include <linux/kthread.h> -#include <linux/sched/mm.h> #include <linux/uaccess.h> #include <drm/drm_print.h> #include <drm/drm_syncobj.h> +#include <drm/drm_user_fence.h> #include <uapi/drm/xe_drm.h> #include "xe_device.h" @@ -19,36 +18,65 @@ #include "xe_macros.h" #include "xe_sched_job_types.h" +/* + * xe_user_fence wraps drm_user_fence with XE-specific fields. + * The drm_user_fence base handles MM borrowing and work-item lifetime. + */ struct xe_user_fence { - struct xe_device *xe; - struct kref refcount; - struct dma_fence_cb cb; - struct work_struct worker; - struct mm_struct *mm; - u64 __user *addr; - u64 value; - int signalled; + struct drm_user_fence base; + struct xe_device *xe; + u64 __user *addr; + u64 value; + int signalled; }; -static void user_fence_destroy(struct kref *kref) +static void xe_ufence_worker(struct drm_user_fence *base, bool mm_ok) { - struct xe_user_fence *ufence = container_of(kref, struct xe_user_fence, - refcount); + struct xe_user_fence *ufence = container_of(base, struct xe_user_fence, base); - mmdrop(ufence->mm); - kfree(ufence); -} + /* + * Mark signalled before the userspace write so that if userspace + * observes the written value and immediately issues a VM unmap, + * check_ufence() will see signalled=1 and not return -EBUSY. + * + * NOTE: copy_to_user() may block if userspace backs the address with + * userfaultfd or FUSE. Drivers requiring non-blocking semantics should + * use a dedicated workqueue instead of xe->ordered_wq. + */ + WRITE_ONCE(ufence->signalled, 1); -static void user_fence_get(struct xe_user_fence *ufence) -{ - kref_get(&ufence->refcount); + if (mm_ok) { + if (copy_to_user(ufence->addr, &ufence->value, + sizeof(ufence->value))) + drm_dbg(&ufence->xe->drm, + "copy_to_user failed, user fence wasn't signaled\n"); + } else { + drm_dbg(&ufence->xe->drm, + "mmget_not_zero() failed, ufence wasn't signaled\n"); + } + + /* + * Ensure both signalled=1 and the fence value written by copy_to_user() + * are visible to all CPUs before wake_up_all() wakes waiters. Without + * this barrier, weakly ordered architectures (e.g. ARM64) may allow + * waiters to observe the wakeup before seeing the updated values. + */ + smp_wmb(); + wake_up_all(&ufence->xe->ufence_wq); } -static void user_fence_put(struct xe_user_fence *ufence) +static void xe_ufence_destroy(struct drm_user_fence *base) { - kref_put(&ufence->refcount, user_fence_destroy); + struct xe_user_fence *ufence = container_of(base, struct xe_user_fence, base); + + kfree(ufence); } +static const struct drm_user_fence_ops xe_ufence_ops = { + .writeback = xe_ufence_worker, + .destroy = xe_ufence_destroy, +}; + static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr, u64 value) { @@ -63,51 +91,22 @@ static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr, if (!ufence) return ERR_PTR(-ENOMEM); - ufence->xe = xe; - kref_init(&ufence->refcount); - ufence->addr = ptr; + ufence->xe = xe; + ufence->addr = ptr; ufence->value = value; - ufence->mm = current->mm; - mmgrab(ufence->mm); + drm_user_fence_init(&ufence->base, xe->ordered_wq, &xe_ufence_ops); return ufence; } -static void user_fence_worker(struct work_struct *w) -{ - struct xe_user_fence *ufence = container_of(w, struct xe_user_fence, worker); - - WRITE_ONCE(ufence->signalled, 1); - if (mmget_not_zero(ufence->mm)) { - kthread_use_mm(ufence->mm); - if (copy_to_user(ufence->addr, &ufence->value, sizeof(ufence->value))) - XE_WARN_ON("Copy to user failed"); - kthread_unuse_mm(ufence->mm); - mmput(ufence->mm); - } else { - drm_dbg(&ufence->xe->drm, "mmget_not_zero() failed, ufence wasn't signaled\n"); - } - - /* - * Wake up waiters only after updating the ufence state, allowing the UMD - * to safely reuse the same ufence without encountering -EBUSY errors. - */ - wake_up_all(&ufence->xe->ufence_wq); - user_fence_put(ufence); -} - -static void kick_ufence(struct xe_user_fence *ufence, struct dma_fence *fence) +static void user_fence_get(struct xe_user_fence *ufence) { - INIT_WORK(&ufence->worker, user_fence_worker); - queue_work(ufence->xe->ordered_wq, &ufence->worker); - dma_fence_put(fence); + drm_user_fence_get(&ufence->base); } -static void user_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb) +static void user_fence_put(struct xe_user_fence *ufence) { - struct xe_user_fence *ufence = container_of(cb, struct xe_user_fence, cb); - - kick_ufence(ufence, fence); + drm_user_fence_put(&ufence->base); } int xe_sync_entry_parse(struct xe_device *xe, struct xe_file *xef, @@ -282,24 +281,15 @@ void xe_sync_entry_signal(struct xe_sync_entry *sync, struct dma_fence *fence) } else if (sync->syncobj) { drm_syncobj_replace_fence(sync->syncobj, fence); } else if (sync->ufence) { - int err; - drm_syncobj_add_point(sync->ufence_syncobj, sync->ufence_chain_fence, fence, sync->ufence_timeline_value); sync->ufence_chain_fence = NULL; fence = drm_syncobj_fence_get(sync->ufence_syncobj); - user_fence_get(sync->ufence); - err = dma_fence_add_callback(fence, &sync->ufence->cb, - user_fence_cb); - if (err == -ENOENT) { - kick_ufence(sync->ufence, fence); - } else if (err) { + if (drm_user_fence_add_callback(&sync->ufence->base, fence)) XE_WARN_ON("failed to add user fence"); - user_fence_put(sync->ufence); - dma_fence_put(fence); - } + /* fence ref consumed by drm_user_fence_add_callback */ } } @@ -434,6 +424,34 @@ void xe_sync_ufence_put(struct xe_user_fence *ufence) user_fence_put(ufence); } +/** + * xe_sync_ufence_cancel() - Non-blocking cancel of user fence callback + * @ufence: user fence reference + * + * Attempts to cancel the pending callback without waiting for the worker. + * Safe to call while holding dma_resv_lock or vm->lock. If the callback + * has already fired, the worker runs independently — xe_ufence_worker + * only accesses device-level and userspace resources, both safe after + * VMA teardown. + */ +void xe_sync_ufence_cancel(struct xe_user_fence *ufence) +{ + drm_user_fence_cancel(&ufence->base); +} + +/** + * xe_sync_ufence_cancel_sync() - Cancel user fence callback and wait for worker + * @ufence: user fence reference + * + * Cancels any pending dma-fence callback and waits for the worker to fully + * complete before returning. Must be called during teardown before freeing + * any resources accessed by the worker. + */ +void xe_sync_ufence_cancel_sync(struct xe_user_fence *ufence) +{ + drm_user_fence_cancel_sync(&ufence->base); +} + /** * xe_sync_ufence_get_status() - Get user fence status * @ufence: user fence diff --git a/drivers/gpu/drm/xe/xe_sync.h b/drivers/gpu/drm/xe/xe_sync.h index 6b949194acff..b5ff3387c592 100644 --- a/drivers/gpu/drm/xe/xe_sync.h +++ b/drivers/gpu/drm/xe/xe_sync.h @@ -44,6 +44,8 @@ static inline bool xe_sync_is_ufence(struct xe_sync_entry *sync) struct xe_user_fence *__xe_sync_ufence_get(struct xe_user_fence *ufence); struct xe_user_fence *xe_sync_ufence_get(struct xe_sync_entry *sync); void xe_sync_ufence_put(struct xe_user_fence *ufence); +void xe_sync_ufence_cancel(struct xe_user_fence *ufence); +void xe_sync_ufence_cancel_sync(struct xe_user_fence *ufence); int xe_sync_ufence_get_status(struct xe_user_fence *ufence); #endif diff --git a/drivers/gpu/drm/xe/xe_sync_types.h b/drivers/gpu/drm/xe/xe_sync_types.h index b88f1833e28c..67048199b070 100644 --- a/drivers/gpu/drm/xe/xe_sync_types.h +++ b/drivers/gpu/drm/xe/xe_sync_types.h @@ -12,7 +12,6 @@ struct drm_syncobj; struct dma_fence; struct dma_fence_chain; struct drm_xe_sync; -struct user_fence; struct xe_sync_entry { struct drm_syncobj *syncobj; diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c index b01f31ed4417..fbd2c28de8a6 100644 --- a/drivers/gpu/drm/xe/xe_vm.c +++ b/drivers/gpu/drm/xe/xe_vm.c @@ -1181,6 +1181,7 @@ static void xe_vma_destroy_late(struct xe_vma *vma) struct xe_bo *bo = xe_vma_bo(vma); if (vma->ufence) { + xe_sync_ufence_cancel(vma->ufence); xe_sync_ufence_put(vma->ufence); vma->ufence = NULL; } -- 2.34.1
