[PATCH v3 0/2] drm: Add Common drm_user_fence helper and Convert XE

[PATCH v3 0/2] drm: Add Common drm_user_fence helper and Convert XE

[Srinivasan Shanmugam]

When a GPU dma-fence signals, drivers often need to access userspace
memory from a kthread context — either to write a fence completion
value to a userspace VA (XE) or to signal a per-queue eventfd (AMDGPU).
Both require borrowing the process MM via kthread_use_mm(), and both
use the same kref-managed dma-fence-callback-to-workqueue pattern.

Extracting this pattern into a shared DRM helper allows both XE and
AMDGPU to use it instead of maintaining independent open-coded
implementations.

This series does that:

Patch 1 introduces drm_user_fence — an embeddable base structure with
driver-supplied worker/destroy callbacks. The common code handles:
 - process MM grab at init (mmgrab)
 - dma-fence callback registration
 - workqueue dispatch on fence signal
 - mmget_not_zero/kthread_use_mm/mmput in the worker
 - kref lifetime management
 - cancellation via drm_user_fence_cancel_sync() for safe driver teardown

Patch 2 converts XE to use the new helper. struct xe_user_fence embeds
struct drm_user_fence as its base. XE-specific fields (xe_device pointer
for ufence_wq wake-up, userspace VA, expected value, signalled flag)
remain in the wrapper. No behavioral change is intended.
A follow-on patch (not in this series) will wire AMDGPU's render-node
EOP eventfd signaling path to the same helper.

v3:
 - Store fence reference inside drm_user_fence during add_callback so
   drm_user_fence_cancel() can be called safely without the caller
   holding a separate fence reference, fixing a potential UAF when a
   foreign dma-fence signals after driver teardown. (Sashiko review)
 - Fix contradictory kernel-doc for drm_user_fence_cancel() which
   incorrectly instructed callers to put references that the function
   already puts internally. (Sashiko review)
 - Add drm_user_fence_cancel_sync() to guarantee the worker has fully
   completed before returning. Wire into xe_sync_entry_cleanup() and
   xe_vma_destroy_late() for safe XE teardown. (Sashiko review)
 - Add xe_sync_ufence_cancel_sync() wrapper in xe_sync.c so xe_vm.c
   does not need to include drm_user_fence.h or access struct
   xe_user_fence internals directly.
 - Add smp_wmb() in xe_ufence_worker() between WRITE_ONCE(signalled)
   and copy_to_user() to prevent store reordering on weakly-ordered
   architectures such as ARM64. (Sashiko review)
 - Replace XE_WARN_ON() on copy_to_user() failure with drm_dbg() to
   prevent unprivileged userspace from triggering a kernel warning by
   unmapping the user fence memory before it signals. (Sashiko review)
v2:
 - Move INIT_WORK() to drm_user_fence_init() fixing lockdep class
   divergence caused by two different INIT_WORK() call sites.
   (Sashiko review)
 - drm_user_fence_add_callback() now consumes the fence reference in
   all paths: transferred to the callback on success, put immediately
   on the already-signaled and error paths. (Sashiko review)
 - Add drm_user_fence_cancel() to allow drivers to safely detach a
   pending callback before context teardown. (Sashiko review)
 - Fix premature dma_fence_put() after drm_user_fence_add_callback()
   in xe_sync_entry_signal(). (Sashiko review)
 - Fix missing newline at end of new files.

Suggested-by: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
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: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
Cc: amd-gfx@lists.freedesktop.org

Srinivasan Shanmugam (2):
  drm: Add common drm_user_fence helper
  drm/xe: Convert xe_user_fence to drm_user_fence

 drivers/gpu/drm/Makefile           |   1 +
 drivers/gpu/drm/drm_user_fence.c   | 210 +++++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_sync.c       | 140 +++++++++----------
 drivers/gpu/drm/xe/xe_sync.h       |   1 +
 drivers/gpu/drm/xe/xe_sync_types.h |   3 +-
 drivers/gpu/drm/xe/xe_vm.c         |   1 +
 include/drm/drm_user_fence.h       |  76 +++++++++++
 7 files changed, 360 insertions(+), 72 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_user_fence.c
 create mode 100644 include/drm/drm_user_fence.h

