Re: [PATCH v6 2/4] drm: Add drm_user_fence helper

[Matthew Brost <matthew.brost@intel.com>]

On Mon, Aug 31, 2026 at 07:15:37PM +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: Matthew Brost <matthew.brost@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: intel-xe@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
>  drivers/gpu/drm/Makefile         |  1 +
>  drivers/gpu/drm/drm_user_fence.c | 69 +++++++++++++++++++++++++
>  include/drm/drm_user_fence.h     | 86 ++++++++++++++++++++++++++++++++
>  3 files changed, 156 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_user_fence.c
>  create mode 100644 include/drm/drm_user_fence.h
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index c5be8e80d0c8..ddb770738992 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -69,6 +69,7 @@ drm-y := \
>  	drm_syncobj.o \
>  	drm_sysfs.o \
>  	drm_trace_points.o \
> +	drm_user_fence.o \
>  	drm_vblank.o \
>  	drm_vblank_work.o \
>  	drm_vma_manager.o \
> diff --git a/drivers/gpu/drm/drm_user_fence.c b/drivers/gpu/drm/drm_user_fence.c
> new file mode 100644
> index 000000000000..664178e2d74c
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_user_fence.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 The Linux Foundation
> + *
> + * DRM user fence — extends drm_work_fence with kthread_use_mm() support.
> + *
> + * Use this when a GPU fence signals and work needs to access userspace
> + * memory (copy_to_user, fault-able operations) from a kthread context.
> + * For work that does not require userspace memory access, use
> + * drm_work_fence directly.
> + */
> +
> +#include <linux/kthread.h>
> +#include <linux/sched/mm.h>
> +
> +#include <drm/drm_user_fence.h>
> +
> +static void drm_user_fence_do_work(struct drm_work_fence *wfence)
> +{
> +	struct drm_user_fence *ufence =
> +		container_of(wfence, struct drm_user_fence, base);
> +	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_async(ufence->mm);

Xe does this incorrectly, but ufence shouldn't be looked after 'worker'.

Also mm_ok probably isn't needed either. I'd write this like:

struct mm_struct *mm = NULL;

if (mmget_not_zero(ufence->mm)) {
	mm = ufence->mm;
	kthread_use_mm(mm);
}

ufence->ops->worker(ufence, !!mm); /* Or just pass in 'mm' */

if (mm) {
	kthread_unuse_mm(mm);
	mmput_async(mm);
}

> +	}
> +}
> +
> +static void drm_user_fence_do_destroy(struct drm_work_fence *wfence)
> +{
> +	struct drm_user_fence *ufence =
> +		container_of(wfence, struct drm_user_fence, base);
> +
> +	mmdrop(ufence->mm);
> +	ufence->ops->destroy(ufence);

I'd invert this:

struct mm_struct *mm = ufence->mm;

ufence->ops->destroy(ufence);
mmdrop(mm);

> +}
> +
> +static const struct drm_work_fence_ops drm_user_fence_wf_ops = {
> +	.work    = drm_user_fence_do_work,

I wouldn't use the name 'work' here. I think writeback is more apporiate.

> +	.destroy = drm_user_fence_do_destroy,
> +};
> +
> +/**
> + * 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)
> +{
> +	drm_work_fence_init(&ufence->base, wq, &drm_user_fence_wf_ops);
> +	ufence->mm = current->mm;
> +	mmgrab(ufence->mm);
> +	ufence->ops = ops;
> +}
> +EXPORT_SYMBOL_GPL(drm_user_fence_init);
> diff --git a/include/drm/drm_user_fence.h b/include/drm/drm_user_fence.h
> new file mode 100644
> index 000000000000..2b2b640f510f
> --- /dev/null
> +++ b/include/drm/drm_user_fence.h
> @@ -0,0 +1,86 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2024 The Linux Foundation
> + */
> +
> +#ifndef __DRM_USER_FENCE_H__
> +#define __DRM_USER_FENCE_H__
> +
> +#include <drm/drm_work_fence.h>
> +
> +struct drm_user_fence;
> +
> +/**
> + * struct drm_user_fence_ops - driver callbacks for a DRM user fence
> + */
> +struct drm_user_fence_ops {
> +	/**
> +	 * @worker: Called from workqueue context with the process MM active.
> +	 *
> +	 * If @mm_ok is true, kthread_use_mm() is active and userspace memory
> +	 * (copy_to_user, etc.) may be accessed safely.
> +	 * If @mm_ok is false, the process MM was already gone; the driver
> +	 * should log a warning and skip the userspace write.

I'd wouldn't dicate if caller should log a warning - rather just say if
should 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 - DRM user fence with MM borrowing
> + *
> + * Extends drm_work_fence with kthread_use_mm() support for drivers
> + * that need to access userspace memory when a GPU fence signals.
> + * For work that does not need userspace memory access, use
> + * drm_work_fence directly.
> + *
> + * Call drm_user_fence_init() at creation and drm_user_fence_add_callback()
> + * to arm on a dma-fence. Call drm_user_fence_cancel_sync() before teardown.
> + */
> +struct drm_user_fence {
> +	/** @base: Base work fence. Must be first. */
> +	struct drm_work_fence base;
> +	/** @mm: Process MM grabbed at init time. */
> +	struct mm_struct *mm;
> +	/** @ops: Driver operations. */
> +	const struct drm_user_fence_ops *ops;
> +};
> +
> +void drm_user_fence_init(struct drm_user_fence *ufence,
> +			 struct workqueue_struct *wq,
> +			 const struct drm_user_fence_ops *ops);
> +

Kernel doc for all the inlines.

Matt

> +static inline void drm_user_fence_get(struct drm_user_fence *ufence)
> +{
> +	drm_work_fence_get(&ufence->base);
> +}
> +
> +static inline void drm_user_fence_put(struct drm_user_fence *ufence)
> +{
> +	drm_work_fence_put(&ufence->base);
> +}
> +
> +static inline int drm_user_fence_add_callback(struct drm_user_fence *ufence,
> +					      struct dma_fence *fence)
> +{
> +	return drm_work_fence_add_callback(&ufence->base, fence);
> +}
> +
> +static inline bool drm_user_fence_cancel(struct drm_user_fence *ufence)
> +{
> +	return drm_work_fence_cancel(&ufence->base);
> +}
> +
> +static inline void drm_user_fence_cancel_sync(struct drm_user_fence *ufence)
> +{
> +	drm_work_fence_cancel_sync(&ufence->base);
> +}
> +
> +#endif /* __DRM_USER_FENCE_H__ */
> -- 
> 2.34.1
>