[PATCH v8 0/3] drm: Extract dma-fence-to-workqueue pattern into common helpers

[PATCH v8 0/3] drm: Extract dma-fence-to-workqueue pattern into common helpers

[Srinivasan Shanmugam]

This series extracts the dma-fence-callback-to-workqueue pattern shared
between XE and AMDGPU into common DRM helpers.

Patch 1 introduces drm_work_fence — a generic helper that queues a work
item when a dma-fence signals, for work that cannot run in IRQ context.

Patch 2 introduces drm_user_fence — extends drm_work_fence with
kthread_use_mm() support for drivers that need to write completion
status to userspace memory.

Patch 3 converts XE's open-coded xe_user_fence to use drm_user_fence.

v8:
- Fix copyright: The Linux Foundation → Advanced Micro Devices, Inc.
- Switch EXPORT_SYMBOL_GPL → EXPORT_SYMBOL for MIT-licensed code (Matt)
- Rename drm_work_fence_ops callback: writeback → worker (Matt)
- Rename drm_user_fence_ops callback: worker → writeback (Matt)
- Fix xe_ufence_worker ordering: WRITE_ONCE before copy_to_user,
  add smp_wmb() before wake_up_all (Matt, confirmed via 8ae04fe9ffc93)
- Rename .worker → .writeback in xe_ufence_ops (Matt)
- Drop Patch 4 pending design decision on compare address read method

Suggested-by: Matthew Brost <matthew.brost@intel.com>
Suggested-by: Christian König <christian.koenig@amd.com>

Srinivasan Shanmugam (3):
  drm: Add drm_work_fence helper
  drm: Add drm_user_fence helper
  drm/xe: Convert xe_user_fence to drm_user_fence

 drivers/gpu/drm/Makefile           |   2 +
 drivers/gpu/drm/drm_user_fence.c   |  70 +++++++++++
 drivers/gpu/drm/drm_work_fence.c   | 184 +++++++++++++++++++++++++++++
 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 +
 include/drm/drm_user_fence.h       | 123 +++++++++++++++++++
 include/drm/drm_work_fence.h       |  69 +++++++++++
 9 files changed, 537 insertions(+), 69 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_user_fence.c
 create mode 100644 drivers/gpu/drm/drm_work_fence.c
 create mode 100644 include/drm/drm_user_fence.h
 create mode 100644 include/drm/drm_work_fence.h

-- 
2.34.1

[PATCH v8 1/3] drm: Add drm_work_fence helper

[Srinivasan Shanmugam]

GPU drivers often need to queue work when a dma-fence signals
because certain operations (copy_to_user, eventfd_signal, memory
allocation) cannot run in IRQ context. This pattern is currently
open-coded in multiple drivers.

Introduce drm_work_fence — an embeddable base structure that handles the
dma-fence-callback-to-workqueue pattern in one place. Drivers embed this
in their own structure and implement ops->writeback() for the deferred
work and ops->destroy() for cleanup.

The helper manages:
 - kref lifetime
 - dma-fence callback registration
 - workqueue dispatch on fence signal
 - safe cancellation before driver teardown

For work that additionally requires borrowing the process MM via
kthread_use_mm(), see drm_user_fence which builds on top of this.

Suggested-by: Matthew Brost <matthew.brost@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
v8:
- Fix copyright: The Linux Foundation → Advanced Micro Devices, Inc.
- Switch EXPORT_SYMBOL_GPL → EXPORT_SYMBOL (MIT license, Matt)
- Rename drm_work_fence_ops callback writeback → worker (Matt)
- Update all kernel-doc references accordingly

 drivers/gpu/drm/Makefile         |   1 +
 drivers/gpu/drm/drm_work_fence.c | 184 +++++++++++++++++++++++++++++++
 include/drm/drm_work_fence.h     |  69 ++++++++++++
 3 files changed, 254 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_work_fence.c
 create mode 100644 include/drm/drm_work_fence.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e97faabcd783..c5be8e80d0c8 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -72,6 +72,7 @@ drm-y := \
 	drm_vblank.o \
 	drm_vblank_work.o \
 	drm_vma_manager.o \
+	drm_work_fence.o \
 	drm_writeback.o
 drm-$(CONFIG_DRM_CLIENT) += \
 	drm_client.o \