-- 
2.34.1

[PATCH v3 1/2] drm: Add common 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: Alex Deucher <alexander.deucher@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
v3:
 - Store fence reference inside drm_user_fence during add_callback so
   drm_user_fence_cancel() can be called safely without the caller
   holding a separate fence reference, fixing a potential UAF when a
   foreign dma-fence signals after driver teardown. (Sashiko review)
 - Fix contradictory kernel-doc for drm_user_fence_cancel() which
   incorrectly instructed callers to put references that the function
   already puts internally. (Sashiko review)
 - Add drm_user_fence_cancel_sync() to guarantee the worker has fully
   completed before returning. (Sashiko review)

v2:
 - Move INIT_WORK() to drm_user_fence_init() fixing lockdep class
   divergence. (Sashiko review)
 - drm_user_fence_add_callback() now consumes the fence reference in
   all paths. (Sashiko review)
 - Add drm_user_fence_cancel() for safe driver teardown. (Sashiko review)
 - Fix missing newline at end of new files.
 
 drivers/gpu/drm/Makefile         |   1 +
 drivers/gpu/drm/drm_user_fence.c | 210 +++++++++++++++++++++++++++++++
 include/drm/drm_user_fence.h     |  76 +++++++++++
 3 files changed, 287 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 e97faabcd783..52de1f474535 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..83920d2be43b
