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

[PATCH v4 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>
---
v4:
 - Check cancel_work_sync() return value in drm_user_fence_cancel_sync()
   and call drm_user_fence_put() if work was dequeued, fixing a memory
   leak of the drm_user_fence, mm_struct and stored dma_fence when a
   pending work item is cancelled. (Sashiko review)

 drivers/gpu/drm/Makefile         |   1 +
 drivers/gpu/drm/drm_user_fence.c | 223 +++++++++++++++++++++++++++++++
 include/drm/drm_user_fence.h     |  76 +++++++++++
 3 files changed, 300 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..cdc47d092cbb
--- /dev/null
+++ b/drivers/gpu/drm/drm_user_fence.c
@@ -0,0 +1,223 @@
+// 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 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)
+{
+	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 no additional
+ * action is needed — the callback handles its own reference.
+ *
+ * If removal succeeds the callback reference is released internally.
+ * The caller must still release its own separate reference via
+ * drm_user_fence_put() when done with the object.
+ *
+ * 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_user_fence_cancel_sync() instead,
+ * which may sleep.
+ *
+ * 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.
+ *
+ * This function may sleep. Must not be called from atomic or interrupt
+ * context. Use drm_user_fence_cancel() instead when sleeping is not
+ * allowed.
+ *
+ * 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);
+	if (cancel_work_sync(&ufence->work))
+		drm_user_fence_put(ufence);
+}
+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 v4 1/2] drm: Add common drm_user_fence helper

[Matthew Brost]

On Fri, Aug 28, 2026 at 12:01:02PM +0530, Srinivasan Shanmugam wrote:
> 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>

First off, I'm supportive of the idea of a common DRM layer for user
fences and updating Xe accordingly.

This isn't a complete review, but here's a quick initial suggestion.

Also, by the way, you're still fighting our CI [1]. Feel free to keep
hammering on it, as that's what it's there for. iirc if kunit fails as
in this case, nothing else will run. Ask AI and should be able to get
instructions on how to build our kunit + run it (it doesn't require
Intel hardware in a lot of cases).

[1] https://patchwork.freedesktop.org/series/172930/