diff --git a/drivers/gpu/drm/drm_work_fence.c b/drivers/gpu/drm/drm_work_fence.c
new file mode 100644
index 000000000000..ac66ba129fff
--- /dev/null
+++ b/drivers/gpu/drm/drm_work_fence.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2026 Advanced Micro Devices, Inc.
+ *
+ * Common DRM work fence helper.
+ *
+ * When a GPU dma-fence signals, drivers often need to perform work that
+ * cannot run in IRQ context (e.g., memory allocation, copy_to_user,
+ * eventfd_signal). This helper queues a work item when a dma-fence
+ * signals, allowing that work to run safely in a workqueue context.
+ *
+ * NOTE: This helper consumes dma_fences but CANNOT implement
+ * dma_fence_ops. Work items queued here may sleep; dma_fence_ops
+ * callbacks are called under the fence spinlock and must not sleep.
+ *
+ * For work that additionally requires accessing userspace memory via
+ * kthread_use_mm(), see drm_user_fence which builds on top of this.
+ */
+
+#include <linux/workqueue.h>
+
+#include <drm/drm_work_fence.h>
+
+static void drm_work_fence_destroy(struct kref *kref)
+{
+	struct drm_work_fence *wfence =
+		container_of(kref, struct drm_work_fence, refcount);
+	struct dma_fence *fence = wfence->fence;
+
+	wfence->ops->destroy(wfence);
+	dma_fence_put(fence);	/* NULL-safe */
+}
+
+/**
+ * drm_work_fence_get - Acquire a reference to a work fence
+ * @wfence: work fence
+ */
+void drm_work_fence_get(struct drm_work_fence *wfence)
+{
+	kref_get(&wfence->refcount);
+}
+EXPORT_SYMBOL(drm_work_fence_get);
+
+/**
+ * drm_work_fence_put - Release a reference to a work fence
+ * @wfence: work fence
+ */
+void drm_work_fence_put(struct drm_work_fence *wfence)
+{
+	kref_put(&wfence->refcount, drm_work_fence_destroy);
+}
+EXPORT_SYMBOL(drm_work_fence_put);
+
+static void drm_work_fence_work(struct work_struct *w)
+{
+	struct drm_work_fence *wfence =
+		container_of(w, struct drm_work_fence, work);
+
+	wfence->ops->worker(wfence);
+	drm_work_fence_put(wfence);
+}
+
+static void drm_work_fence_queue(struct drm_work_fence *wfence)
+{
+	queue_work(wfence->wq, &wfence->work);
+}
+
+static void drm_work_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
+{
+	struct drm_work_fence *wfence =
+		container_of(cb, struct drm_work_fence, cb);
+
+	drm_work_fence_queue(wfence);
+	/* Single ref: wfence->fence released in drm_work_fence_destroy(). */
+}
+
+/**
+ * drm_work_fence_init - Initialize a work fence
+ * @wfence: work fence to initialize
+ * @wq: workqueue to run the worker on (must be ordered if sequencing matters)
+ * @ops: driver operations
+ */
+void drm_work_fence_init(struct drm_work_fence *wfence,
+			 struct workqueue_struct *wq,
+			 const struct drm_work_fence_ops *ops)
+{
+	kref_init(&wfence->refcount);
+	wfence->wq = wq;
+	wfence->ops = ops;
+	wfence->fence = NULL;
+	INIT_WORK(&wfence->work, drm_work_fence_work);
+}
+EXPORT_SYMBOL(drm_work_fence_init);
+
+/**
+ * drm_work_fence_add_callback - Attach a work fence to a dma-fence
+ * @wfence: work fence; caller retains their reference and must release
+ *          it via drm_work_fence_put() when no longer needed
+ * @fence: dma-fence to watch; one reference is consumed on any return value
+ *
+ * When @fence signals, a work item is queued that calls ops->worker().
+ * If @fence has already signaled, the work item is queued immediately.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int drm_work_fence_add_callback(struct drm_work_fence *wfence,
+				struct dma_fence *fence)
+{
+	int err;
+
+	drm_work_fence_get(wfence);
+	wfence->fence = fence;	/* transfer caller's ref — single ref, no get */
+
+	err = dma_fence_add_callback(fence, &wfence->cb, drm_work_fence_cb);
+	if (err == -ENOENT) {
+		drm_work_fence_queue(wfence);
+		err = 0;
+	} else if (err) {
+		wfence->fence = NULL;
+		dma_fence_put(fence);
+		drm_work_fence_put(wfence);
+	}
+
+	return err;
+}
+EXPORT_SYMBOL(drm_work_fence_add_callback);
+
+/**
+ * drm_work_fence_cancel - Cancel a pending work fence callback
+ * @wfence: work fence
+ *
+ * Attempts to remove the pending callback before driver context teardown.
+ * The caller must hold a reference to @wfence across this call.
+ *
+ * If the callback has already fired this returns false and all cleanup
+ * has been handled internally.
+ *
+ * If removal succeeds the callback reference is released internally.
+ * The caller must still release its own reference via drm_work_fence_put().
+ *
+ * This function is safe to call from atomic context as it only acquires
+ * the dma-fence spinlock internally. If the caller also needs to wait
+ * for the worker to finish, use drm_work_fence_cancel_sync() instead,
+ * which may sleep.
+ *
+ * Return: true if callback was removed, false if it had already fired.
+ */
+bool drm_work_fence_cancel(struct drm_work_fence *wfence)
+{
+	struct dma_fence *fence = wfence->fence;
+
+	if (!fence)
+		return false;
+
+	if (dma_fence_remove_callback(fence, &wfence->cb)) {
+		drm_work_fence_put(wfence);	/* drop ref from add_callback */
+		return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(drm_work_fence_cancel);
+
+/**
+ * drm_work_fence_cancel_sync - Cancel callback and wait for worker to finish
+ * @wfence: work fence
+ *
+ * Calls drm_work_fence_cancel() then cancel_work_sync() to guarantee
+ * the worker has fully completed before returning.
+ *
+ * This function may sleep. Must not be called from atomic or interrupt
+ * context. Use drm_work_fence_cancel() instead when sleeping is not allowed.
+ *
+ * Drivers must call this during teardown before freeing any resources
+ * accessed by ops->worker().
+ */
+void drm_work_fence_cancel_sync(struct drm_work_fence *wfence)
+{
+	if (drm_work_fence_cancel(wfence))
+		return;
+	if (cancel_work_sync(&wfence->work))
+		drm_work_fence_put(wfence);
+}
+EXPORT_SYMBOL(drm_work_fence_cancel_sync);
diff --git a/include/drm/drm_work_fence.h b/include/drm/drm_work_fence.h
new file mode 100644
index 000000000000..7fa1e2561295
--- /dev/null
+++ b/include/drm/drm_work_fence.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2026 Advanced Micro Devices, Inc.
+ */
+
+#ifndef __DRM_WORK_FENCE_H__
+#define __DRM_WORK_FENCE_H__
+
+#include <linux/dma-fence.h>
+#include <linux/kref.h>
+#include <linux/workqueue.h>
+
+struct drm_work_fence;
+
+/**
+ * struct drm_work_fence_ops - driver callbacks for a DRM work fence
+ */
+struct drm_work_fence_ops {
+	/**
+	 * @worker: Called from workqueue context when the dma-fence signals.
+	 *
+	 * Perform the deferred work here (copy_to_user, eventfd_signal, etc.).
+	 * May sleep. Must not requeue the fence.
+	 */
+	void (*worker)(struct drm_work_fence *wfence);
+
+	/**
+	 * @destroy: Called when the last reference is dropped.
+	 * Free the containing structure here.
+	 */
+	void (*destroy)(struct drm_work_fence *wfence);
+};
+
+/**
+ * struct drm_work_fence - DRM dma-fence-to-workqueue helper
+ *
+ * Embeddable base structure that queues a work item when a dma-fence signals.
+ * Drivers embed this in their own structure and implement ops->worker()
+ * for the deferred work and ops->destroy() for cleanup.
+ *
+ * Call drm_work_fence_init() at creation and drm_work_fence_add_callback()
+ * to arm on a dma-fence. Call drm_work_fence_cancel_sync() before teardown.
+ */
+struct drm_work_fence {
+	/** @refcount: Reference count. */
+	struct kref refcount;
+	/** @wq: Workqueue on which to run the worker. */
+	struct workqueue_struct *wq;
+	/** @ops: Driver operations. */
+	const struct drm_work_fence_ops *ops;
+	/** @fence: The watched dma-fence; holds a single reference. */
+	struct dma_fence *fence;
+	/** @work: Work item queued when the fence signals. */
+	struct work_struct work;
+	/** @cb: Callback registered on the dma-fence. */
+	struct dma_fence_cb cb;
+};
+
+void drm_work_fence_init(struct drm_work_fence *wfence,
+			 struct workqueue_struct *wq,
+			 const struct drm_work_fence_ops *ops);
+void drm_work_fence_get(struct drm_work_fence *wfence);
+void drm_work_fence_put(struct drm_work_fence *wfence);
+int  drm_work_fence_add_callback(struct drm_work_fence *wfence,
+				 struct dma_fence *fence);
+bool drm_work_fence_cancel(struct drm_work_fence *wfence);
+void drm_work_fence_cancel_sync(struct drm_work_fence *wfence);
+
+#endif /* __DRM_WORK_FENCE_H__ */
-- 
2.34.1

Re: [PATCH v8 1/3] drm: Add drm_work_fence helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Race conditions in dma-fence signaling and callback addition allow deferred work to be queued after cancellation completes, leading to Use-After-Free.
--

commit 43b8d6b8cd3ff463894478af19fe890e4879d181
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Subject: drm: Add drm_work_fence helper

GPU drivers often need to queue work when a dma-fence signals
because certain operations (copy_to_user, eventfd_signal, memory
allocation) cannot run in IRQ context. This helper handles the
dma-fence-callback-to-workqueue pattern.

> diff --git a/drivers/gpu/drm/drm_work_fence.c b/drivers/gpu/drm/drm_work_fence.c
> index 0000000000000..ac66ba129ffff
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_work_fence.c
[ ... ]
> +static void drm_work_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
> +{
> +	struct drm_work_fence *wfence =
> +		container_of(cb, struct drm_work_fence, cb);
> +
> +	drm_work_fence_queue(wfence);
> +	/* Single ref: wfence->fence released in drm_work_fence_destroy(). */
> +}

[Severity: Critical]
Does this callback execution leave a race window?

The callback in drm_work_fence_cb() executes asynchronously. Between its
execution and the call to queue_work(), there appears to be no state indicating
to cancel_work_sync() that the work is active.

[ ... ]
> +int drm_work_fence_add_callback(struct drm_work_fence *wfence,
> +				struct dma_fence *fence)
> +{
> +	int err;
> +
> +	drm_work_fence_get(wfence);
> +	wfence->fence = fence;	/* transfer caller's ref — single ref, no get */
> +
> +	err = dma_fence_add_callback(fence, &wfence->cb, drm_work_fence_cb);
> +	if (err == -ENOENT) {
> +		drm_work_fence_queue(wfence);
> +		err = 0;

[Severity: Critical]
Does this -ENOENT fast path also contain the same race window?

If drm_work_fence_cancel_sync() is called by a teardown thread just before
drm_work_fence_queue() is executed here, it seems cancel_work_sync() could
fail to wait for the work to finish.

[ ... ]
> +void drm_work_fence_cancel_sync(struct drm_work_fence *wfence)
> +{
> +	if (drm_work_fence_cancel(wfence))
> +		return;
> +	if (cancel_work_sync(&wfence->work))
> +		drm_work_fence_put(wfence);
> +}

[Severity: Critical]
Could this logic lead to a use-after-free regression during teardown?

This function seems to assume that if the callback is not in the list, either
the work is already pending/running, or the work has finished. It appears to
ignore the window where the work is about to be queued but is not yet pending.

Can this sequence occur when an unprivileged userspace application concurrently
signals a fence and destroys the context?

CPU1 (signaling thread or -ENOENT path)
Removes callback from the list, then executes drm_work_fence_cb() but is
preempted before calling queue_work().

CPU2 (teardown thread) sees this in drm_work_fence_cancel_sync():
    if (drm_work_fence_cancel(wfence))
        // Returns false because callback is not in the list

    if (cancel_work_sync(&wfence->work))
        // Returns false immediately because work hasn't been queued yet

Function returns, allowing the driver to tear down resources.

CPU1 resumes:
Calls queue_work(), and the worker executes, accessing the freed driver
resources.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908084627.352535-1-srinivasan.shanmugam@amd.com?part=1

Re: [PATCH v8 1/3] drm: Add drm_work_fence helper

[Matthew Brost]

On Tue, Sep 08, 2026 at 08:58:39AM +0000, sashiko-bot@kernel.org wrote:

All of this looks right. So I believe the solution is:

- s/cancel_work_/disable_work_
- In drm_work_fence_queue if queue_work returns false, call drm_work_fence_put()

Matt

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] Race conditions in dma-fence signaling and callback addition allow deferred work to be queued after cancellation completes, leading to Use-After-Free.
> --
> 
> commit 43b8d6b8cd3ff463894478af19fe890e4879d181
> Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> Subject: drm: Add drm_work_fence helper
> 
> GPU drivers often need to queue work when a dma-fence signals
> because certain operations (copy_to_user, eventfd_signal, memory
> allocation) cannot run in IRQ context. This helper handles the
> dma-fence-callback-to-workqueue pattern.
> 
> > diff --git a/drivers/gpu/drm/drm_work_fence.c b/drivers/gpu/drm/drm_work_fence.c
> > index 0000000000000..ac66ba129ffff
> > --- /dev/null
> > +++ b/drivers/gpu/drm/drm_work_fence.c
> [ ... ]
> > +static void drm_work_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
> > +{
> > +	struct drm_work_fence *wfence =
> > +		container_of(cb, struct drm_work_fence, cb);
> > +
> > +	drm_work_fence_queue(wfence);
> > +	/* Single ref: wfence->fence released in drm_work_fence_destroy(). */
> > +}
> 
> [Severity: Critical]
> Does this callback execution leave a race window?
> 
> The callback in drm_work_fence_cb() executes asynchronously. Between its
> execution and the call to queue_work(), there appears to be no state indicating
> to cancel_work_sync() that the work is active.
> 
> [ ... ]
> > +int drm_work_fence_add_callback(struct drm_work_fence *wfence,
> > +				struct dma_fence *fence)
> > +{
> > +	int err;
> > +
> > +	drm_work_fence_get(wfence);
> > +	wfence->fence = fence;	/* transfer caller's ref — single ref, no get */
> > +
> > +	err = dma_fence_add_callback(fence, &wfence->cb, drm_work_fence_cb);
> > +	if (err == -ENOENT) {
> > +		drm_work_fence_queue(wfence);
> > +		err = 0;
> 
> [Severity: Critical]
> Does this -ENOENT fast path also contain the same race window?
> 
> If drm_work_fence_cancel_sync() is called by a teardown thread just before
> drm_work_fence_queue() is executed here, it seems cancel_work_sync() could
> fail to wait for the work to finish.
> 
> [ ... ]
> > +void drm_work_fence_cancel_sync(struct drm_work_fence *wfence)
> > +{
> > +	if (drm_work_fence_cancel(wfence))
> > +		return;
> > +	if (cancel_work_sync(&wfence->work))
> > +		drm_work_fence_put(wfence);
> > +}
> 
> [Severity: Critical]
> Could this logic lead to a use-after-free regression during teardown?
> 
> This function seems to assume that if the callback is not in the list, either
> the work is already pending/running, or the work has finished. It appears to
> ignore the window where the work is about to be queued but is not yet pending.
> 
> Can this sequence occur when an unprivileged userspace application concurrently
> signals a fence and destroys the context?
> 
> CPU1 (signaling thread or -ENOENT path)
> Removes callback from the list, then executes drm_work_fence_cb() but is
> preempted before calling queue_work().
> 
> CPU2 (teardown thread) sees this in drm_work_fence_cancel_sync():
>     if (drm_work_fence_cancel(wfence))
>         // Returns false because callback is not in the list
> 
>     if (cancel_work_sync(&wfence->work))
>         // Returns false immediately because work hasn't been queued yet
> 
> Function returns, allowing the driver to tear down resources.
> 
> CPU1 resumes:
> Calls queue_work(), and the worker executes, accessing the freed driver
> resources.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260908084627.352535-1-srinivasan.shanmugam@amd.com?part=1

Re: [PATCH v8 1/3] drm: Add drm_work_fence helper

[SRINIVASAN SHANMUGAM]

On 9/9/2026 1:46 AM, Matthew Brost wrote:
> On Tue, Sep 08, 2026 at 08:58:39AM +0000, sashiko-bot@kernel.org wrote:
>
> All of this looks right. So I believe the solution is:
>
> - s/cancel_work_/disable_work_
> - In drm_work_fence_queue if queue_work returns false, call drm_work_fence_put()
>
> Matt

Agreed — the race is real. Will fix in Patch 1

    as you suggested:
    - In drm_work_fence_queue: drop ref if queue_work returns false

    - In drm_work_fence_cancel_sync: switch to disable_work_sync()

Srini

Re: [PATCH v8 1/3] drm: Add drm_work_fence helper

[SRINIVASAN SHANMUGAM]

On 9/9/2026 8:30 AM, SRINIVASAN SHANMUGAM wrote:
>
> On 9/9/2026 1:46 AM, Matthew Brost wrote:
>> On Tue, Sep 08, 2026 at 08:58:39AM +0000, sashiko-bot@kernel.org wrote:
>>
>> All of this looks right. So I believe the solution is:
>>
>> - s/cancel_work_/disable_work_
>> - In drm_work_fence_queue if queue_work returns false, call 
>> drm_work_fence_put()
>>
>> Matt
>
> Agreed — the race is real. Will fix in Patch 1
>
>    as you suggested:
>    - In drm_work_fence_queue: drop ref if queue_work returns false
>
>    - In drm_work_fence_cancel_sync: switch to disable_work_sync()

Hi Matt,

Sashiko flagged two more issues on v9 Patch 1 [1][2]:

In v9 we applied suggested fix:
- drm_work_fence_queue: drop ref if queue_work() returns false
- drm_work_fence_cancel_sync: switched to disable_work_sync()

However, Sashiko found two remaining problems:

1. ops->destroy() called in IRQ context:

    When the GPU fence signals, the callback runs. If queue_work()
    returns false (work already done or disabled), we call
    drm_work_fence_put() immediately — but we are inside an interrupt
    with IRQs disabled. If that drops the last ref, ops->destroy()
    runs in that interrupt context, which is not safe for drivers that
    sleep in destroy.

    Fix: instead of calling drm_work_fence_put() directly, post a
    small cleanup task via schedule_work() on system_wq. This ensures
    destroy() always runs in process context where sleeping is allowed.

2. disable_work_sync() still has a timing gap:

    The callback does two things in sequence:
      a. Removes itself from the fence list
      b. Calls queue_work()

    If driver teardown starts between (a) and (b), disable_work_sync()
    sees no pending work and returns immediately — teardown proceeds.
    The callback then calls queue_work() on an already-destroyed
    workqueue, causing a use-after-free.

    Fix: add an atomic flag. The callback sets the flag ON before (b)
    and OFF after. cancel_sync() waits until the flag is OFF before
    returning. This closes the gap between (a) and (b).

Are these approaches acceptable, or may I kno pls if do you have a 
preferred solution?

Also for Patch 3 — xe_sync_ufence_cancel() is now unused after v9
removed its only caller. Will remove it in v10.

[1] v9 Patch 1:
https://patchwork.freedesktop.org/patch/751979/?series=173582&rev=2

[2] Sashiko v9 review:
https://sashiko.dev/#/patchset/20260909044454.399340-1-srinivasan.shanmugam%40amd.com

Thanks,
Srini

[PATCH v8 2/3] drm: Add drm_user_fence helper

[Srinivasan Shanmugam]

Introduce a common DRM user fence helper providing the kref-managed,
MM-borrowing dma-fence-callback-to-workqueue pattern used by drivers
that must access userspace memory from a kthread context when a GPU
fence signals.

XE uses this pattern (xe_sync.c) to write a fence completion value
to a userspace VA. AMDGPU will use the same pattern to signal a
per-queue eventfd from a user-queue EOP fence callback.

The helper provides:
 - struct drm_user_fence: embeddable base structure
 - struct drm_user_fence_ops: worker/destroy callbacks
 - drm_user_fence_init(): initialize and grab the process MM
 - drm_user_fence_get/put(): reference counting
 - drm_user_fence_add_callback(): attach to a dma-fence

The worker callback receives a bool indicating whether the process
MM was successfully obtained, allowing drivers to handle the
unavailable-MM case (log, skip the userspace write, etc.) without
duplicating the mmget/kthread_use_mm/mmput boilerplate.

Suggested-by: Christian König <christian.koenig@amd.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
v8:
- Fix copyright: The Linux Foundation → Advanced Micro Devices, Inc.
- Switch EXPORT_SYMBOL_GPL → EXPORT_SYMBOL (MIT license, Matt)
- Rename drm_user_fence_ops callback worker → writeback (Matt)
- Register as .worker in drm_work_fence_ops initializer
- Add WARNING in @writeback kdoc about userfaultfd/FUSE blocking
- Add mmput_async comment noting CONFIG_MMU requirement

 drivers/gpu/drm/Makefile         |   1 +
 drivers/gpu/drm/drm_user_fence.c |  70 ++++++++++++++++++
 include/drm/drm_user_fence.h     | 123 +++++++++++++++++++++++++++++++
 3 files changed, 194 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_user_fence.c
 create mode 100644 include/drm/drm_user_fence.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index c5be8e80d0c8..ddb770738992 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -69,6 +69,7 @@ drm-y := \
 	drm_syncobj.o \
 	drm_sysfs.o \
 	drm_trace_points.o \
+	drm_user_fence.o \
 	drm_vblank.o \
 	drm_vblank_work.o \
 	drm_vma_manager.o \
diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
new file mode 100644
index 000000000000..f7882208a8b7
--- /dev/null
+++ b/drivers/gpu/drm/drm_user_fence.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2026 Advanced Micro Devices, Inc.
+ *
+ * DRM user fence — extends drm_work_fence with kthread_use_mm() support.
+ *
+ * Use this when a GPU fence signals and work needs to access userspace
+ * memory (copy_to_user, fault-able operations) from a kthread context.
+ * For work that does not require userspace memory access, use
+ * drm_work_fence directly.
+ */
+
+#include <linux/kthread.h>
+#include <linux/sched/mm.h>
+
+#include <drm/drm_user_fence.h>
+
+static void drm_user_fence_do_destroy(struct drm_work_fence *wfence)
+{
+	struct drm_user_fence *ufence =
+		container_of(wfence, struct drm_user_fence, base);
+	struct mm_struct *mm = ufence->mm;
+
+	ufence->ops->destroy(ufence);
+	mmdrop(mm);
+}
+
+static void drm_user_fence_do_work(struct drm_work_fence *wfence)
+{
+	struct drm_user_fence *ufence =
+		container_of(wfence, struct drm_user_fence, base);
+	struct mm_struct *mm = NULL;
+
+	if (mmget_not_zero(ufence->mm)) {
+		mm = ufence->mm;
+		kthread_use_mm(mm);
+	}
+
+	ufence->ops->writeback(ufence, !!mm);
+
+	if (mm) {
+		kthread_unuse_mm(mm);
+		mmput_async(mm);	/* requires CONFIG_MMU — GPU requires MMU */
+	}
+}
+
+static const struct drm_work_fence_ops drm_user_fence_wfence_ops = {
+	.worker  = drm_user_fence_do_work,
+	.destroy = drm_user_fence_do_destroy,
+};
+
+/**
+ * drm_user_fence_init - Initialize a user fence
+ * @ufence: user fence to initialize
+ * @wq: workqueue on which to run the worker
+ * @ops: driver operations
+ *
+ * Must be called from process context with a valid current->mm.
+ * Grabs a reference to current->mm via mmgrab().
+ */
+void drm_user_fence_init(struct drm_user_fence *ufence,
+			 struct workqueue_struct *wq,
+			 const struct drm_user_fence_ops *ops)
+{
+	drm_work_fence_init(&ufence->base, wq, &drm_user_fence_wfence_ops);
+	ufence->mm = current->mm;
+	mmgrab(ufence->mm);
+	ufence->ops = ops;
+}
+EXPORT_SYMBOL(drm_user_fence_init);
diff --git a/include/drm/drm_user_fence.h b/include/drm/drm_user_fence.h
new file mode 100644
index 000000000000..db410c12b286
--- /dev/null
+++ b/include/drm/drm_user_fence.h
@@ -0,0 +1,123 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2026 Advanced Micro Devices, Inc.
+ */
+
+#ifndef __DRM_USER_FENCE_H__
+#define __DRM_USER_FENCE_H__
+
+#include <linux/dma-fence.h>
+
+#include <drm/drm_work_fence.h>
+
+struct drm_user_fence;
+
+/**
+ * struct drm_user_fence_ops - driver callbacks for a DRM user fence
+ */
+struct drm_user_fence_ops {
+	/**
+	 * @writeback: Called from workqueue context with the process MM active.
+	 *
+	 * If @mm_ok is true, kthread_use_mm() is active and userspace memory
+	 * (copy_to_user, etc.) may be accessed safely.
+	 * If @mm_ok is false, the process MM was already gone; skip the
+	 * userspace write.
+	 *
+	 * wake_up() or other post-signal housekeeping should also happen here.
+	 *
+	 * WARNING: Fault-able operations such as copy_to_user() may block
+	 * indefinitely if userspace registers the target address with
+	 * userfaultfd or backs it with a FUSE mount. Drivers that cannot
+	 * tolerate blocking should use copy_to_user_nofault() instead.
+	 */
+	void (*writeback)(struct drm_user_fence *ufence, bool mm_ok);
+
+	/**
+	 * @destroy: Called when the last reference is dropped.
+	 * Free the containing structure here.
+	 */
+	void (*destroy)(struct drm_user_fence *ufence);
+};
+
+/**
+ * struct drm_user_fence - DRM user fence with MM borrowing
+ *
+ * Extends drm_work_fence with kthread_use_mm() support for drivers
+ * that need to access userspace memory when a GPU fence signals.
+ *
+ * Call drm_user_fence_init() at creation and drm_user_fence_add_callback()
+ * to arm on a dma-fence. Call drm_user_fence_cancel_sync() before teardown.
+ */
+struct drm_user_fence {
+	/** @base: Base work fence. Must be first. */
+	struct drm_work_fence base;
+	/** @mm: Process MM grabbed at init time. */
+	struct mm_struct *mm;
+	/** @ops: Driver operations. */
+	const struct drm_user_fence_ops *ops;
+};
+
+void drm_user_fence_init(struct drm_user_fence *ufence,
+			 struct workqueue_struct *wq,
+			 const struct drm_user_fence_ops *ops);
+
+/**
+ * drm_user_fence_get - Acquire a reference to a user fence
+ * @ufence: user fence
+ */
+static inline void drm_user_fence_get(struct drm_user_fence *ufence)
+{
+	drm_work_fence_get(&ufence->base);
+}
+
+/**
+ * drm_user_fence_put - Release a reference to a user fence
+ * @ufence: user fence
+ */
+static inline void drm_user_fence_put(struct drm_user_fence *ufence)
+{
+	drm_work_fence_put(&ufence->base);
+}
+
+/**
+ * drm_user_fence_add_callback - Attach a user fence to a dma-fence
+ * @ufence: user fence; caller retains their reference and must release
+ *          it via drm_user_fence_put() when no longer needed
+ * @fence: dma-fence to watch; one reference is consumed on any return value
+ *
+ * When @fence signals, ops->writeback() is called from workqueue context.
+ * If @fence has already signaled, the worker is queued immediately.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+static inline int drm_user_fence_add_callback(struct drm_user_fence *ufence,
+					      struct dma_fence *fence)
+{
+	return drm_work_fence_add_callback(&ufence->base, fence);
+}
+
+/**
+ * drm_user_fence_cancel - Cancel a pending user fence callback
+ * @ufence: user fence
+ *
+ * Return: true if callback was removed, false if it had already fired.
+ */
+static inline bool drm_user_fence_cancel(struct drm_user_fence *ufence)
+{
+	return drm_work_fence_cancel(&ufence->base);
+}
+
+/**
+ * drm_user_fence_cancel_sync - Cancel callback and wait for worker to finish
+ * @ufence: user fence
+ *
+ * Must be called during teardown before freeing any resources accessed
+ * by ops->writeback(). May sleep.
+ */
+static inline void drm_user_fence_cancel_sync(struct drm_user_fence *ufence)
+{
+	drm_work_fence_cancel_sync(&ufence->base);
+}
+
+#endif /* __DRM_USER_FENCE_H__ */
-- 
2.34.1

Re: [PATCH v8 2/3] drm: Add drm_user_fence helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Unconditional use of mmput_async() breaks compilation and linking on NOMMU configurations.
--

commit 890c55fa04660dfbfe7364ac5c78b52162fca15b
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>

drm: Add drm_user_fence helper

This commit introduces a common DRM user fence helper providing a
kref-managed, MM-borrowing dma-fence-callback-to-workqueue pattern.
It is used by drivers that must access userspace memory from a kthread
context when a GPU fence signals.

> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 8895ca0476d89..3557e6a8d31c9 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -69,6 +69,7 @@ drm-y := \
>  	drm_syncobj.o \
>  	drm_sysfs.o \
>  	drm_trace_points.o \
> +	drm_user_fence.o \
>  	drm_vblank.o \

[ ... ]

> diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
> new file mode 100644
> index 0000000000000..f7882208a8b73
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_user_fence.c

[ ... ]

> +static void drm_user_fence_do_work(struct drm_work_fence *wfence)
> +{
> +	struct drm_user_fence *ufence =
> +		container_of(wfence, struct drm_user_fence, base);
> +	struct mm_struct *mm = NULL;
> +
> +	if (mmget_not_zero(ufence->mm)) {
> +		mm = ufence->mm;
> +		kthread_use_mm(mm);
> +	}
> +
> +	ufence->ops->writeback(ufence, !!mm);
> +
> +	if (mm) {
> +		kthread_unuse_mm(mm);
> +		mmput_async(mm);	/* requires CONFIG_MMU — GPU requires MMU */

[Severity: Low]
Does this unconditional call to mmput_async() break the build on systems
without CONFIG_MMU?

While the comment notes that the GPU requires an MMU, the drm_user_fence.o
file is added unconditionally to the core DRM build in
drivers/gpu/drm/Makefile. If the kernel is built with CONFIG_DRM enabled
but CONFIG_MMU disabled, this will result in an implicit function
declaration and an undefined reference to mmput_async() during linking.

> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908084627.352535-1-srinivasan.shanmugam@amd.com?part=2

[PATCH v8 3/3] drm/xe: Convert xe_user_fence to drm_user_fence

[Srinivasan Shanmugam]

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 <matthew.brost@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
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

Re: [PATCH v8 3/3] drm/xe: Convert xe_user_fence to drm_user_fence

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Missing memory barrier allows visibility inversion between the user fence value and the signalled flag.

Pre-existing issues:
- [High] Denial of Service (DoS) due to indefinitely blocking `copy_to_user` stalling the device-wide ordered workqueue.
--

commit a3a566db4faca9c6c8375b3c0711ddb1c2742bca
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Subject: drm/xe: Convert xe_user_fence to drm_user_fence

This commit replaces the open-coded user fence implementation in the xe driver
with the new common drm_user_fence helper. It wraps the new helper in
xe_user_fence and moves the worker and callback logic to the provided
writeback and destroy ops.

> diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
> index 37866768d64c2..c0e720cf579ac 100644
> --- a/drivers/gpu/drm/xe/xe_sync.c
> +++ b/drivers/gpu/drm/xe/xe_sync.c
[ ... ]
> +static void xe_ufence_worker(struct drm_user_fence *base, bool mm_ok)
>  {
> +	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)))

[Severity: High]
Does this missing memory barrier allow visibility inversion?

In xe_ufence_worker(), WRITE_ONCE(ufence->signalled, 1) is called before
copy_to_user(). If a weakly ordered CPU (like ARM64) reorders these stores,
could userspace observe the new value in memory and issue a VM unmap ioctl
before the signalled flag is globally visible?

The previous implementation relied on the implicit full memory barrier of
mmget_not_zero() between these two operations, but this refactoring moved
that check before the worker executes. Should there be an smp_wmb() between
the WRITE_ONCE() and copy_to_user() calls to prevent spurious -EBUSY
failures in the VM unmap path?

> +			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");
> +	}
[ ... ]
> -	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);

[Severity: High]
This is a pre-existing issue, but can this indefinitely blocking userspace
memory access stall the device-wide ordered workqueue?

If an unprivileged userspace application maps a memory region with
userfaultfd, submits a user fence operation pointing to this region, and
refuses to resolve the fault, copy_to_user() will block indefinitely.

Since the worker is initialized with xe->ordered_wq, this could permanently
stall the single-threaded workqueue used for critical device-wide operations
like GT resets, TDRs, and compute mode preempt rebinds, leading to a denial
of service.

>  
>  	return ufence;
>  }
>  
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908084627.352535-1-srinivasan.shanmugam@amd.com?part=3

Re: [PATCH v8 3/3] drm/xe: Convert xe_user_fence to drm_user_fence

[Matthew Brost]

On Tue, Sep 08, 2026 at 02:16:27PM +0530, Srinivasan Shanmugam wrote:
> 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 <matthew.brost@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: intel-xe@lists.freedesktop.org
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
> 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);
> +}