--- /dev/null
+++ b/drivers/gpu/drm/drm_user_fence.c
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 The Linux Foundation
+ *
+ * Common DRM user fence helper.
+ *
+ * When a GPU dma-fence signals, drivers often need to write a value to a
+ * userspace VA or notify userspace via an eventfd. Both operations require
+ * a valid process MM, which is not available in IRQ context.
+ *
+ * This helper queues a work item on fence signal. The work item borrows the
+ * process MM via kthread_use_mm() and calls ops->worker(), which the driver
+ * implements to perform the actual userspace access.
+ */
+
+#include <linux/kthread.h>
+#include <linux/sched/mm.h>
+#include <linux/workqueue.h>
+
+#include <drm/drm_user_fence.h>
+
+static void drm_user_fence_destroy(struct kref *kref)
+{
+	struct drm_user_fence *ufence =
+		container_of(kref, struct drm_user_fence, refcount);
+
+	/* Release the extra reference stored for cancel() */
+	if (ufence->fence)
+		dma_fence_put(ufence->fence);
+
+	mmdrop(ufence->mm);
+	ufence->ops->destroy(ufence);
+}
+
+/**
+ * drm_user_fence_get - Acquire a reference to a user fence
+ * @ufence: user fence
+ */
+void drm_user_fence_get(struct drm_user_fence *ufence)
+{
+	kref_get(&ufence->refcount);
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_get);
+
+/**
+ * drm_user_fence_put - Release a reference to a user fence
+ * @ufence: user fence
+ */
+void drm_user_fence_put(struct drm_user_fence *ufence)
+{
+	kref_put(&ufence->refcount, drm_user_fence_destroy);
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_put);
+
+static void drm_user_fence_work(struct work_struct *w)
+{
+	struct drm_user_fence *ufence =
+		container_of(w, struct drm_user_fence, work);
+	bool mm_ok = false;
+
+	if (mmget_not_zero(ufence->mm)) {
+		kthread_use_mm(ufence->mm);
+		mm_ok = true;
+	}
+
+	ufence->ops->worker(ufence, mm_ok);
+
+	if (mm_ok) {
+		kthread_unuse_mm(ufence->mm);
+		mmput(ufence->mm);
+	}
+
+	drm_user_fence_put(ufence);
+}
+
+static void drm_user_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
+{
+	struct drm_user_fence *ufence =
+		container_of(cb, struct drm_user_fence, cb);
+
+	queue_work(ufence->wq, &ufence->work);
+	/*
+	 * Put the transferred reference from add_callback. The stored
+	 * reference in ufence->fence is released in drm_user_fence_destroy().
+	 */
+	dma_fence_put(fence);
+}
+
+/**
+ * drm_user_fence_init - Initialize a user fence
+ * @ufence: user fence to initialize
+ * @wq: workqueue to run the worker on (must be ordered if sequencing matters)
+ * @ops: driver operations
+ *
+ * Must be called from process context. Grabs a reference to current->mm.
+ */
+void drm_user_fence_init(struct drm_user_fence *ufence,
+			 struct workqueue_struct *wq,
+			 const struct drm_user_fence_ops *ops)
+{
+	kref_init(&ufence->refcount);
+	ufence->mm = current->mm;
+	mmgrab(ufence->mm);
+	ufence->wq = wq;
+	ufence->ops = ops;
+	ufence->fence = NULL;
+	INIT_WORK(&ufence->work, drm_user_fence_work);
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_init);
+
+/**
+ * drm_user_fence_add_callback - Attach a user fence to a dma-fence
+ * @ufence: user fence
+ * @fence: dma-fence to watch; ownership of this reference is transferred
+ *         to the callback — caller must NOT put it afterward.
+ *
+ * When @fence signals, a work item is queued that calls ops->worker() with
+ * the process MM active. If @fence has already signaled the work item is
+ * queued immediately.
+ *
+ * An additional reference to @fence is stored internally in @ufence to
+ * allow drm_user_fence_cancel() to be called safely without the caller
+ * needing to hold a separate fence reference.
+ *
+ * On any return value the caller's fence reference is consumed.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int drm_user_fence_add_callback(struct drm_user_fence *ufence,
+				struct dma_fence *fence)
+{
+	int err;
+
+	drm_user_fence_get(ufence);
+
+	/* Extra ref stored for cancel() — lives until drm_user_fence_destroy() */
+	ufence->fence = dma_fence_get(fence);
+
+	err = dma_fence_add_callback(fence, &ufence->cb, drm_user_fence_cb);
+	if (err == -ENOENT) {
+		/* fence already signaled — queue work and release transferred ref */
+		queue_work(ufence->wq, &ufence->work);
+		dma_fence_put(fence);
+		err = 0;
+	} else if (err) {
+		dma_fence_put(ufence->fence);
+		ufence->fence = NULL;
+		drm_user_fence_put(ufence);
+		dma_fence_put(fence);
+	}
+	/* on success: transferred ref goes to drm_user_fence_cb */
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_add_callback);
+
+/**
+ * drm_user_fence_cancel - Cancel a pending user fence callback
+ * @ufence: user fence
+ *
+ * Attempts to remove the pending callback before driver context teardown.
+ * Must be called before the driver tears down its workqueue or ops.
+ * The caller must hold a reference to @ufence across this call.
+ *
+ * If the callback has already fired this returns false and all cleanup
+ * has already been handled internally — the caller needs no further action.
+ *
+ * If removal succeeds all references are released internally — the caller
+ * must NOT call drm_user_fence_put() or dma_fence_put() afterward.
+ *
+ * Return: true if callback was removed, false if it had already fired.
+ */
+bool drm_user_fence_cancel(struct drm_user_fence *ufence)
+{
+	struct dma_fence *fence = ufence->fence;
+
+	if (!fence)
+		return false;
+
+	if (dma_fence_remove_callback(fence, &ufence->cb)) {
+		/*
+		 * Callback will not fire — release the transferred reference
+		 * that would have been put by drm_user_fence_cb(). The stored
+		 * reference in ufence->fence is released in destroy().
+		 */
+		dma_fence_put(fence);
+		drm_user_fence_put(ufence);
+		return true;
+	}
+
+	/* Callback already fired — it handled its own cleanup */
+	return false;
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_cancel);
+
+/**
+ * drm_user_fence_cancel_sync - Cancel callback and wait for worker to finish
+ * @ufence: user fence
+ *
+ * Calls drm_user_fence_cancel() then cancel_work_sync() to guarantee
+ * the worker has fully completed before returning. Drivers must call
+ * this during teardown before freeing any resources accessed by
+ * ops->worker().
+ */
+void drm_user_fence_cancel_sync(struct drm_user_fence *ufence)
+{
+	drm_user_fence_cancel(ufence);
+	cancel_work_sync(&ufence->work);
+}
+EXPORT_SYMBOL_GPL(drm_user_fence_cancel_sync);
diff --git a/include/drm/drm_user_fence.h b/include/drm/drm_user_fence.h
new file mode 100644
index 000000000000..02a02266ab93
--- /dev/null
+++ b/include/drm/drm_user_fence.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 The Linux Foundation
+ */
+
+#ifndef __DRM_USER_FENCE_H__
+#define __DRM_USER_FENCE_H__
+
+#include <linux/dma-fence.h>
+#include <linux/kref.h>
+#include <linux/workqueue.h>
+
+struct drm_user_fence;
+
+/**
+ * struct drm_user_fence_ops - driver callbacks for a DRM user fence
+ */
+struct drm_user_fence_ops {
+	/**
+	 * @worker: Called from workqueue context.
+	 *
+	 * If @mm_ok is true, kthread_use_mm() is active and userspace memory
+	 * (copy_to_user, eventfd_signal, etc.) may be accessed safely.
+	 * If @mm_ok is false, the process MM was already gone; the driver
+	 * should log a warning and skip the userspace write.
+	 *
+	 * wake_up() or other post-signal housekeeping should also happen here.
+	 */
+	void (*worker)(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 - embeddable DRM user fence
+ *
+ * Drivers embed this in their own structure and implement
+ * &drm_user_fence_ops. 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 driver teardown.
+ */
+struct drm_user_fence {
+	/** @refcount: Reference count. */
+	struct kref refcount;
+	/** @mm: Process MM grabbed at init time. */
+	struct mm_struct *mm;
+	/** @work: Work item queued when the dma-fence signals. */
+	struct work_struct work;
+	/** @cb: dma-fence callback. */
+	struct dma_fence_cb cb;
+	/**
+	 * @fence: Extra reference held for safe cancel(). Set during
+	 * add_callback, released in destroy().
+	 */
+	struct dma_fence *fence;
+	/** @wq: Workqueue to run @work on. */
+	struct workqueue_struct *wq;
+	/** @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);
+void drm_user_fence_get(struct drm_user_fence *ufence);
+void drm_user_fence_put(struct drm_user_fence *ufence);
+int drm_user_fence_add_callback(struct drm_user_fence *ufence,
+				struct dma_fence *fence);
+bool drm_user_fence_cancel(struct drm_user_fence *ufence);
+void drm_user_fence_cancel_sync(struct drm_user_fence *ufence);
+
+#endif /* __DRM_USER_FENCE_H__ */
-- 
2.34.1

Re: [PATCH v3 1/2] drm: Add common drm_user_fence helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Memory leak of `drm_user_fence`, `mm_struct`, and `dma_fence` when a pending work item is successfully cancelled.
--

commit 922879f241f70a69a977d292a4790f93e66ca3e2
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>

drm: Add common drm_user_fence helper

This commit introduces a common DRM user fence helper that provides a
kref-managed, MM-borrowing dma-fence-callback-to-workqueue pattern. This
allows drivers to 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..5e6d4c1071306
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_user_fence.c
[ ... ]
> +void drm_user_fence_cancel_sync(struct drm_user_fence *ufence)
> +{
> +	drm_user_fence_cancel(ufence);
> +	cancel_work_sync(&ufence->work);

[Severity: High]
Can ignoring the return value of cancel_work_sync() cause a memory leak
of the drm_user_fence, mm_struct, and an extra dma_fence reference?

If a user arms a user fence and the hardware signals it, the signal
callback drm_user_fence_cb() queues the work item. If the user then tears
down the object before the work item executes, drm_user_fence_cancel_sync()
is called.

If cancel_work_sync() successfully dequeues the pending work, it returns
true, and drm_user_fence_work() will never run. This means the call to
drm_user_fence_put() at the end of drm_user_fence_work() is skipped:

static void drm_user_fence_work(struct work_struct *w)
{
    ...
    drm_user_fence_put(ufence);
}

Leaking the mm_struct pins memory, and leaking the dma_fence can prevent GPU
resource cleanup.

Would it be safer to check the return value and drop the reference?

	if (cancel_work_sync(&ufence->work))
		drm_user_fence_put(ufence);

> +}

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

[PATCH v3 2/2] 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.worker and
xe_ufence_ops.destroy, which are called by drm_user_fence_work().

Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: intel-xe@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
v3:
 - Add xe_sync_ufence_cancel_sync() wrapper so xe_vm.c does not need
   to include drm_user_fence.h or access struct xe_user_fence internals.
 - Call xe_sync_ufence_cancel_sync() in xe_vma_destroy_late() before
   dropping the VMA's ufence reference. (Sashiko review)
 - Call drm_user_fence_cancel_sync() in xe_sync_entry_cleanup() to
   ensure the worker has completed before teardown. (Sashiko review)
 - Add smp_wmb() between WRITE_ONCE(signalled) and copy_to_user() to
   prevent store reordering on ARM64. (Sashiko review)
 - Replace XE_WARN_ON() on copy_to_user() failure with drm_dbg() to
   prevent unprivileged DoS via panic_on_warn. (Sashiko review)

v2:
 - Remove unconditional dma_fence_put(fence) after
   drm_user_fence_add_callback() in xe_sync_entry_signal(). The fence
   reference is now consumed by drm_user_fence_add_callback() in all
   paths. (Sashiko review)

 drivers/gpu/drm/xe/xe_sync.c       | 140 ++++++++++++++---------------
 drivers/gpu/drm/xe/xe_sync.h       |   1 +
 drivers/gpu/drm/xe/xe_sync_types.h |   3 +-
 drivers/gpu/drm/xe/xe_vm.c         |   1 +
 4 files changed, 73 insertions(+), 72 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
index 37866768d64c..7da61ad01c36 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,60 @@
 #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 waking waiters so UMD can safely reuse
+	 * the same ufence without hitting -EBUSY.
+	 */
+	WRITE_ONCE(ufence->signalled, 1);
 
-static void user_fence_get(struct xe_user_fence *ufence)
-{
-	kref_get(&ufence->refcount);
+	/*
+	 * Ensure the signalled store is visible before the user memory write
+	 * on weakly ordered architectures (e.g. ARM64). Without this barrier
+	 * the CPU may reorder stores, causing userspace to observe the user
+	 * memory update before signalled == 1.
+	 */
+	smp_wmb();
+
+	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");
+	}
+
+	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 = {
+	.worker  = 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 +86,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 +276,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 */
 	}
 }
 
@@ -310,8 +295,10 @@ void xe_sync_entry_cleanup(struct xe_sync_entry *sync)
 	dma_fence_put(sync->fence);
 	dma_fence_chain_free(sync->chain_fence);
 	dma_fence_chain_free(sync->ufence_chain_fence);
-	if (!IS_ERR_OR_NULL(sync->ufence))
+	if (!IS_ERR_OR_NULL(sync->ufence)) {
+		drm_user_fence_cancel_sync(&sync->ufence->base);
 		user_fence_put(sync->ufence);
+	}
 }
 
 /**
@@ -434,6 +421,19 @@ void xe_sync_ufence_put(struct xe_user_fence *ufence)
 	user_fence_put(ufence);
 }
 
+/**
+ * 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
@@ -443,4 +443,3 @@ void xe_sync_ufence_put(struct xe_user_fence *ufence)
 int xe_sync_ufence_get_status(struct xe_user_fence *ufence)
 {
 	return READ_ONCE(ufence->signalled);
-}
+}
diff --git a/drivers/gpu/drm/xe/xe_sync.h b/drivers/gpu/drm/xe/xe_sync.h
index 6b949194acff..ff47882b3f6f 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_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..49df2bfa817c 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;
@@ -28,4 +27,3 @@ struct xe_sync_entry {
 	u32 flags;
 };
 
-#endif
+#endif
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index b01f31ed4417..0a1f7615aed9 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_sync(vma->ufence);
 		xe_sync_ufence_put(vma->ufence);
 		vma->ufence = NULL;
 	}
-- 
2.34.1

Re: [PATCH v3 2/2] 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:
- [High] Canceling a queued drm_user_fence work item leaks a reference to the user fence, its mm_struct, and the stored dma_fence.
- [High] Unconditional cancellation of user fences in xe_sync_entry_cleanup() completely breaks VM_BIND user fences.
--

commit 0c5a87e71dfa22fb04a23aa8d1dc27d00d244cde
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>

drm/xe: Convert xe_user_fence to drm_user_fence

This patch replaces the open-coded user fence implementation in the xe driver
with the new common drm_user_fence helper. It moves the local destruction and
worker logic into xe_ufence_ops callbacks which are executed by the core helper.

> diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
> --- a/drivers/gpu/drm/xe/xe_sync.c
> +++ b/drivers/gpu/drm/xe/xe_sync.c

[ ... ]

> @@ -310,8 +295,10 @@ void xe_sync_entry_cleanup(struct xe_sync_entry *sync)
>  	dma_fence_put(sync->fence);
>  	dma_fence_chain_free(sync->chain_fence);
>  	dma_fence_chain_free(sync->ufence_chain_fence);
> -	if (!IS_ERR_OR_NULL(sync->ufence))
> +	if (!IS_ERR_OR_NULL(sync->ufence)) {
> +		drm_user_fence_cancel_sync(&sync->ufence->base);

[Severity: High]
Does this unconditionally cancel the user fence worker during the ioctl's
teardown path?

During the normal, successful execution of xe_vm_bind_ioctl() in
drivers/gpu/drm/xe/xe_vm.c, the function falls through to the free_syncs
label and calls xe_sync_entry_cleanup() for all parsed sync entries:

    free_syncs:
        ...
        while (num_syncs--)
            xe_sync_entry_cleanup(&syncs[num_syncs]);

If the worker is canceled immediately after the ioctl submits the GPU work,
it appears the user fence is disarmed before the GPU finishes. Could this
cause userspace applications using VM_BIND with user fences to hang
indefinitely, as the completion signal is never delivered?

[Severity: High]
Additionally, when drm_user_fence_cancel_sync() is called here, can it leak
a reference to the user fence?

Looking at the implementation of drm_user_fence_cancel_sync() in
drivers/gpu/drm/drm_user_fence.c:

    void drm_user_fence_cancel_sync(struct drm_user_fence *ufence)
    {
        drm_user_fence_cancel(ufence);
        cancel_work_sync(&ufence->work);
    }

The return value of cancel_work_sync() is ignored. If it returns true
(meaning the work was queued but hadn't started yet), the 
drm_user_fence_work() worker function is bypassed. 

Since the work item owns a reference to the user fence (acquired during
drm_user_fence_add_callback()), skipping the worker means
drm_user_fence_put() is never called. Could this result in a memory leak of
the drm_user_fence structure, its mmgrab() reference, and its stored 
dma_fence?

>  		user_fence_put(sync->ufence);
> +	}
>  }

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

✗ CI.checkpatch: warning for drm: Add Common drm_user_fence helper and Convert XE (rev2)

[Patchwork]

== Series Details ==

Series: drm: Add Common drm_user_fence helper and Convert XE (rev2)
URL   : https://patchwork.freedesktop.org/series/172833/
State : warning

== Summary ==

+ KERNEL=/kernel
+ git clone https://gitlab.freedesktop.org/drm/maintainer-tools mt
Cloning into 'mt'...
warning: redirecting to https://gitlab.freedesktop.org/drm/maintainer-tools.git/
+ git -C mt rev-list -n1 origin/master
061140b9bc586ae7f40abc1249c97e1cc72d1b9d
+ cd /kernel
+ git config --global --add safe.directory /kernel
+ git log -n1
commit 028e53acf31d7f303586bf861b67ca17f2316548
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Date:   Thu Aug 27 20:49:50 2026 +0530

    drm/xe: Convert xe_user_fence to drm_user_fence
    
    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.worker and
    xe_ufence_ops.destroy, which are called by drm_user_fence_work().
    
    Cc: Matthew Brost <matthew.brost@intel.com>
    Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
    Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
    Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
    Cc: David Airlie <airlied@gmail.com>
    Cc: Simona Vetter <simona@ffwll.ch>
    Cc: Sumit Semwal <sumit.semwal@linaro.org>
    Cc: Christian König <christian.koenig@amd.com>
    Cc: Alex Deucher <alexander.deucher@amd.com>
    Cc: intel-xe@lists.freedesktop.org
    Cc: dri-devel@lists.freedesktop.org
    Cc: linux-media@vger.kernel.org
    Cc: linaro-mm-sig@lists.linaro.org
    Cc: linux-kernel@vger.kernel.org
    Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
+ /mt/dim checkpatch 98ac27fc649cf3f3f07a56c0d3df839be8bfa95d drm-intel
28720c6be0e2 drm: Add common drm_user_fence helper
-:61: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#61: 
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 291 lines checked
028e53acf31d drm/xe: Convert xe_user_fence to drm_user_fence

✗ CI.KUnit: failure for drm: Add Common drm_user_fence helper and Convert XE (rev2)

[Patchwork]

== Series Details ==

Series: drm: Add Common drm_user_fence helper and Convert XE (rev2)
URL   : https://patchwork.freedesktop.org/series/172833/
State : failure

== Summary ==

+ trap cleanup EXIT
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig
ERROR:root:In file included from ../drivers/gpu/drm/xe/xe_sync.h:9,
                 from ../drivers/gpu/drm/xe/xe_exec.c:21:
../drivers/gpu/drm/xe/xe_sync_types.h:6: error: unterminated #ifndef
    6 | #ifndef _XE_SYNC_TYPES_H_
      | 
make[7]: *** [../scripts/Makefile.build:289: drivers/gpu/drm/xe/xe_exec.o] Error 1
make[7]: *** Waiting for unfinished jobs....
make[6]: *** [../scripts/Makefile.build:549: drivers/gpu/drm/xe] Error 2
make[6]: *** Waiting for unfinished jobs....
make[5]: *** [../scripts/Makefile.build:549: drivers/gpu/drm] Error 2
make[4]: *** [../scripts/Makefile.build:549: drivers/gpu] Error 2
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [../scripts/Makefile.build:549: drivers] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [/kernel/Makefile:2187: .] Error 2
make[1]: *** [/kernel/Makefile:248: __sub-make] Error 2
make: *** [Makefile:248: __sub-make] Error 2

[15:50:19] Configuring KUnit Kernel ...
Generating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[15:50:23] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=48
+ cleanup
++ stat -c %u:%g /kernel
+ chown -R 1003:1003 /kernel