> 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>
> ---
> v4:
>  - Check cancel_work_sync() return value in drm_user_fence_cancel_sync()
>    and call drm_user_fence_put() if work was dequeued, fixing a memory
>    leak of the drm_user_fence, mm_struct and stored dma_fence when a
>    pending work item is cancelled. (Sashiko review)
> 
>  drivers/gpu/drm/Makefile         |   1 +
>  drivers/gpu/drm/drm_user_fence.c | 223 +++++++++++++++++++++++++++++++
>  include/drm/drm_user_fence.h     |  76 +++++++++++
>  3 files changed, 300 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..cdc47d092cbb
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_user_fence.c
> @@ -0,0 +1,223 @@
> +// 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 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)
> +{
> +	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 no additional
> + * action is needed — the callback handles its own reference.
> + *
> + * If removal succeeds the callback reference is released internally.
> + * The caller must still release its own separate reference via
> + * drm_user_fence_put() when done with the object.
> + *
> + * 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_user_fence_cancel_sync() instead,
> + * which may sleep.
> + *
> + * 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.
> + *
> + * This function may sleep. Must not be called from atomic or interrupt
> + * context. Use drm_user_fence_cancel() instead when sleeping is not
> + * allowed.
> + *
> + * 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);
> +	if (cancel_work_sync(&ufence->work))
> +		drm_user_fence_put(ufence);
> +}
> +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 {

Should this common layer be split into two distinct concepts?

- drm_work_fence: 90% of what is here, minus the kthread_use_mm() and
  mm-related code.
- drm_user_fence: a subclass of drm_work_fence that adds the
  kthread_use_mm() and mm-related code.

I suggest this because I was thinking about it the other day (I forget
the exact context) and reconsidered a pattern where a fence signals and
then I need a worker because some work must be done outside of IRQ
context. A user fence is one example, since copy_to_user() can fault,
which is not allowed in IRQ context. At various times in Xe we've had
multiple patterns like this, although at the moment user fences are
probably the only case that requires it. If we looked across DRM as a
whole, I suspect we'd find this pattern open-coded in a number of
places.

Yes, drm_user_fence would be a very thin layer on top of drm_work_fence,
but I still see value in the split.
 
Matt

> +	/** @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 v4 1/2] drm: Add common drm_user_fence helper

[SHANMUGAM, SRINIVASAN]

AMD General

> -----Original Message-----
> From: Matthew Brost <matthew.brost@intel.com>
> Sent: Friday, August 28, 2026 12:47 PM
> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>
> Cc: Koenig, Christian <Christian.Koenig@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>;
> Thomas Zimmermann <tzimmermann@suse.de>; David Airlie
> <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Sumit Semwal
> <sumit.semwal@linaro.org>; Thomas Hellström
> <thomas.hellstrom@linux.intel.com>; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; linux-media@vger.kernel.org; linaro-mm-
> sig@lists.linaro.org; linux-kernel@vger.kernel.org; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH v4 1/2] drm: Add common drm_user_fence helper
>
> On Fri, Aug 28, 2026 at 12:01:02PM +0530, Srinivasan Shanmugam wrote:
> > 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>
>
> First off, I'm supportive of the idea of a common DRM layer for user fences and
> updating Xe accordingly.
>
> This isn't a complete review, but here's a quick initial suggestion.
>
> Also, by the way, you're still fighting our CI [1]. Feel free to keep hammering on it, as
> that's what it's there for. iirc if kunit fails as in this case, nothing else will run. Ask AI
> and should be able to get instructions on how to build our kunit + run it (it doesn't
> require Intel hardware in a lot of cases).
>
> [1] https://patchwork.freedesktop.org/series/172930/
>
> > 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>
> > ---
> > v4:
> >  - Check cancel_work_sync() return value in drm_user_fence_cancel_sync()
> >    and call drm_user_fence_put() if work was dequeued, fixing a memory
> >    leak of the drm_user_fence, mm_struct and stored dma_fence when a
> >    pending work item is cancelled. (Sashiko review)
> >
> >  drivers/gpu/drm/Makefile         |   1 +
> >  drivers/gpu/drm/drm_user_fence.c | 223
> +++++++++++++++++++++++++++++++
> >  include/drm/drm_user_fence.h     |  76 +++++++++++
> >  3 files changed, 300 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..cdc47d092cbb
> > --- /dev/null
> > +++ b/drivers/gpu/drm/drm_user_fence.c
> > @@ -0,0 +1,223 @@
> > +// 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 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) {
> > +   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 no
> > +additional
> > + * action is needed — the callback handles its own reference.
> > + *
> > + * If removal succeeds the callback reference is released internally.
> > + * The caller must still release its own separate reference via
> > + * drm_user_fence_put() when done with the object.
> > + *
> > + * 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_user_fence_cancel_sync()
> > +instead,
> > + * which may sleep.
> > + *
> > + * 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.
> > + *
> > + * This function may sleep. Must not be called from atomic or
> > +interrupt
> > + * context. Use drm_user_fence_cancel() instead when sleeping is not
> > + * allowed.
> > + *
> > + * 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);
> > +   if (cancel_work_sync(&ufence->work))
> > +           drm_user_fence_put(ufence);
> > +}
> > +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 {
>
> Should this common layer be split into two distinct concepts?
>
> - drm_work_fence: 90% of what is here, minus the kthread_use_mm() and
>   mm-related code.
> - drm_user_fence: a subclass of drm_work_fence that adds the
>   kthread_use_mm() and mm-related code.
>
> I suggest this because I was thinking about it the other day (I forget the exact
> context) and reconsidered a pattern where a fence signals and then I need a worker
> because some work must be done outside of IRQ context. A user fence is one
> example, since copy_to_user() can fault, which is not allowed in IRQ context. At
> various times in Xe we've had multiple patterns like this, although at the moment
> user fences are probably the only case that requires it. If we looked across DRM as
> a whole, I suspect we'd find this pattern open-coded in a number of places.
>
> Yes, drm_user_fence would be a very thin layer on top of drm_work_fence, but I still
> see value in the split.

Hi Matt,

Thanks for the review and for being supportive of the idea.

The split into drm_work_fence (general fence-to-workqueue pattern) and
drm_user_fence (subclass adding kthread_use_mm) makes sense. I'll
restructure v5 as follows:

  drm_work_fence: kref, work_struct, dma_fence_cb, stored fence ref,
                  wq, ops — add_callback, cancel, cancel_sync

  drm_user_fence: embeds drm_work_fence, adds mm_struct and the
                  kthread_use_mm/mmput boilerplate, thin wrappers

XE will continue to use drm_user_fence. For AMDGPU, The long-term
per-signal filtering approach (reading the fence value via copy_from_user
before signaling) will use drm_user_fence — further validating both
layers of the split.

Regarding the CI failure — the root cause was a missing trailing newline
at the end of xe_sync_types.h which caused the kunit build to fail with
"unterminated #ifndef". I've set up kunit locally and confirmed the fix:

  Testing complete. Ran 588 tests: passed: 570, skipped: 18
  Elapsed time: 22.916s total, 3.949s configuring, 18.350s building,
  0.601s running

The 18 skipped tests require Intel hardware — expected. The CI fix will
be included in v5 along with the drm_work_fence restructuring.

Thanks,
Srini

>
> Matt

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

[Christian König]

On 8/28/26 10:06, SHANMUGAM, SRINIVASAN wrote:
...
>>> +/**
>>> + * 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 {
>>
>> Should this common layer be split into two distinct concepts?
>>
>> - drm_work_fence: 90% of what is here, minus the kthread_use_mm() and
>>   mm-related code.
>> - drm_user_fence: a subclass of drm_work_fence that adds the
>>   kthread_use_mm() and mm-related code.
>>
>> I suggest this because I was thinking about it the other day (I forget the exact
>> context) and reconsidered a pattern where a fence signals and then I need a worker
>> because some work must be done outside of IRQ context. A user fence is one
>> example, since copy_to_user() can fault, which is not allowed in IRQ context. At
>> various times in Xe we've had multiple patterns like this, although at the moment
>> user fences are probably the only case that requires it. If we looked across DRM as
>> a whole, I suspect we'd find this pattern open-coded in a number of places.
>>
>> Yes, drm_user_fence would be a very thin layer on top of drm_work_fence, but I still
>> see value in the split.
> 
> Hi Matt,
> 
> Thanks for the review and for being supportive of the idea.
> 
> The split into drm_work_fence (general fence-to-workqueue pattern) and
> drm_user_fence (subclass adding kthread_use_mm) makes sense. I'll
> restructure v5 as follows:
> 
>   drm_work_fence: kref, work_struct, dma_fence_cb, stored fence ref,
>                   wq, ops — add_callback, cancel, cancel_sync

Yeah, this pattern came up so often that I already considered adding it to the core dma_fence framework.

So if you feel really brave make that a dma_fence_work helper. If I'm not completely mistaken AI should be able to find quite a number of use cases for that already.

> 
>   drm_user_fence: embeds drm_work_fence, adds mm_struct and the
>                   kthread_use_mm/mmput boilerplate, thin wrappers
> 
> XE will continue to use drm_user_fence. For AMDGPU, The long-term
> per-signal filtering approach (reading the fence value via copy_from_user
> before signaling) will use drm_user_fence — further validating both
> layers of the split.

It would be really nice if we could move those compare functionality (>, <, !=, == etc...) XE has for the user value into the drm_user_fence handling as well.

We also need to add a heck of documentation that while this is able to consume dma_fences it *CAN'T* be used to implement dma_fence_ops. I had more than enough headache because of that.

Regards,
Christian.
> 
> Regarding the CI failure — the root cause was a missing trailing newline
> at the end of xe_sync_types.h which caused the kunit build to fail with
> "unterminated #ifndef". I've set up kunit locally and confirmed the fix:
> 
>   Testing complete. Ran 588 tests: passed: 570, skipped: 18
>   Elapsed time: 22.916s total, 3.949s configuring, 18.350s building,
>   0.601s running
> 
> The 18 skipped tests require Intel hardware — expected. The CI fix will
> be included in v5 along with the drm_work_fence restructuring.
> 
> Thanks,
> Srini
> 
>>
>> Matt

RE: [PATCH v4 1/2] drm: Add common drm_user_fence helper

[SHANMUGAM, SRINIVASAN]

AMD General

> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Friday, August 28, 2026 1:48 PM
> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
> Matthew Brost <matthew.brost@intel.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>;
> Thomas Zimmermann <tzimmermann@suse.de>; David Airlie
> <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Sumit Semwal
> <sumit.semwal@linaro.org>; Thomas Hellström
> <thomas.hellstrom@linux.intel.com>; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; linux-media@vger.kernel.org; linaro-mm-
> sig@lists.linaro.org; linux-kernel@vger.kernel.org; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH v4 1/2] drm: Add common drm_user_fence helper
>
> On 8/28/26 10:06, SHANMUGAM, SRINIVASAN wrote:
> ...
> >>> +/**
> >>> + * 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 {
> >>
> >> Should this common layer be split into two distinct concepts?
> >>
> >> - drm_work_fence: 90% of what is here, minus the kthread_use_mm() and
> >>   mm-related code.
> >> - drm_user_fence: a subclass of drm_work_fence that adds the
> >>   kthread_use_mm() and mm-related code.
> >>
> >> I suggest this because I was thinking about it the other day (I
> >> forget the exact
> >> context) and reconsidered a pattern where a fence signals and then I
> >> need a worker because some work must be done outside of IRQ context.
> >> A user fence is one example, since copy_to_user() can fault, which is
> >> not allowed in IRQ context. At various times in Xe we've had multiple
> >> patterns like this, although at the moment user fences are probably
> >> the only case that requires it. If we looked across DRM as a whole, I suspect
> we'd find this pattern open-coded in a number of places.
> >>
> >> Yes, drm_user_fence would be a very thin layer on top of
> >> drm_work_fence, but I still see value in the split.
> >
> > Hi Matt,
> >
> > Thanks for the review and for being supportive of the idea.
> >
> > The split into drm_work_fence (general fence-to-workqueue pattern) and
> > drm_user_fence (subclass adding kthread_use_mm) makes sense. I'll
> > restructure v5 as follows:
> >
> >   drm_work_fence: kref, work_struct, dma_fence_cb, stored fence ref,
> >                   wq, ops — add_callback, cancel, cancel_sync
>
> Yeah, this pattern came up so often that I already considered adding it to the core
> dma_fence framework.

Hi Christian,

Thanks for the feedback.

On dma_fence_work: would you prefer I place the generic fence-to-work
helper directly in the core dma_fence framework (drivers/dma-buf/),
or is starting with drm_work_fence in DRM and promoting it later also
acceptable?

I'll add the value comparison logic and will add a clear note that this cannot be
used to implement dma_fence_ops.

Thanks,
Srini

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

[Christian König]

On 8/28/26 10:31, SHANMUGAM, SRINIVASAN wrote:
> AMD General
> 
>> -----Original Message-----
>> From: Koenig, Christian <Christian.Koenig@amd.com>
>> Sent: Friday, August 28, 2026 1:48 PM
>> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
>> Matthew Brost <matthew.brost@intel.com>
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Maarten Lankhorst
>> <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>;
>> Thomas Zimmermann <tzimmermann@suse.de>; David Airlie
>> <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Sumit Semwal
>> <sumit.semwal@linaro.org>; Thomas Hellström
>> <thomas.hellstrom@linux.intel.com>; dri-devel@lists.freedesktop.org; intel-
>> xe@lists.freedesktop.org; linux-media@vger.kernel.org; linaro-mm-
>> sig@lists.linaro.org; linux-kernel@vger.kernel.org; amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH v4 1/2] drm: Add common drm_user_fence helper
>>
>> On 8/28/26 10:06, SHANMUGAM, SRINIVASAN wrote:
>> ...
>>>>> +/**
>>>>> + * 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 {
>>>>
>>>> Should this common layer be split into two distinct concepts?
>>>>
>>>> - drm_work_fence: 90% of what is here, minus the kthread_use_mm() and
>>>>   mm-related code.
>>>> - drm_user_fence: a subclass of drm_work_fence that adds the
>>>>   kthread_use_mm() and mm-related code.
>>>>
>>>> I suggest this because I was thinking about it the other day (I
>>>> forget the exact
>>>> context) and reconsidered a pattern where a fence signals and then I
>>>> need a worker because some work must be done outside of IRQ context.
>>>> A user fence is one example, since copy_to_user() can fault, which is
>>>> not allowed in IRQ context. At various times in Xe we've had multiple
>>>> patterns like this, although at the moment user fences are probably
>>>> the only case that requires it. If we looked across DRM as a whole, I suspect
>> we'd find this pattern open-coded in a number of places.
>>>>
>>>> Yes, drm_user_fence would be a very thin layer on top of
>>>> drm_work_fence, but I still see value in the split.
>>>
>>> Hi Matt,
>>>
>>> Thanks for the review and for being supportive of the idea.
>>>
>>> The split into drm_work_fence (general fence-to-workqueue pattern) and
>>> drm_user_fence (subclass adding kthread_use_mm) makes sense. I'll
>>> restructure v5 as follows:
>>>
>>>   drm_work_fence: kref, work_struct, dma_fence_cb, stored fence ref,
>>>                   wq, ops — add_callback, cancel, cancel_sync
>>
>> Yeah, this pattern came up so often that I already considered adding it to the core
>> dma_fence framework.
> 
> Hi Christian,
> 
> Thanks for the feedback.
> 
> On dma_fence_work: would you prefer I place the generic fence-to-work
> helper directly in the core dma_fence framework (drivers/dma-buf/),
> or is starting with drm_work_fence in DRM and promoting it later also
> acceptable?

Maybe ask AI to search for use cases. If you find something outside of drivers/gpu/drm then please place it under drivers/dma-buf.

If you don't find any existing use case drivers/gpu/drm should do as well.

Thanks,
Christian.

> 
> I'll add the value comparison logic and will add a clear note that this cannot be
> used to implement dma_fence_ops.
> 
> Thanks,
> Srini

RE: [PATCH v4 1/2] drm: Add common drm_user_fence helper

[SHANMUGAM, SRINIVASAN]

AMD General

> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Friday, August 28, 2026 2:43 PM
> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
> Matthew Brost <matthew.brost@intel.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>;
> Thomas Zimmermann <tzimmermann@suse.de>; David Airlie
> <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Sumit Semwal
> <sumit.semwal@linaro.org>; Thomas Hellström
> <thomas.hellstrom@linux.intel.com>; dri-devel@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org; linux-media@vger.kernel.org; linaro-mm-
> sig@lists.linaro.org; linux-kernel@vger.kernel.org; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH v4 1/2] drm: Add common drm_user_fence helper
>
> On 8/28/26 10:31, SHANMUGAM, SRINIVASAN wrote:
> > AMD General
> >
> >> -----Original Message-----
> >> From: Koenig, Christian <Christian.Koenig@amd.com>
> >> Sent: Friday, August 28, 2026 1:48 PM
> >> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
> Matthew
> >> Brost <matthew.brost@intel.com>
> >> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Maarten Lankhorst
> >> <maarten.lankhorst@linux.intel.com>; Maxime Ripard
> >> <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David
> >> Airlie <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Sumit
> >> Semwal <sumit.semwal@linaro.org>; Thomas Hellström
> >> <thomas.hellstrom@linux.intel.com>; dri-devel@lists.freedesktop.org;
> >> intel- xe@lists.freedesktop.org; linux-media@vger.kernel.org;
> >> linaro-mm- sig@lists.linaro.org; linux-kernel@vger.kernel.org;
> >> amd-gfx@lists.freedesktop.org
> >> Subject: Re: [PATCH v4 1/2] drm: Add common drm_user_fence helper
> >>
> >> On 8/28/26 10:06, SHANMUGAM, SRINIVASAN wrote:
> >> ...
> >>>>> +/**
> >>>>> + * 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 {
> >>>>
> >>>> Should this common layer be split into two distinct concepts?
> >>>>
> >>>> - drm_work_fence: 90% of what is here, minus the kthread_use_mm() and
> >>>>   mm-related code.
> >>>> - drm_user_fence: a subclass of drm_work_fence that adds the
> >>>>   kthread_use_mm() and mm-related code.
> >>>>
> >>>> I suggest this because I was thinking about it the other day (I
> >>>> forget the exact
> >>>> context) and reconsidered a pattern where a fence signals and then
> >>>> I need a worker because some work must be done outside of IRQ context.
> >>>> A user fence is one example, since copy_to_user() can fault, which
> >>>> is not allowed in IRQ context. At various times in Xe we've had
> >>>> multiple patterns like this, although at the moment user fences are
> >>>> probably the only case that requires it. If we looked across DRM as
> >>>> a whole, I suspect
> >> we'd find this pattern open-coded in a number of places.
> >>>>
> >>>> Yes, drm_user_fence would be a very thin layer on top of
> >>>> drm_work_fence, but I still see value in the split.
> >>>
> >>> Hi Matt,
> >>>
> >>> Thanks for the review and for being supportive of the idea.
> >>>
> >>> The split into drm_work_fence (general fence-to-workqueue pattern)
> >>> and drm_user_fence (subclass adding kthread_use_mm) makes sense.
> >>> I'll restructure v5 as follows:
> >>>
> >>>   drm_work_fence: kref, work_struct, dma_fence_cb, stored fence ref,
> >>>                   wq, ops — add_callback, cancel, cancel_sync
> >>
> >> Yeah, this pattern came up so often that I already considered adding
> >> it to the core dma_fence framework.
> >
> > Hi Christian,
> >
> > Thanks for the feedback.
> >
> > On dma_fence_work: would you prefer I place the generic fence-to-work
> > helper directly in the core dma_fence framework (drivers/dma-buf/), or
> > is starting with drm_work_fence in DRM and promoting it later also
> > acceptable?
>
> Maybe ask AI to search for use cases. If you find something outside of
> drivers/gpu/drm then please place it under drivers/dma-buf.
>
> If you don't find any existing use case drivers/gpu/drm should do as well.

Hi Christian,

Searched the tree for files combining dma_fence_cb + work_struct
outside of drivers/gpu/drm/:

  $ grep -rl "dma_fence_cb" . --include="*.c" | \
      grep -v "drivers/gpu/drm" | \
      xargs grep -l "work_struct" 2>/dev/null
  (no output)

No matches found outside drivers/gpu/drm/. Files using
dma_fence_add_callback outside DRM are the core framework itself
(dma-fence.c, dma-fence-chain.c, dma-buf.c) — not consumers of the
fence-to-work pattern.

Based on your suggestions, I'll keep the helper as drm_work_fence in
drivers/gpu/drm/. I'll add the value comparison logic and the
dma_fence_ops warning in the next version.

Thanks,
Srini

[PATCH v4 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>
---
v4:
 - Remove drm_user_fence_cancel_sync() from xe_sync_entry_cleanup().
   Cancelling the callback there breaks VM_BIND user fences by
   disarming the completion signal before the GPU finishes.
   (Sashiko review)

 drivers/gpu/drm/xe/xe_sync.c       | 136 ++++++++++++++---------------
 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, 70 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
index 37866768d64c..56f8e290424e 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 */
 	}
 }
 