This is unused in this patch.

> +
>  /**
>   * 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);

This is a behavior change in this patch. Why is this change?

Also can't this prevent a user fence signaling on unbind if
xe_vma_destroy_late() executes before the user fence? I'm somewhat
suprised our CI didn't fail because of this change or Sashiko didn't
complain about this, so maybe I'm missing something.

Matt

>  		xe_sync_ufence_put(vma->ufence);
>  		vma->ufence = NULL;
>  	}
> -- 
> 2.34.1
> 

Re: [PATCH v8 3/3] drm/xe: Convert xe_user_fence to drm_user_fence

[SRINIVASAN SHANMUGAM]

On 9/9/2026 1:52 AM, Matthew Brost wrote:
> On Tue, Sep 08, 2026 at 02:16:27PM +0530, Srinivasan Shanmugam wrote:
>> 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 <matthew.brost@intel.com>
>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: intel-xe@lists.freedesktop.org
>> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
>> ---
>> 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);
>> +}
> This is unused in this patch.

Agreed — it has no caller in this patch. Will remove in v9.


>
>> +
>>   /**
>>    * 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);
> This is a behavior change in this patch. Why is this change?

The cancel was added defensively to prevent the user fence worker from
running after VMA teardown, since the concern was that the callback
could fire during or after xe_vma_destroy_late() and access freed
resources. However, you are correct that this is unnecessary —
xe_ufence_worker only accesses device-level resources (xe->ufence_wq)
and the userspace address via copy_to_user(), and does not touch any
VMA-specific state. So the worker is safe to run after VMA teardown,
exactly as the original code allowed. Will remove the cancel call in
v9, keeping only xe_sync_ufence_put().

Srini