* [PATCH v9 1/3] drm: Add drm_work_fence helper
2026-09-09 4:44 [PATCH v9 0/3] drm: Extract dma-fence-to-workqueue pattern into common helpers Srinivasan Shanmugam
@ 2026-09-09 4:44 ` Srinivasan Shanmugam
2026-09-09 4:56 ` sashiko-bot
2026-09-09 4:44 ` [PATCH v9 2/3] drm: Add drm_user_fence helper Srinivasan Shanmugam
2026-09-09 4:44 ` [PATCH v9 3/3] drm/xe: Convert xe_user_fence to drm_user_fence Srinivasan Shanmugam
2 siblings, 1 reply; 7+ messages in thread
From: Srinivasan Shanmugam @ 2026-09-09 4:44 UTC (permalink / raw)
To: matthew.brost
Cc: dri-devel, intel-xe, amd-gfx, Srinivasan Shanmugam,
Maarten Lankhorst, Christian König
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->worker() 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>
---
v9:
- Fix drm_work_fence_queue: drop ref if queue_work returns false,
closing the race between callback firing and queue_work() (Matt)
- Fix drm_work_fence_cancel_sync: use disable_work_sync() instead of
cancel_work_sync() to close the UAF race window (Matt)
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)
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/drm_work_fence.c | 186 +++++++++++++++++++++++++++++++
include/drm/drm_work_fence.h | 69 ++++++++++++
3 files changed, 256 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..5e0cbc53edb8
--- /dev/null
+++ b/drivers/gpu/drm/drm_work_fence.c
@@ -0,0 +1,186 @@
+// 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)
+{
+ if (!queue_work(wfence->wq, &wfence->work))
+ drm_work_fence_put(wfence);
+}
+
+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 disable_work_sync() to guarantee
+ * the worker has fully completed before returning, closing the race
+ * window between the callback firing and queue_work() being called.
+ *
+ * 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 (disable_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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v9 1/3] drm: Add drm_work_fence helper
2026-09-09 4:44 ` [PATCH v9 1/3] drm: Add drm_work_fence helper Srinivasan Shanmugam
@ 2026-09-09 4:56 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-09-09 4:56 UTC (permalink / raw)
To: Srinivasan Shanmugam; +Cc: dri-devel, intel-xe
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm_work_fence_cancel_sync() fails to synchronize with the dma-fence callback, allowing queue_work() to be called concurrently with driver teardown.
- [High] The driver's ops->destroy() callback can unexpectedly be executed in hard IRQ context during the cancellation race.
--
commit 781c5ea671786cbee47b9bed0159aa892cc5e2b7
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
drm: Add drm_work_fence helper
This commit introduces drm_work_fence, a helper that queues a work item
when a dma-fence signals. It manages dma-fence callbacks, kref lifetime,
and workqueue dispatch.
> diff --git a/drivers/gpu/drm/drm_work_fence.c b/drivers/gpu/drm/drm_work_fence.c
> new file mode 100644
> index 0000000000000..5e0cbc53edb80
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_work_fence.c
[ ... ]
> +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);
[Severity: High]
Can this ops->destroy() callback be executed in hard IRQ context?
If a dma-fence callback races with driver teardown and queue_work() fails,
the final reference might be dropped directly from the IRQ callback:
drm_work_fence_cb() -> drm_work_fence_queue()
If queue_work() returns false, drm_work_fence_put() drops the last reference
here, triggering this destroy callback under the dma-fence spinlock with
IRQs disabled.
> + dma_fence_put(fence); /* NULL-safe */
> +}
[ ... ]
> +static void drm_work_fence_queue(struct drm_work_fence *wfence)
> +{
> + if (!queue_work(wfence->wq, &wfence->work))
> + drm_work_fence_put(wfence);
> +}
[Severity: High]
Is there a use-after-free risk if wfence->wq is accessed here during a
concurrent driver teardown?
This relies on drm_work_fence_cancel_sync() successfully synchronizing
against the callback. However, if the synchronization fails, wfence->wq
could be accessed after the workqueue has been destroyed by the caller.
[ ... ]
> +void drm_work_fence_cancel_sync(struct drm_work_fence *wfence)
> +{
> + if (drm_work_fence_cancel(wfence))
> + return;
> + if (disable_work_sync(&wfence->work))
> + drm_work_fence_put(wfence);
> +}
[Severity: High]
Does this provide the expected synchronous cancellation guarantee, or does
it leave a window for concurrent execution during teardown?
If the dma-fence signals, the callback is dequeued but hasn't called
queue_work() yet. If drm_work_fence_cancel_sync() is called concurrently:
1. drm_work_fence_cancel() calls dma_fence_remove_callback(), which
returns false because the callback is already dequeued.
2. disable_work_sync() returns false immediately because the work is
not yet queued.
3. The function returns, falsely indicating the work is fully canceled.
This allows the driver teardown to proceed while the in-flight callback
might then call queue_work() on a potentially destroyed workqueue.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909044454.399340-1-srinivasan.shanmugam@amd.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v9 2/3] drm: Add drm_user_fence helper
2026-09-09 4:44 [PATCH v9 0/3] drm: Extract dma-fence-to-workqueue pattern into common helpers Srinivasan Shanmugam
2026-09-09 4:44 ` [PATCH v9 1/3] drm: Add drm_work_fence helper Srinivasan Shanmugam
@ 2026-09-09 4:44 ` Srinivasan Shanmugam
2026-09-09 4:54 ` sashiko-bot
2026-09-09 4:44 ` [PATCH v9 3/3] drm/xe: Convert xe_user_fence to drm_user_fence Srinivasan Shanmugam
2 siblings, 1 reply; 7+ messages in thread
From: Srinivasan Shanmugam @ 2026-09-09 4:44 UTC (permalink / raw)
To: matthew.brost
Cc: dri-devel, intel-xe, amd-gfx, Srinivasan Shanmugam,
Christian König, Maarten Lankhorst
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>
---
v9:
- No code changes
v8:
- Fix copyright: The Linux Foundation → Advanced Micro Devices, Inc.
- Switch EXPORT_SYMBOL_GPL → EXPORT_SYMBOL for MIT-licensed code (Matt)
- Rename drm_user_fence_ops callback: worker → writeback (Matt)
- Add WARNING in @writeback kdoc about userfaultfd/FUSE blocking
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v9 2/3] drm: Add drm_user_fence helper
2026-09-09 4:44 ` [PATCH v9 2/3] drm: Add drm_user_fence helper Srinivasan Shanmugam
@ 2026-09-09 4:54 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-09-09 4:54 UTC (permalink / raw)
To: Srinivasan Shanmugam; +Cc: intel-xe, dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Build failure on NOMMU architectures due to unconditional use of mmput_async()
--
commit dc8e122093c744c8bd5d72581571fdb0ccb6856c
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
drm: Add drm_user_fence helper
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.
> 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 cause a build failure on architectures where CONFIG_MMU is disabled?
Although the comment notes that GPUs require an MMU, the DRM core and this
file are compiled unconditionally as part of CONFIG_DRM. Calling mmput_async()
here without an #ifdef CONFIG_MMU guard or similar protection could result in
an implicit function declaration or linker error on NOMMU builds.
> + }
> +}
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909044454.399340-1-srinivasan.shanmugam@amd.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v9 3/3] drm/xe: Convert xe_user_fence to drm_user_fence
2026-09-09 4:44 [PATCH v9 0/3] drm: Extract dma-fence-to-workqueue pattern into common helpers Srinivasan Shanmugam
2026-09-09 4:44 ` [PATCH v9 1/3] drm: Add drm_work_fence helper Srinivasan Shanmugam
2026-09-09 4:44 ` [PATCH v9 2/3] drm: Add drm_user_fence helper Srinivasan Shanmugam
@ 2026-09-09 4:44 ` Srinivasan Shanmugam
2026-09-09 4:57 ` sashiko-bot
2 siblings, 1 reply; 7+ messages in thread
From: Srinivasan Shanmugam @ 2026-09-09 4:44 UTC (permalink / raw)
To: matthew.brost
Cc: dri-devel, intel-xe, amd-gfx, Srinivasan Shanmugam, Mika Kuoppala,
Thomas Hellström, Maarten Lankhorst, Christian König
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>
---
v9:
- Remove xe_sync_ufence_cancel_sync — no caller in this patch (Matt)
- Remove xe_sync_ufence_cancel from xe_vma_destroy_late — unnecessary
behaviour change; xe_ufence_worker only accesses device-level
resources, safe to run after VMA teardown (Matt)
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)
drivers/gpu/drm/xe/xe_sync.c | 141 +++++++++++++++--------------
drivers/gpu/drm/xe/xe_sync.h | 1 +
drivers/gpu/drm/xe/xe_sync_types.h | 1 -
3 files changed, 74 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
index 37866768d64c..a335dd5f4478 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,21 @@ 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_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..a21f0e0501c6 100644
--- a/drivers/gpu/drm/xe/xe_sync.h
+++ b/drivers/gpu/drm/xe/xe_sync.h
@@ -44,6 +44,7 @@ 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);
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;
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v9 3/3] drm/xe: Convert xe_user_fence to drm_user_fence
2026-09-09 4:44 ` [PATCH v9 3/3] drm/xe: Convert xe_user_fence to drm_user_fence Srinivasan Shanmugam
@ 2026-09-09 4:57 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-09-09 4:57 UTC (permalink / raw)
To: Srinivasan Shanmugam; +Cc: intel-xe, dri-devel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Low] Unused function xe_sync_ufence_cancel() introduced.
Pre-existing issues:
- [High] User fence signaling can block the device-wide ordered workqueue, causing a cross-process denial of service.
- [High] Userspace can trigger kernel WARNINGs and potential DoS by concurrently resetting the syncobj fence.
--
commit c30f53c58eadaed9972496eb0819631ccfe8878e
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
drm/xe: Convert xe_user_fence to drm_user_fence
This commit replaces the open-coded user fence implementation in xe_sync.c
with the new common drm_user_fence helper. It moves the local user fence
logic into xe_ufence_ops.writeback and xe_ufence_ops.destroy callbacks.
> diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
> index 37866768d64c2..a335dd5f4478b 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(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)))
[Severity: High]
This isn't a bug introduced by this patch, but can this copy_to_user() block
the device-wide ordered workqueue?
As noted in the comment above it, if userspace backs the fence address with
userfaultfd or FUSE, the copy_to_user() call can be intentionally stalled
indefinitely. Since this worker runs on xe->ordered_wq which has max_active=1,
a stalled copy could block all other work items on this queue, resulting in a
denial of service for the device.
Should this use a dedicated non-blocking workqueue instead?
[ ... ]
> @@ -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))
[Severity: High]
This isn't a bug introduced by this patch, but can this sequence trigger a
kernel WARNING?
In xe_sync_entry_signal(), drm_syncobj_fence_get() retrieves a fence from the
syncobj. If userspace concurrently resets the syncobj using
DRM_IOCTL_SYNCOBJ_RESET, the retrieved fence could be NULL. Passing this
NULL fence to drm_user_fence_add_callback() would trigger a WARN_ON inside
the underlying dma_fence_add_callback() function.
Is there a way to safely handle the NULL fence case here?
> 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,21 @@ 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);
> +}
> +
[Severity: Low]
Is this function still needed?
The commit message mentions that callers for xe_sync_ufence_cancel() were
removed in v9, but the function definition itself remains here, resulting
in unused code.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909044454.399340-1-srinivasan.shanmugam@amd.com?part=3
^ permalink raw reply [flat|nested] 7+ messages in thread