@@ -434,6 +419,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 +441,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

[PATCH v4 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>
---
v4:
 - Check cancel_work_sync() return value in drm_user_fence_cancel_sync()
   and call drm_user_fence_put() if work was dequeued, fixing a memory
   leak of the drm_user_fence, mm_struct and stored dma_fence when a
   pending work item is cancelled. (Sashiko review)

 drivers/gpu/drm/Makefile         |   1 +
 drivers/gpu/drm/drm_user_fence.c | 223 +++++++++++++++++++++++++++++++
 include/drm/drm_user_fence.h     |  76 +++++++++++
 3 files changed, 300 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..cdc47d092cbb
--- /dev/null
+++ b/drivers/gpu/drm/drm_user_fence.c
@@ -0,0 +1,223 @@
+// 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 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)
+{
+	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 no additional
+ * action is needed — the callback handles its own reference.
+ *
+ * If removal succeeds the callback reference is released internally.
+ * The caller must still release its own separate reference via
+ * drm_user_fence_put() when done with the object.
+ *
+ * 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_user_fence_cancel_sync() instead,
+ * which may sleep.
+ *
+ * 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.
+ *
+ * This function may sleep. Must not be called from atomic or interrupt
+ * context. Use drm_user_fence_cancel() instead when sleeping is not
+ * allowed.
+ *
+ * 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);
+	if (cancel_work_sync(&ufence->work))
+		drm_user_fence_put(ufence);
+}
+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