dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] dma-buf/sw_sync: put fence signaling into work item
@ 2025-08-12 14:34 Christian König
  2025-08-12 14:34 ` [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL Christian König
  0 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2025-08-12 14:34 UTC (permalink / raw)
  To: simona.vetter, tvrtko.ursulin, phasta, airlied, dakr,
	sumit.semwal, dri-devel, linux-media

From: Christian König <ckoenig@able.fritz.box>

Offload signaling fence in the sw_sync component into a work item to
improve testing the real world signaling conditions.

Needs more testing before pushing it upstream!

Signed-off-by: Christian König <ckoenig@able.fritz.box>
---
 drivers/dma-buf/sw_sync.c    | 25 ++++++++++++++-----------
 drivers/dma-buf/sync_debug.h |  2 ++
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 3c20f1d31cf5..43b8ac32482d 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -80,6 +80,7 @@ struct sw_sync_get_deadline {
 
 #define SW_SYNC_HAS_DEADLINE_BIT	DMA_FENCE_FLAG_USER_BITS
 
+static void sync_timeline_signal(struct work_struct *work);
 static const struct dma_fence_ops timeline_fence_ops;
 
 static inline struct sync_pt *dma_fence_to_sync_pt(struct dma_fence *fence)
@@ -110,6 +111,7 @@ static struct sync_timeline *sync_timeline_create(const char *name)
 
 	obj->pt_tree = RB_ROOT;
 	INIT_LIST_HEAD(&obj->pt_list);
+	INIT_WORK(&obj->signal_work, sync_timeline_signal);
 	spin_lock_init(&obj->lock);
 
 	sync_timeline_debug_add(obj);
@@ -123,6 +125,7 @@ static void sync_timeline_free(struct kref *kref)
 		container_of(kref, struct sync_timeline, kref);
 
 	sync_timeline_debug_remove(obj);
+	flush_work(&obj->signal_work);
 
 	kfree(obj);
 }
@@ -199,23 +202,20 @@ static const struct dma_fence_ops timeline_fence_ops = {
 
 /**
  * sync_timeline_signal() - signal a status change on a sync_timeline
- * @obj:	sync_timeline to signal
- * @inc:	num to increment on timeline->value
+ * @work: the work item
  *
- * A sync implementation should call this any time one of it's fences
- * has signaled or has an error condition.
+ * Signal all fences where the sequence number indicate to do so.
  */
-static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
+static void sync_timeline_signal(struct work_struct *work)
 {
+	struct sync_timeline *obj = container_of(work, typeof(*obj),
+						 signal_work);
 	LIST_HEAD(signalled);
 	struct sync_pt *pt, *next;
 
 	trace_sync_timeline(obj);
 
 	spin_lock_irq(&obj->lock);
-
-	obj->value += inc;
-
 	list_for_each_entry_safe(pt, next, &obj->pt_list, link) {
 		if (!timeline_fence_signaled(&pt->base))
 			break;
@@ -227,7 +227,6 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
 
 		dma_fence_signal_locked(&pt->base);
 	}
-
 	spin_unlock_irq(&obj->lock);
 
 	list_for_each_entry_safe(pt, next, &signalled, link) {
@@ -394,11 +393,15 @@ static long sw_sync_ioctl_inc(struct sync_timeline *obj, unsigned long arg)
 		return -EFAULT;
 
 	while (value > INT_MAX)  {
-		sync_timeline_signal(obj, INT_MAX);
+		obj->value += INT_MAX;
+
+		schedule_work(&obj->signal_work);
+		flush_work(&obj->signal_work);
 		value -= INT_MAX;
 	}
 
-	sync_timeline_signal(obj, value);
+	obj->value += value;
+	schedule_work(&obj->signal_work);
 
 	return 0;
 }
diff --git a/drivers/dma-buf/sync_debug.h b/drivers/dma-buf/sync_debug.h
index 02af347293d0..a1b03c48d82a 100644
--- a/drivers/dma-buf/sync_debug.h
+++ b/drivers/dma-buf/sync_debug.h
@@ -17,6 +17,7 @@
 #include <linux/rbtree.h>
 #include <linux/spinlock.h>
 #include <linux/dma-fence.h>
+#include <linux/workqueue.h>
 
 #include <linux/sync_file.h>
 #include <uapi/linux/sync_file.h>
@@ -40,6 +41,7 @@ struct sync_timeline {
 
 	struct rb_root		pt_tree;
 	struct list_head	pt_list;
+	struct work_struct	signal_work;
 	spinlock_t		lock;
 
 	struct list_head	sync_timeline_list;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
  2025-08-12 14:34 [PATCH 1/2] dma-buf/sw_sync: put fence signaling into work item Christian König
@ 2025-08-12 14:34 ` Christian König
  2025-08-13  8:16   ` Philipp Stanner
                     ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Christian König @ 2025-08-12 14:34 UTC (permalink / raw)
  To: simona.vetter, tvrtko.ursulin, phasta, airlied, dakr,
	sumit.semwal, dri-devel, linux-media

From: Christian König <ckoenig@able.fritz.box>

We have the re-occurring problem that people try to invent a
DMA-fences implementation which signals fences based on an userspace
IOCTL.

This is well known as source of hard to track down crashes and is
documented to be an invalid approach. The problem is that it seems
to work during initial testing and only long term tests points out
why this can never work correctly.

So give at least a warning when people try to signal a fence from
task context and not from interrupts or a work item. This check is
certainly not perfect but better than nothing.

Signed-off-by: Christian König <ckoenig@able.fritz.box>
---
 drivers/dma-buf/dma-fence.c | 59 +++++++++++++++++++++++++++----------
 include/linux/dma-fence.h   |  9 ++++--
 2 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 3f78c56b58dc..2bce620eacac 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -345,33 +345,23 @@ void __dma_fence_might_wait(void)
 }
 #endif
 
-
 /**
- * dma_fence_signal_timestamp_locked - signal completion of a fence
+ * dma_fence_signal_internal - internal signal completion of a fence
  * @fence: the fence to signal
  * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
  *
- * Signal completion for software callbacks on a fence, this will unblock
- * dma_fence_wait() calls and run all the callbacks added with
- * dma_fence_add_callback(). Can be called multiple times, but since a fence
- * can only go from the unsignaled to the signaled state and not back, it will
- * only be effective the first time. Set the timestamp provided as the fence
- * signal timestamp.
- *
- * Unlike dma_fence_signal_timestamp(), this function must be called with
- * &dma_fence.lock held.
+ * Internal signal the dma_fence without error checking. Should *NEVER* be used
+ * by drivers or external code directly.
  *
  * Returns 0 on success and a negative error value when @fence has been
  * signalled already.
  */
-int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
-				      ktime_t timestamp)
+int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp)
 {
 	struct dma_fence_cb *cur, *tmp;
 	struct list_head cb_list;
 
 	lockdep_assert_held(fence->lock);
-
 	if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
 				      &fence->flags)))
 		return -EINVAL;
@@ -390,7 +380,46 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
 
 	return 0;
 }
-EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
+EXPORT_SYMBOL(dma_fence_signal_internal);
+
+/**
+ * dma_fence_signal_timestamp_locked - signal completion of a fence
+ * @fence: the fence to signal
+ * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
+ *
+ * Signal completion for software callbacks on a fence, this will unblock
+ * dma_fence_wait() calls and run all the callbacks added with
+ * dma_fence_add_callback(). Can be called multiple times, but since a fence
+ * can only go from the unsignaled to the signaled state and not back, it will
+ * only be effective the first time. Set the timestamp provided as the fence
+ * signal timestamp.
+ *
+ * Unlike dma_fence_signal_timestamp(), this function must be called with
+ * &dma_fence.lock held.
+ *
+ * Returns 0 on success and a negative error value when @fence has been
+ * signalled already.
+ */
+int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
+				      ktime_t timestamp)
+{
+	/*
+	 * We have the re-occurring problem that people try to invent a
+	 * DMA-fences implementation which signals fences based on an userspace
+	 * IOCTL.
+	 *
+	 * This is well known as source of hard to track down crashes and is
+	 * documented to be an invalid approach. The problem is that it seems
+	 * to work during initial testing and only long term tests points out
+	 * why this can never work correctly.
+	 *
+	 * So give at least a warning when people try to signal a fence from
+	 * task context and not from interrupts or a work item. This check is
+	 * certainly not perfect but better than nothing.
+	 */
+	WARN_ON_ONCE(!in_interrupt() && !current_work());
+	return dma_fence_signal_internal(fence, timestamp);
+}
 
 /**
  * dma_fence_signal_timestamp - signal completion of a fence
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 64639e104110..8dbcd66989b8 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -369,6 +369,7 @@ int dma_fence_signal_locked(struct dma_fence *fence);
 int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
 int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
 				      ktime_t timestamp);
+int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp);
 signed long dma_fence_default_wait(struct dma_fence *fence,
 				   bool intr, signed long timeout);
 int dma_fence_add_callback(struct dma_fence *fence,
@@ -422,7 +423,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
 		return true;
 
 	if (fence->ops->signaled && fence->ops->signaled(fence)) {
-		dma_fence_signal_locked(fence);
+		dma_fence_signal_internal(fence, ktime_get());
 		return true;
 	}
 
@@ -448,11 +449,15 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
 static inline bool
 dma_fence_is_signaled(struct dma_fence *fence)
 {
+	unsigned long flags;
+
 	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
 		return true;
 
 	if (fence->ops->signaled && fence->ops->signaled(fence)) {
-		dma_fence_signal(fence);
+		spin_lock_irqsave(fence->lock, flags);
+		dma_fence_signal_internal(fence, ktime_get());
+		spin_unlock_irqrestore(fence->lock, flags);
 		return true;
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
  2025-08-12 14:34 ` [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL Christian König
@ 2025-08-13  8:16   ` Philipp Stanner
  2025-08-13  9:18     ` Christian König
  2025-08-13  8:20   ` Tvrtko Ursulin
  2025-08-20  7:27   ` kernel test robot
  2 siblings, 1 reply; 12+ messages in thread
From: Philipp Stanner @ 2025-08-13  8:16 UTC (permalink / raw)
  To: Christian König, simona.vetter, tvrtko.ursulin, airlied,
	dakr, sumit.semwal, dri-devel, linux-media

On Tue, 2025-08-12 at 16:34 +0200, Christian König wrote:
> From: Christian König <ckoenig@able.fritz.box>

Is this the correct mail addr? :)

> 
> We have the re-occurring problem that people try to invent a
> DMA-fences implementation which signals fences based on an userspace
> IOCTL.
> 
> This is well known as source of hard to track down crashes and is
> documented to be an invalid approach. The problem is that it seems
> to work during initial testing and only long term tests points out
> why this can never work correctly.
> 
> So give at least a warning when people try to signal a fence from
> task context and not from interrupts or a work item. This check is
> certainly not perfect but better than nothing.
> 
> Signed-off-by: Christian König <ckoenig@able.fritz.box>
> ---
>  drivers/dma-buf/dma-fence.c | 59 +++++++++++++++++++++++++++----------
>  include/linux/dma-fence.h   |  9 ++++--
>  2 files changed, 51 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 3f78c56b58dc..2bce620eacac 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -345,33 +345,23 @@ void __dma_fence_might_wait(void)
>  }
>  #endif
>  
> -
>  /**
> - * dma_fence_signal_timestamp_locked - signal completion of a fence
> + * dma_fence_signal_internal - internal signal completion of a fence
>   * @fence: the fence to signal
>   * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
>   *
> - * Signal completion for software callbacks on a fence, this will unblock
> - * dma_fence_wait() calls and run all the callbacks added with
> - * dma_fence_add_callback(). Can be called multiple times, but since a fence
> - * can only go from the unsignaled to the signaled state and not back, it will
> - * only be effective the first time. Set the timestamp provided as the fence
> - * signal timestamp.
> - *
> - * Unlike dma_fence_signal_timestamp(), this function must be called with
> - * &dma_fence.lock held.
> + * Internal signal the dma_fence without error checking. Should *NEVER* be used
> + * by drivers or external code directly.

s/Internal/Internally

>   *
>   * Returns 0 on success and a negative error value when @fence has been
>   * signalled already.
>   */
> -int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
> -				      ktime_t timestamp)
> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp)
>  {
>  	struct dma_fence_cb *cur, *tmp;
>  	struct list_head cb_list;
>  
>  	lockdep_assert_held(fence->lock);
> -
>  	if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>  				      &fence->flags)))
>  		return -EINVAL;
> @@ -390,7 +380,46 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
> +EXPORT_SYMBOL(dma_fence_signal_internal);

If it must only be used internally, can it be kept private, without
exporting the symbol?

> +
> +/**
> + * dma_fence_signal_timestamp_locked - signal completion of a fence
> + * @fence: the fence to signal
> + * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
> + *
> + * Signal completion for software callbacks on a fence, this will unblock
> + * dma_fence_wait() calls and run all the callbacks added with
> + * dma_fence_add_callback(). Can be called multiple times, but since a fence
> + * can only go from the unsignaled to the signaled state and not back, it will
> + * only be effective the first time. Set the timestamp provided as the fence
> + * signal timestamp.
> + *
> + * Unlike dma_fence_signal_timestamp(), this function must be called with
> + * &dma_fence.lock held.
> + *
> + * Returns 0 on success and a negative error value when @fence has been
> + * signalled already.
> + */
> +int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
> +				      ktime_t timestamp)
> +{
> +	/*
> +	 * We have the re-occurring problem that people try to invent a
> +	 * DMA-fences implementation which signals fences based on an userspace
> +	 * IOCTL.
> +	 *
> +	 * This is well known as source of hard to track down crashes and is
> +	 * documented to be an invalid approach. The problem is that it seems
> +	 * to work during initial testing and only long term tests points out
> +	 * why this can never work correctly.
> +	 *
> +	 * So give at least a warning when people try to signal a fence from
> +	 * task context and not from interrupts or a work item. This check is
> +	 * certainly not perfect but better than nothing.
> +	 */
> +	WARN_ON_ONCE(!in_interrupt() && !current_work());
> +	return dma_fence_signal_internal(fence, timestamp);
> +}

So this now is the point to decide what we want: do you want to *allow*
drivers to do it, or want to *prohibit* it?

If you want to prohibit it, then (additionally) returning an error code
here would make sense.


P.

>  
>  /**
>   * dma_fence_signal_timestamp - signal completion of a fence
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 64639e104110..8dbcd66989b8 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -369,6 +369,7 @@ int dma_fence_signal_locked(struct dma_fence *fence);
>  int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
>  int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>  				      ktime_t timestamp);
> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp);
>  signed long dma_fence_default_wait(struct dma_fence *fence,
>  				   bool intr, signed long timeout);
>  int dma_fence_add_callback(struct dma_fence *fence,
> @@ -422,7 +423,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>  		return true;
>  
>  	if (fence->ops->signaled && fence->ops->signaled(fence)) {
> -		dma_fence_signal_locked(fence);
> +		dma_fence_signal_internal(fence, ktime_get());
>  		return true;
>  	}
>  
> @@ -448,11 +449,15 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>  static inline bool
>  dma_fence_is_signaled(struct dma_fence *fence)
>  {
> +	unsigned long flags;
> +
>  	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>  		return true;
>  
>  	if (fence->ops->signaled && fence->ops->signaled(fence)) {
> -		dma_fence_signal(fence);
> +		spin_lock_irqsave(fence->lock, flags);
> +		dma_fence_signal_internal(fence, ktime_get());
> +		spin_unlock_irqrestore(fence->lock, flags);
>  		return true;
>  	}
>  


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
  2025-08-12 14:34 ` [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL Christian König
  2025-08-13  8:16   ` Philipp Stanner
@ 2025-08-13  8:20   ` Tvrtko Ursulin
  2025-08-13 11:10     ` Christian König
  2025-08-20  7:27   ` kernel test robot
  2 siblings, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2025-08-13  8:20 UTC (permalink / raw)
  To: Christian König, simona.vetter, phasta, airlied, dakr,
	sumit.semwal, dri-devel, linux-media


On 12/08/2025 15:34, Christian König wrote:
> From: Christian König <ckoenig@able.fritz.box>
> 
> We have the re-occurring problem that people try to invent a
> DMA-fences implementation which signals fences based on an userspace
> IOCTL.
> 
> This is well known as source of hard to track down crashes and is
> documented to be an invalid approach. The problem is that it seems
> to work during initial testing and only long term tests points out
> why this can never work correctly.
> 
> So give at least a warning when people try to signal a fence from
> task context and not from interrupts or a work item. This check is
> certainly not perfect but better than nothing.

I lack context as to why this should be disallowed so strongly (maybe 
cover letter is missing to better explain it all?), but at least if 
feels overly restrictive to for example exclude threads and thread workers.

Even the fact opportunistic signalling needs to bypass the assert makes 
it sound like there isn't anything fundamentally wrong with signalling 
from task context.

The first patch also feels a bit too much if it has no purpose apart 
from checking the new asserts would otherwise trigger.

Regards,

Tvrtko

> Signed-off-by: Christian König <ckoenig@able.fritz.box>
> ---
>   drivers/dma-buf/dma-fence.c | 59 +++++++++++++++++++++++++++----------
>   include/linux/dma-fence.h   |  9 ++++--
>   2 files changed, 51 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 3f78c56b58dc..2bce620eacac 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -345,33 +345,23 @@ void __dma_fence_might_wait(void)
>   }
>   #endif
>   
> -
>   /**
> - * dma_fence_signal_timestamp_locked - signal completion of a fence
> + * dma_fence_signal_internal - internal signal completion of a fence
>    * @fence: the fence to signal
>    * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
>    *
> - * Signal completion for software callbacks on a fence, this will unblock
> - * dma_fence_wait() calls and run all the callbacks added with
> - * dma_fence_add_callback(). Can be called multiple times, but since a fence
> - * can only go from the unsignaled to the signaled state and not back, it will
> - * only be effective the first time. Set the timestamp provided as the fence
> - * signal timestamp.
> - *
> - * Unlike dma_fence_signal_timestamp(), this function must be called with
> - * &dma_fence.lock held.
> + * Internal signal the dma_fence without error checking. Should *NEVER* be used
> + * by drivers or external code directly.
>    *
>    * Returns 0 on success and a negative error value when @fence has been
>    * signalled already.
>    */
> -int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
> -				      ktime_t timestamp)
> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp)
>   {
>   	struct dma_fence_cb *cur, *tmp;
>   	struct list_head cb_list;
>   
>   	lockdep_assert_held(fence->lock);
> -
>   	if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>   				      &fence->flags)))
>   		return -EINVAL;
> @@ -390,7 +380,46 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>   
>   	return 0;
>   }
> -EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
> +EXPORT_SYMBOL(dma_fence_signal_internal);
> +
> +/**
> + * dma_fence_signal_timestamp_locked - signal completion of a fence
> + * @fence: the fence to signal
> + * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
> + *
> + * Signal completion for software callbacks on a fence, this will unblock
> + * dma_fence_wait() calls and run all the callbacks added with
> + * dma_fence_add_callback(). Can be called multiple times, but since a fence
> + * can only go from the unsignaled to the signaled state and not back, it will
> + * only be effective the first time. Set the timestamp provided as the fence
> + * signal timestamp.
> + *
> + * Unlike dma_fence_signal_timestamp(), this function must be called with
> + * &dma_fence.lock held.
> + *
> + * Returns 0 on success and a negative error value when @fence has been
> + * signalled already.
> + */
> +int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
> +				      ktime_t timestamp)
> +{
> +	/*
> +	 * We have the re-occurring problem that people try to invent a
> +	 * DMA-fences implementation which signals fences based on an userspace
> +	 * IOCTL.
> +	 *
> +	 * This is well known as source of hard to track down crashes and is
> +	 * documented to be an invalid approach. The problem is that it seems
> +	 * to work during initial testing and only long term tests points out
> +	 * why this can never work correctly.
> +	 *
> +	 * So give at least a warning when people try to signal a fence from
> +	 * task context and not from interrupts or a work item. This check is
> +	 * certainly not perfect but better than nothing.
> +	 */
> +	WARN_ON_ONCE(!in_interrupt() && !current_work());
> +	return dma_fence_signal_internal(fence, timestamp);
> +}
>   
>   /**
>    * dma_fence_signal_timestamp - signal completion of a fence
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 64639e104110..8dbcd66989b8 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -369,6 +369,7 @@ int dma_fence_signal_locked(struct dma_fence *fence);
>   int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
>   int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>   				      ktime_t timestamp);
> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp);
>   signed long dma_fence_default_wait(struct dma_fence *fence,
>   				   bool intr, signed long timeout);
>   int dma_fence_add_callback(struct dma_fence *fence,
> @@ -422,7 +423,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>   		return true;
>   
>   	if (fence->ops->signaled && fence->ops->signaled(fence)) {
> -		dma_fence_signal_locked(fence);
> +		dma_fence_signal_internal(fence, ktime_get());
>   		return true;
>   	}
>   
> @@ -448,11 +449,15 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>   static inline bool
>   dma_fence_is_signaled(struct dma_fence *fence)
>   {
> +	unsigned long flags;
> +
>   	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>   		return true;
>   
>   	if (fence->ops->signaled && fence->ops->signaled(fence)) {
> -		dma_fence_signal(fence);
> +		spin_lock_irqsave(fence->lock, flags);
> +		dma_fence_signal_internal(fence, ktime_get());
> +		spin_unlock_irqrestore(fence->lock, flags);
>   		return true;
>   	}
>   


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
  2025-08-13  8:16   ` Philipp Stanner
@ 2025-08-13  9:18     ` Christian König
  0 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2025-08-13  9:18 UTC (permalink / raw)
  To: phasta, simona.vetter, tvrtko.ursulin, airlied, dakr,
	sumit.semwal, dri-devel, linux-media

On 13.08.25 10:16, Philipp Stanner wrote:
> On Tue, 2025-08-12 at 16:34 +0200, Christian König wrote:
>> From: Christian König <ckoenig@able.fritz.box>
> 
> Is this the correct mail addr? :)

^^ No it isn't and how the heck did that happen?

Looks like my gitconfig is somehow changed, but I have no idea why.

>>
>> We have the re-occurring problem that people try to invent a
>> DMA-fences implementation which signals fences based on an userspace
>> IOCTL.
>>
>> This is well known as source of hard to track down crashes and is
>> documented to be an invalid approach. The problem is that it seems
>> to work during initial testing and only long term tests points out
>> why this can never work correctly.
>>
>> So give at least a warning when people try to signal a fence from
>> task context and not from interrupts or a work item. This check is
>> certainly not perfect but better than nothing.
>>
>> Signed-off-by: Christian König <ckoenig@able.fritz.box>
>> ---
>>  drivers/dma-buf/dma-fence.c | 59 +++++++++++++++++++++++++++----------
>>  include/linux/dma-fence.h   |  9 ++++--
>>  2 files changed, 51 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>> index 3f78c56b58dc..2bce620eacac 100644
>> --- a/drivers/dma-buf/dma-fence.c
>> +++ b/drivers/dma-buf/dma-fence.c
>> @@ -345,33 +345,23 @@ void __dma_fence_might_wait(void)
>>  }
>>  #endif
>>  
>> -
>>  /**
>> - * dma_fence_signal_timestamp_locked - signal completion of a fence
>> + * dma_fence_signal_internal - internal signal completion of a fence
>>   * @fence: the fence to signal
>>   * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
>>   *
>> - * Signal completion for software callbacks on a fence, this will unblock
>> - * dma_fence_wait() calls and run all the callbacks added with
>> - * dma_fence_add_callback(). Can be called multiple times, but since a fence
>> - * can only go from the unsignaled to the signaled state and not back, it will
>> - * only be effective the first time. Set the timestamp provided as the fence
>> - * signal timestamp.
>> - *
>> - * Unlike dma_fence_signal_timestamp(), this function must be called with
>> - * &dma_fence.lock held.
>> + * Internal signal the dma_fence without error checking. Should *NEVER* be used
>> + * by drivers or external code directly.
> 
> s/Internal/Internally
> 
>>   *
>>   * Returns 0 on success and a negative error value when @fence has been
>>   * signalled already.
>>   */
>> -int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>> -				      ktime_t timestamp)
>> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp)
>>  {
>>  	struct dma_fence_cb *cur, *tmp;
>>  	struct list_head cb_list;
>>  
>>  	lockdep_assert_held(fence->lock);
>> -
>>  	if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>>  				      &fence->flags)))
>>  		return -EINVAL;
>> @@ -390,7 +380,46 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>  
>>  	return 0;
>>  }
>> -EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
>> +EXPORT_SYMBOL(dma_fence_signal_internal);
> 
> If it must only be used internally, can it be kept private, without
> exporting the symbol?

I have gone back and force about that as well.

We would then need to uninline dma_fence_is_signaled().

>> +
>> +/**
>> + * dma_fence_signal_timestamp_locked - signal completion of a fence
>> + * @fence: the fence to signal
>> + * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
>> + *
>> + * Signal completion for software callbacks on a fence, this will unblock
>> + * dma_fence_wait() calls and run all the callbacks added with
>> + * dma_fence_add_callback(). Can be called multiple times, but since a fence
>> + * can only go from the unsignaled to the signaled state and not back, it will
>> + * only be effective the first time. Set the timestamp provided as the fence
>> + * signal timestamp.
>> + *
>> + * Unlike dma_fence_signal_timestamp(), this function must be called with
>> + * &dma_fence.lock held.
>> + *
>> + * Returns 0 on success and a negative error value when @fence has been
>> + * signalled already.
>> + */
>> +int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>> +				      ktime_t timestamp)
>> +{
>> +	/*
>> +	 * We have the re-occurring problem that people try to invent a
>> +	 * DMA-fences implementation which signals fences based on an userspace
>> +	 * IOCTL.
>> +	 *
>> +	 * This is well known as source of hard to track down crashes and is
>> +	 * documented to be an invalid approach. The problem is that it seems
>> +	 * to work during initial testing and only long term tests points out
>> +	 * why this can never work correctly.
>> +	 *
>> +	 * So give at least a warning when people try to signal a fence from
>> +	 * task context and not from interrupts or a work item. This check is
>> +	 * certainly not perfect but better than nothing.
>> +	 */
>> +	WARN_ON_ONCE(!in_interrupt() && !current_work());
>> +	return dma_fence_signal_internal(fence, timestamp);
>> +}
> 
> So this now is the point to decide what we want: do you want to *allow*
> drivers to do it, or want to *prohibit* it?
> 
> If you want to prohibit it, then (additionally) returning an error code
> here would make sense.

I'm actually trying to remove the return value for quite a while now.

IIRC nobody is actually using it any more since it doesn't really signal an error condition.

Thanks,
Christian.

> 
> 
> P.
> 
>>  
>>  /**
>>   * dma_fence_signal_timestamp - signal completion of a fence
>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>> index 64639e104110..8dbcd66989b8 100644
>> --- a/include/linux/dma-fence.h
>> +++ b/include/linux/dma-fence.h
>> @@ -369,6 +369,7 @@ int dma_fence_signal_locked(struct dma_fence *fence);
>>  int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
>>  int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>  				      ktime_t timestamp);
>> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp);
>>  signed long dma_fence_default_wait(struct dma_fence *fence,
>>  				   bool intr, signed long timeout);
>>  int dma_fence_add_callback(struct dma_fence *fence,
>> @@ -422,7 +423,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>>  		return true;
>>  
>>  	if (fence->ops->signaled && fence->ops->signaled(fence)) {
>> -		dma_fence_signal_locked(fence);
>> +		dma_fence_signal_internal(fence, ktime_get());
>>  		return true;
>>  	}
>>  
>> @@ -448,11 +449,15 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>>  static inline bool
>>  dma_fence_is_signaled(struct dma_fence *fence)
>>  {
>> +	unsigned long flags;
>> +
>>  	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>>  		return true;
>>  
>>  	if (fence->ops->signaled && fence->ops->signaled(fence)) {
>> -		dma_fence_signal(fence);
>> +		spin_lock_irqsave(fence->lock, flags);
>> +		dma_fence_signal_internal(fence, ktime_get());
>> +		spin_unlock_irqrestore(fence->lock, flags);
>>  		return true;
>>  	}
>>  
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
  2025-08-13  8:20   ` Tvrtko Ursulin
@ 2025-08-13 11:10     ` Christian König
  2025-08-13 11:59       ` Tvrtko Ursulin
  0 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2025-08-13 11:10 UTC (permalink / raw)
  To: Tvrtko Ursulin, simona.vetter, phasta, airlied, dakr,
	sumit.semwal, dri-devel, linux-media

On 13.08.25 10:20, Tvrtko Ursulin wrote:
> 
> On 12/08/2025 15:34, Christian König wrote:
>> From: Christian König <ckoenig@able.fritz.box>
>>
>> We have the re-occurring problem that people try to invent a
>> DMA-fences implementation which signals fences based on an userspace
>> IOCTL.
>>
>> This is well known as source of hard to track down crashes and is
>> documented to be an invalid approach. The problem is that it seems
>> to work during initial testing and only long term tests points out
>> why this can never work correctly.
>>
>> So give at least a warning when people try to signal a fence from
>> task context and not from interrupts or a work item. This check is
>> certainly not perfect but better than nothing.
> 
> I lack context as to why this should be disallowed so strongly (maybe cover letter is missing to better explain it all?)

See here https://www.kernel.org/doc/html/latest/driver-api/dma-buf.html#indefinite-dma-fences

I was hoping that this problem is so well known by now that it doesn't need more explanation.

Going to expand the commit message a bit.

>, but at least if feels overly restrictive to for example exclude threads and thread workers.

Good point. Could be that someone is using a pure kernel thread for fence signaling. Going to check for that instead of a worker.

> 
> Even the fact opportunistic signalling needs to bypass the assert makes it sound like there isn't anything fundamentally wrong with signalling from task context.
> 

Opportunistic signaling can happen from everywhere. But when an implementation tries to signal it from an IOCTL that is certainly invalid.

> The first patch also feels a bit too much if it has no purpose apart from checking the new asserts would otherwise trigger.

The sw_sync code is is only used for testing and debugging. See the Kconfig of it:

          A sync object driver that uses a 32bit counter to coordinate
          synchronization.  Useful when there is no hardware primitive backing
          the synchronization.

          WARNING: improper use of this can result in deadlocking kernel
          drivers from userspace. Intended for test and debug only.

Thanks,
Christian.

> 
> Regards,
> 
> Tvrtko
> 
>> Signed-off-by: Christian König <ckoenig@able.fritz.box>
>> ---
>>   drivers/dma-buf/dma-fence.c | 59 +++++++++++++++++++++++++++----------
>>   include/linux/dma-fence.h   |  9 ++++--
>>   2 files changed, 51 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>> index 3f78c56b58dc..2bce620eacac 100644
>> --- a/drivers/dma-buf/dma-fence.c
>> +++ b/drivers/dma-buf/dma-fence.c
>> @@ -345,33 +345,23 @@ void __dma_fence_might_wait(void)
>>   }
>>   #endif
>>   -
>>   /**
>> - * dma_fence_signal_timestamp_locked - signal completion of a fence
>> + * dma_fence_signal_internal - internal signal completion of a fence
>>    * @fence: the fence to signal
>>    * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
>>    *
>> - * Signal completion for software callbacks on a fence, this will unblock
>> - * dma_fence_wait() calls and run all the callbacks added with
>> - * dma_fence_add_callback(). Can be called multiple times, but since a fence
>> - * can only go from the unsignaled to the signaled state and not back, it will
>> - * only be effective the first time. Set the timestamp provided as the fence
>> - * signal timestamp.
>> - *
>> - * Unlike dma_fence_signal_timestamp(), this function must be called with
>> - * &dma_fence.lock held.
>> + * Internal signal the dma_fence without error checking. Should *NEVER* be used
>> + * by drivers or external code directly.
>>    *
>>    * Returns 0 on success and a negative error value when @fence has been
>>    * signalled already.
>>    */
>> -int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>> -                      ktime_t timestamp)
>> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp)
>>   {
>>       struct dma_fence_cb *cur, *tmp;
>>       struct list_head cb_list;
>>         lockdep_assert_held(fence->lock);
>> -
>>       if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>>                         &fence->flags)))
>>           return -EINVAL;
>> @@ -390,7 +380,46 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>         return 0;
>>   }
>> -EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
>> +EXPORT_SYMBOL(dma_fence_signal_internal);
>> +
>> +/**
>> + * dma_fence_signal_timestamp_locked - signal completion of a fence
>> + * @fence: the fence to signal
>> + * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
>> + *
>> + * Signal completion for software callbacks on a fence, this will unblock
>> + * dma_fence_wait() calls and run all the callbacks added with
>> + * dma_fence_add_callback(). Can be called multiple times, but since a fence
>> + * can only go from the unsignaled to the signaled state and not back, it will
>> + * only be effective the first time. Set the timestamp provided as the fence
>> + * signal timestamp.
>> + *
>> + * Unlike dma_fence_signal_timestamp(), this function must be called with
>> + * &dma_fence.lock held.
>> + *
>> + * Returns 0 on success and a negative error value when @fence has been
>> + * signalled already.
>> + */
>> +int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>> +                      ktime_t timestamp)
>> +{
>> +    /*
>> +     * We have the re-occurring problem that people try to invent a
>> +     * DMA-fences implementation which signals fences based on an userspace
>> +     * IOCTL.
>> +     *
>> +     * This is well known as source of hard to track down crashes and is
>> +     * documented to be an invalid approach. The problem is that it seems
>> +     * to work during initial testing and only long term tests points out
>> +     * why this can never work correctly.
>> +     *
>> +     * So give at least a warning when people try to signal a fence from
>> +     * task context and not from interrupts or a work item. This check is
>> +     * certainly not perfect but better than nothing.
>> +     */
>> +    WARN_ON_ONCE(!in_interrupt() && !current_work());
>> +    return dma_fence_signal_internal(fence, timestamp);
>> +}
>>     /**
>>    * dma_fence_signal_timestamp - signal completion of a fence
>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>> index 64639e104110..8dbcd66989b8 100644
>> --- a/include/linux/dma-fence.h
>> +++ b/include/linux/dma-fence.h
>> @@ -369,6 +369,7 @@ int dma_fence_signal_locked(struct dma_fence *fence);
>>   int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
>>   int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>                         ktime_t timestamp);
>> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp);
>>   signed long dma_fence_default_wait(struct dma_fence *fence,
>>                      bool intr, signed long timeout);
>>   int dma_fence_add_callback(struct dma_fence *fence,
>> @@ -422,7 +423,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>>           return true;
>>         if (fence->ops->signaled && fence->ops->signaled(fence)) {
>> -        dma_fence_signal_locked(fence);
>> +        dma_fence_signal_internal(fence, ktime_get());
>>           return true;
>>       }
>>   @@ -448,11 +449,15 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>>   static inline bool
>>   dma_fence_is_signaled(struct dma_fence *fence)
>>   {
>> +    unsigned long flags;
>> +
>>       if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>>           return true;
>>         if (fence->ops->signaled && fence->ops->signaled(fence)) {
>> -        dma_fence_signal(fence);
>> +        spin_lock_irqsave(fence->lock, flags);
>> +        dma_fence_signal_internal(fence, ktime_get());
>> +        spin_unlock_irqrestore(fence->lock, flags);
>>           return true;
>>       }
>>   
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
  2025-08-13 11:10     ` Christian König
@ 2025-08-13 11:59       ` Tvrtko Ursulin
  2025-08-21 12:46         ` Christian König
  0 siblings, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2025-08-13 11:59 UTC (permalink / raw)
  To: Christian König, simona.vetter, phasta, airlied, dakr,
	sumit.semwal, dri-devel, linux-media


On 13/08/2025 12:10, Christian König wrote:
> On 13.08.25 10:20, Tvrtko Ursulin wrote:
>>
>> On 12/08/2025 15:34, Christian König wrote:
>>> From: Christian König <ckoenig@able.fritz.box>
>>>
>>> We have the re-occurring problem that people try to invent a
>>> DMA-fences implementation which signals fences based on an userspace
>>> IOCTL.
>>>
>>> This is well known as source of hard to track down crashes and is
>>> documented to be an invalid approach. The problem is that it seems
>>> to work during initial testing and only long term tests points out
>>> why this can never work correctly.
>>>
>>> So give at least a warning when people try to signal a fence from
>>> task context and not from interrupts or a work item. This check is
>>> certainly not perfect but better than nothing.
>>
>> I lack context as to why this should be disallowed so strongly (maybe cover letter is missing to better explain it all?)
> 
> See here https://www.kernel.org/doc/html/latest/driver-api/dma-buf.html#indefinite-dma-fences
> 
> I was hoping that this problem is so well known by now that it doesn't need more explanation.
> 
> Going to expand the commit message a bit.

I probably got thrown by the mention of crashes. But yes, expanding the 
commit and clarifying it is about deadlocks and indefinite fences, or 
adding the cover letter would be better thanks!

>> , but at least if feels overly restrictive to for example exclude threads and thread workers.
> 
> Good point. Could be that someone is using a pure kernel thread for fence signaling. Going to check for that instead of a worker.

Ok, I am curious how to do it reliably. Non-null current and PF_KTHREAD 
and PF_WQ_WORKER not set?

>> Even the fact opportunistic signalling needs to bypass the assert makes it sound like there isn't anything fundamentally wrong with signalling from task context.
>>
> 
> Opportunistic signaling can happen from everywhere. But when an implementation tries to signal it from an IOCTL that is certainly invalid.
> 
>> The first patch also feels a bit too much if it has no purpose apart from checking the new asserts would otherwise trigger.
> 
> The sw_sync code is is only used for testing and debugging. See the Kconfig of it:
> 
>            A sync object driver that uses a 32bit counter to coordinate
>            synchronization.  Useful when there is no hardware primitive backing
>            the synchronization.
> 
>            WARNING: improper use of this can result in deadlocking kernel
>            drivers from userspace. Intended for test and debug only.
> 

But it is adding kernel code for a questionable benefit.

What about calling the non-asserting version instead of adding 
complexity? It is kernel code so should be fine.

Because even with the worker sw_sync can still create infinite fences. 
Worker just defeats the asserts so I do not see the value in 
complicating it.

Regards,

Tvrtko

>>
>>> Signed-off-by: Christian König <ckoenig@able.fritz.box>
>>> ---
>>>    drivers/dma-buf/dma-fence.c | 59 +++++++++++++++++++++++++++----------
>>>    include/linux/dma-fence.h   |  9 ++++--
>>>    2 files changed, 51 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>> index 3f78c56b58dc..2bce620eacac 100644
>>> --- a/drivers/dma-buf/dma-fence.c
>>> +++ b/drivers/dma-buf/dma-fence.c
>>> @@ -345,33 +345,23 @@ void __dma_fence_might_wait(void)
>>>    }
>>>    #endif
>>>    -
>>>    /**
>>> - * dma_fence_signal_timestamp_locked - signal completion of a fence
>>> + * dma_fence_signal_internal - internal signal completion of a fence
>>>     * @fence: the fence to signal
>>>     * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
>>>     *
>>> - * Signal completion for software callbacks on a fence, this will unblock
>>> - * dma_fence_wait() calls and run all the callbacks added with
>>> - * dma_fence_add_callback(). Can be called multiple times, but since a fence
>>> - * can only go from the unsignaled to the signaled state and not back, it will
>>> - * only be effective the first time. Set the timestamp provided as the fence
>>> - * signal timestamp.
>>> - *
>>> - * Unlike dma_fence_signal_timestamp(), this function must be called with
>>> - * &dma_fence.lock held.
>>> + * Internal signal the dma_fence without error checking. Should *NEVER* be used
>>> + * by drivers or external code directly.
>>>     *
>>>     * Returns 0 on success and a negative error value when @fence has been
>>>     * signalled already.
>>>     */
>>> -int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>> -                      ktime_t timestamp)
>>> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp)
>>>    {
>>>        struct dma_fence_cb *cur, *tmp;
>>>        struct list_head cb_list;
>>>          lockdep_assert_held(fence->lock);
>>> -
>>>        if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>>>                          &fence->flags)))
>>>            return -EINVAL;
>>> @@ -390,7 +380,46 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>>          return 0;
>>>    }
>>> -EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
>>> +EXPORT_SYMBOL(dma_fence_signal_internal);
>>> +
>>> +/**
>>> + * dma_fence_signal_timestamp_locked - signal completion of a fence
>>> + * @fence: the fence to signal
>>> + * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
>>> + *
>>> + * Signal completion for software callbacks on a fence, this will unblock
>>> + * dma_fence_wait() calls and run all the callbacks added with
>>> + * dma_fence_add_callback(). Can be called multiple times, but since a fence
>>> + * can only go from the unsignaled to the signaled state and not back, it will
>>> + * only be effective the first time. Set the timestamp provided as the fence
>>> + * signal timestamp.
>>> + *
>>> + * Unlike dma_fence_signal_timestamp(), this function must be called with
>>> + * &dma_fence.lock held.
>>> + *
>>> + * Returns 0 on success and a negative error value when @fence has been
>>> + * signalled already.
>>> + */
>>> +int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>> +                      ktime_t timestamp)
>>> +{
>>> +    /*
>>> +     * We have the re-occurring problem that people try to invent a
>>> +     * DMA-fences implementation which signals fences based on an userspace
>>> +     * IOCTL.
>>> +     *
>>> +     * This is well known as source of hard to track down crashes and is
>>> +     * documented to be an invalid approach. The problem is that it seems
>>> +     * to work during initial testing and only long term tests points out
>>> +     * why this can never work correctly.
>>> +     *
>>> +     * So give at least a warning when people try to signal a fence from
>>> +     * task context and not from interrupts or a work item. This check is
>>> +     * certainly not perfect but better than nothing.
>>> +     */
>>> +    WARN_ON_ONCE(!in_interrupt() && !current_work());
>>> +    return dma_fence_signal_internal(fence, timestamp);
>>> +}
>>>      /**
>>>     * dma_fence_signal_timestamp - signal completion of a fence
>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>> index 64639e104110..8dbcd66989b8 100644
>>> --- a/include/linux/dma-fence.h
>>> +++ b/include/linux/dma-fence.h
>>> @@ -369,6 +369,7 @@ int dma_fence_signal_locked(struct dma_fence *fence);
>>>    int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
>>>    int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>>                          ktime_t timestamp);
>>> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp);
>>>    signed long dma_fence_default_wait(struct dma_fence *fence,
>>>                       bool intr, signed long timeout);
>>>    int dma_fence_add_callback(struct dma_fence *fence,
>>> @@ -422,7 +423,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>>>            return true;
>>>          if (fence->ops->signaled && fence->ops->signaled(fence)) {
>>> -        dma_fence_signal_locked(fence);
>>> +        dma_fence_signal_internal(fence, ktime_get());
>>>            return true;
>>>        }
>>>    @@ -448,11 +449,15 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>>>    static inline bool
>>>    dma_fence_is_signaled(struct dma_fence *fence)
>>>    {
>>> +    unsigned long flags;
>>> +
>>>        if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>>>            return true;
>>>          if (fence->ops->signaled && fence->ops->signaled(fence)) {
>>> -        dma_fence_signal(fence);
>>> +        spin_lock_irqsave(fence->lock, flags);
>>> +        dma_fence_signal_internal(fence, ktime_get());
>>> +        spin_unlock_irqrestore(fence->lock, flags);
>>>            return true;
>>>        }
>>>    
>>
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
  2025-08-12 14:34 ` [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL Christian König
  2025-08-13  8:16   ` Philipp Stanner
  2025-08-13  8:20   ` Tvrtko Ursulin
@ 2025-08-20  7:27   ` kernel test robot
  2025-08-21 13:00     ` Christian König
  2 siblings, 1 reply; 12+ messages in thread
From: kernel test robot @ 2025-08-20  7:27 UTC (permalink / raw)
  To: Christian König
  Cc: oe-lkp, lkp, linux-media, dri-devel, linaro-mm-sig, ltp,
	simona.vetter, tvrtko.ursulin, phasta, airlied, dakr,
	sumit.semwal, oliver.sang



Hello,

kernel test robot noticed "WARNING:at_drivers/dma-buf/dma-fence.c:#dma_fence_signal" on:

commit: 409db68e04bdf052bc03f620e70339764b598ade ("[PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL")
url: https://github.com/intel-lab-lkp/linux/commits/Christian-K-nig/dma-buf-add-warning-when-dma_fence-is-signaled-from-IOCTL/20250812-223543
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 53e760d8949895390e256e723e7ee46618310361
patch link: https://lore.kernel.org/all/20250812143402.8619-2-christian.koenig@amd.com/
patch subject: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL

in testcase: ltp
version: ltp-x86_64-9f512c1d8-1_20250809
with following parameters:

	test: syscalls-ipc-msgstress



config: x86_64-rhel-9.4-ltp
compiler: gcc-12
test machine: 8 threads 1 sockets Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz (Haswell) with 16G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)


If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202508200843.8b006132-lkp@intel.com


[   51.636268][  T218] ------------[ cut here ]------------
[ 51.636273][ T218] WARNING: CPU: 3 PID: 218 at drivers/dma-buf/dma-fence.c:420 dma_fence_signal (drivers/dma-buf/dma-fence.c:420 drivers/dma-buf/dma-fence.c:502) 
[   51.636292][  T218] Modules linked in: coretemp sd_mod snd_hda_codec_realtek_lib snd_hda_codec_generic sg ipmi_devintf kvm_intel snd_hda_intel ipmi_msghandler platform_profile i915(+) kvm snd_hda_codec intel_gtt dell_wmi snd_hda_core drm_buddy binfmt_misc dell_smbios snd_intel_dspcfg ttm dell_wmi_descriptor snd_intel_sdw_acpi snd_hwdep mei_wdt sparse_keymap irqbypass drm_display_helper ahci ghash_clmulni_intel snd_pcm libahci rfkill cec mei_me rapl intel_cstate dcdbas snd_timer drm_client_lib libata intel_uncore mei snd drm_kms_helper i2c_i801 i2c_smbus pcspkr lpc_ich soundcore video wmi fuse drm loop dm_mod
[   51.636385][  T218] CPU: 3 UID: 0 PID: 218 Comm: (udev-worker) Not tainted 6.17.0-rc1-00006-g409db68e04bd #1 PREEMPT(voluntary)
[   51.636395][  T218] Hardware name: Dell Inc. OptiPlex 9020/0DNKMN, BIOS A05 12/05/2013
[ 51.636399][ T218] RIP: 0010:dma_fence_signal (drivers/dma-buf/dma-fence.c:420 drivers/dma-buf/dma-fence.c:502) 
[ 51.636415][ T218] Code: 00 fc ff df 80 3c 02 00 75 36 48 8b 3b 4c 89 e6 e8 10 33 27 01 89 e8 5b 5d 41 5c c3 cc cc cc cc e8 b0 2e 77 fe 48 85 c0 75 bc <0f> 0b eb b8 0f 0b bd ea ff ff ff 5b 89 e8 5d 41 5c c3 cc cc cc cc
All code
========
   0:	00 fc                	add    %bh,%ah
   2:	ff                   	(bad)
   3:	df 80 3c 02 00 75    	filds  0x7500023c(%rax)
   9:	36 48 8b 3b          	ss mov (%rbx),%rdi
   d:	4c 89 e6             	mov    %r12,%rsi
  10:	e8 10 33 27 01       	call   0x1273325
  15:	89 e8                	mov    %ebp,%eax
  17:	5b                   	pop    %rbx
  18:	5d                   	pop    %rbp
  19:	41 5c                	pop    %r12
  1b:	c3                   	ret
  1c:	cc                   	int3
  1d:	cc                   	int3
  1e:	cc                   	int3
  1f:	cc                   	int3
  20:	e8 b0 2e 77 fe       	call   0xfffffffffe772ed5
  25:	48 85 c0             	test   %rax,%rax
  28:	75 bc                	jne    0xffffffffffffffe6
  2a:*	0f 0b                	ud2		<-- trapping instruction
  2c:	eb b8                	jmp    0xffffffffffffffe6
  2e:	0f 0b                	ud2
  30:	bd ea ff ff ff       	mov    $0xffffffea,%ebp
  35:	5b                   	pop    %rbx
  36:	89 e8                	mov    %ebp,%eax
  38:	5d                   	pop    %rbp
  39:	41 5c                	pop    %r12
  3b:	c3                   	ret
  3c:	cc                   	int3
  3d:	cc                   	int3
  3e:	cc                   	int3
  3f:	cc                   	int3

Code starting with the faulting instruction
===========================================
   0:	0f 0b                	ud2
   2:	eb b8                	jmp    0xffffffffffffffbc
   4:	0f 0b                	ud2
   6:	bd ea ff ff ff       	mov    $0xffffffea,%ebp
   b:	5b                   	pop    %rbx
   c:	89 e8                	mov    %ebp,%eax
   e:	5d                   	pop    %rbp
   f:	41 5c                	pop    %r12
  11:	c3                   	ret
  12:	cc                   	int3
  13:	cc                   	int3
  14:	cc                   	int3
  15:	cc                   	int3
[   51.636420][  T218] RSP: 0018:ffffc90000a9ed30 EFLAGS: 00010046
[   51.636428][  T218] RAX: 0000000000000000 RBX: ffff88811750fc00 RCX: 0000000000000018
[   51.636437][  T218] RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffff88810691512c
[   51.636440][  T218] RBP: 0000000be56b1408 R08: 0000000000000001 R09: fffff52000153d9a
[   51.636445][  T218] R10: 0000000000000003 R11: ffff888108145000 R12: 0000000000000246
[   51.636452][  T218] R13: ffffffffc1c9b060 R14: ffff88810406ba0c R15: 1ffff92000153dc2
[   51.636455][  T218] FS:  00007efd90c038c0(0000) GS:ffff8883e4077000(0000) knlGS:0000000000000000
[   51.636459][  T218] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   51.636462][  T218] CR2: 00007f5bd8238c20 CR3: 000000040ed8a005 CR4: 00000000001726f0
[   51.636466][  T218] Call Trace:
[   51.636469][  T218]  <TASK>
[ 51.636477][ T218] fence_work (include/linux/dma-fence.h:272 drivers/gpu/drm/i915/i915_sw_fence_work.c:23) i915 
[ 51.637304][ T218] fence_notify (drivers/gpu/drm/i915/i915_sw_fence_work.c:39) i915 
[ 51.637827][ T218] __i915_sw_fence_complete (drivers/gpu/drm/i915/i915_sw_fence.c:201) i915 
[ 51.638300][ T218] i915_vma_pin_ww (drivers/gpu/drm/i915/i915_vma.c:1601) i915 
[ 51.638763][ T218] ? __pfx_i915_vma_pin_ww (drivers/gpu/drm/i915/i915_vma.c:1434) i915 
[ 51.639218][ T218] ? i915_gem_object_make_unshrinkable (include/linux/list.h:215 include/linux/list.h:287 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:500) i915 
[ 51.639648][ T218] ? i915_vma_make_unshrinkable (drivers/gpu/drm/i915/i915_vma.c:2292) i915 
[ 51.640091][ T218] ? intel_ring_pin (drivers/gpu/drm/i915/gt/intel_ring.c:73) i915 
[ 51.640505][ T218] intel_ring_submission_setup (drivers/gpu/drm/i915/gt/intel_ring_submission.c:1290 drivers/gpu/drm/i915/gt/intel_ring_submission.c:1421) i915 
[ 51.640918][ T218] ? __pfx_intel_ring_submission_setup (drivers/gpu/drm/i915/gt/intel_ring_submission.c:1349) i915 
[   51.641232][   T65] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
[ 51.641321][ T218] ? intel_engine_init_whitelist (drivers/gpu/drm/i915/gt/intel_workarounds.c:2104) i915 
[ 51.641735][ T218] ? __intel_wakeref_init (arch/x86/include/asm/atomic.h:28 include/linux/atomic/atomic-arch-fallback.h:503 include/linux/atomic/atomic-instrumented.h:68 drivers/gpu/drm/i915/intel_wakeref.c:109) i915 
[ 51.642126][ T218] intel_engines_init (drivers/gpu/drm/i915/gt/intel_engine_cs.c:1514) i915 
[ 51.642521][ T218] ? i915_gem_object_make_unshrinkable (arch/x86/include/asm/atomic.h:93 include/linux/atomic/atomic-arch-fallback.h:667 include/linux/atomic/atomic-arch-fallback.h:1119 include/linux/atomic/atomic-instrumented.h:524 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:498) i915 
[ 51.642929][ T218] ? __pfx_intel_ring_submission_setup (drivers/gpu/drm/i915/gt/intel_ring_submission.c:1349) i915 
[ 51.643331][ T218] intel_gt_init (drivers/gpu/drm/i915/gt/intel_gt.c:719) i915 
[ 51.643728][ T218] i915_gem_init (drivers/gpu/drm/i915/i915_gem.c:1191) i915 
[ 51.644140][ T218] i915_driver_probe (drivers/gpu/drm/i915/i915_driver.c:831) i915 
[ 51.644524][ T218] ? __pfx_i915_driver_probe (drivers/gpu/drm/i915/i915_driver.c:780) i915 
[ 51.644903][ T218] ? drm_privacy_screen_get (drivers/gpu/drm/drm_privacy_screen.c:169) drm 
[ 51.645047][ T218] ? intel_display_driver_probe_defer (drivers/gpu/drm/i915/display/intel_display_driver.c:84) i915 
[ 51.645483][ T218] ? i915_pci_probe (drivers/gpu/drm/i915/i915_pci.c:995) i915 
[ 51.645876][ T218] ? __pfx_i915_pci_probe (drivers/gpu/drm/i915/i915_pci.c:956) i915 
[ 51.646267][ T218] local_pci_probe (drivers/pci/pci-driver.c:324) 
[ 51.646283][ T218] pci_call_probe (drivers/pci/pci-driver.c:392) 
[ 51.646295][ T218] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:107 include/linux/atomic/atomic-arch-fallback.h:2170 include/linux/atomic/atomic-instrumented.h:1302 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:187 include/linux/spinlock_api_smp.h:134 kernel/locking/spinlock.c:154) 
[ 51.646308][ T218] ? __pfx_pci_call_probe (drivers/pci/pci-driver.c:352) 
[ 51.646321][ T218] ? kernfs_add_one (fs/kernfs/dir.c:834) 
[ 51.646337][ T218] ? pci_assign_irq (drivers/pci/irq.c:149) 
[ 51.646350][ T218] ? pci_match_device (drivers/pci/pci-driver.c:159 (discriminator 1)) 
[ 51.646362][ T218] ? kernfs_put (arch/x86/include/asm/atomic.h:67 (discriminator 1) include/linux/atomic/atomic-arch-fallback.h:2278 (discriminator 1) include/linux/atomic/atomic-instrumented.h:1384 (discriminator 1) fs/kernfs/dir.c:569 (discriminator 1)) 
[ 51.646368][ T218] pci_device_probe (drivers/pci/pci-driver.c:452) 
[ 51.646377][ T218] really_probe (drivers/base/dd.c:581 drivers/base/dd.c:659) 
[ 51.646391][ T218] __driver_probe_device (drivers/base/dd.c:801) 
[ 51.646404][ T218] driver_probe_device (drivers/base/dd.c:831) 
[ 51.646416][ T218] __driver_attach (drivers/base/dd.c:1218) 
[ 51.646424][ T218] ? __pfx___driver_attach (drivers/base/dd.c:1158) 
[ 51.646428][ T218] bus_for_each_dev (drivers/base/bus.c:369) 
[ 51.646441][ T218] ? __pfx_bus_for_each_dev (drivers/base/bus.c:358) 
[ 51.646444][ T218] ? __kmalloc_cache_noprof (arch/x86/include/asm/jump_label.h:46 include/linux/memcontrol.h:1714 mm/slub.c:2210 mm/slub.c:4190 mm/slub.c:4229 mm/slub.c:4391) 
[ 51.646456][ T218] ? klist_add_tail (include/linux/list.h:150 include/linux/list.h:183 lib/klist.c:104 lib/klist.c:137) 
[ 51.646468][ T218] bus_add_driver (drivers/base/bus.c:678) 
[ 51.646482][ T218] driver_register (drivers/base/driver.c:249) 
[ 51.646490][ T218] i915_init (drivers/gpu/drm/i915/i915_driver.c:1428) i915 
[ 51.646891][ T218] ? __pfx_i915_init (drivers/gpu/drm/i915/i915_config.c:13) i915 
[   51.647101][   T67] sd 2:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
[ 51.647277][ T218] do_one_initcall (init/main.c:1269) 
[ 51.647292][ T218] ? kfree (mm/slub.c:4680 mm/slub.c:4879) 
[ 51.647304][ T218] ? __pfx_do_one_initcall (init/main.c:1260) 
[ 51.647315][ T218] ? kasan_unpoison (mm/kasan/shadow.c:156 mm/kasan/shadow.c:182) 
[ 51.647327][ T218] ? __kasan_slab_alloc (mm/kasan/common.c:329 mm/kasan/common.c:356) 
[ 51.647340][ T218] ? __kmalloc_cache_noprof (mm/slub.c:4180 mm/slub.c:4229 mm/slub.c:4391) 
[ 51.647352][ T218] ? kasan_save_track (arch/x86/include/asm/current.h:25 mm/kasan/common.c:60 mm/kasan/common.c:69) 
[ 51.647365][ T218] ? kasan_unpoison (mm/kasan/shadow.c:156 mm/kasan/shadow.c:182) 
[ 51.647377][ T218] do_init_module (kernel/module/main.c:3039) 
[ 51.647388][ T218] ? __pfx_do_init_module (kernel/module/main.c:3011) 
[ 51.647402][ T218] ? kfree (mm/slub.c:4680 mm/slub.c:4879) 
[ 51.647414][ T218] ? klp_module_coming (kernel/livepatch/core.c:1317) 
[ 51.647426][ T218] ? load_module (kernel/module/main.c:2468 kernel/module/main.c:2463 kernel/module/main.c:3504) 
[ 51.647441][ T218] load_module (kernel/module/main.c:3509) 
[ 51.647449][ T218] ? ima_post_read_file (security/integrity/ima/ima_main.c:896 security/integrity/ima/ima_main.c:878) 
[ 51.647466][ T218] ? __pfx_load_module (kernel/module/main.c:3353) 
[ 51.647478][ T218] ? __pfx_kernel_read_file (fs/kernel_read_file.c:38) 
[ 51.647489][ T218] ? init_module_from_file (kernel/module/main.c:3701) 
[ 51.647499][ T218] init_module_from_file (kernel/module/main.c:3701) 
[ 51.647514][ T218] ? __pfx_init_module_from_file (kernel/module/main.c:3677) 
[ 51.647525][ T218] ? mm_get_unmapped_area (arch/x86/include/asm/bitops.h:206 arch/x86/include/asm/bitops.h:238 include/asm-generic/bitops/instrumented-non-atomic.h:142 mm/mmap.c:805 mm/mmap.c:871) 
[ 51.647540][ T218] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:107 include/linux/atomic/atomic-arch-fallback.h:2170 include/linux/atomic/atomic-instrumented.h:1302 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:187 include/linux/spinlock_api_smp.h:134 kernel/locking/spinlock.c:154) 
[ 51.647547][ T218] ? __pfx__raw_spin_lock (kernel/locking/spinlock.c:153) 
[ 51.647560][ T218] idempotent_init_module (kernel/module/main.c:3713) 
[ 51.647573][ T218] ? __pfx_idempotent_init_module (kernel/module/main.c:3705) 
[ 51.647582][ T218] ? __pfx___seccomp_filter (kernel/seccomp.c:1244) 
[ 51.647590][ T218] ? fdget (include/linux/atomic/atomic-arch-fallback.h:479 include/linux/atomic/atomic-instrumented.h:50 fs/file.c:1167 fs/file.c:1181) 
[ 51.647607][ T218] ? security_capable (security/security.c:1142) 
[ 51.647615][ T218] __x64_sys_finit_module (include/linux/file.h:62 include/linux/file.h:83 kernel/module/main.c:3736 kernel/module/main.c:3723 kernel/module/main.c:3723) 
[ 51.647627][ T218] ? syscall_trace_enter (kernel/entry/syscall-common.c:44) 
[ 51.647640][ T218] do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94) 
[ 51.647657][ T218] ? fput (arch/x86/include/asm/atomic64_64.h:79 include/linux/atomic/atomic-arch-fallback.h:2913 include/linux/atomic/atomic-arch-fallback.h:3364 include/linux/atomic/atomic-long.h:698 include/linux/atomic/atomic-instrumented.h:3767 include/linux/file_ref.h:157 fs/file_table.c:544) 
[ 51.647668][ T218] ? ksys_mmap_pgoff (mm/mmap.c:609) 
[ 51.647682][ T218] ? do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94) 
[ 51.647694][ T218] ? from_kgid_munged (kernel/user_namespace.c:535) 
[ 51.647708][ T218] ? _copy_to_user (arch/x86/include/asm/uaccess_64.h:126 arch/x86/include/asm/uaccess_64.h:147 include/linux/uaccess.h:197 lib/usercopy.c:26) 
[ 51.647722][ T218] ? cp_new_stat (fs/stat.c:471) 
[ 51.647732][ T218] ? __pfx_cp_new_stat (fs/stat.c:471) 


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250820/202508200843.8b006132-lkp@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
  2025-08-13 11:59       ` Tvrtko Ursulin
@ 2025-08-21 12:46         ` Christian König
  2025-08-22  8:08           ` Tvrtko Ursulin
  0 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2025-08-21 12:46 UTC (permalink / raw)
  To: Tvrtko Ursulin, simona.vetter, phasta, airlied, dakr,
	sumit.semwal, dri-devel, linux-media

On 13.08.25 13:59, Tvrtko Ursulin wrote:
>> Good point. Could be that someone is using a pure kernel thread for fence signaling. Going to check for that instead of a worker.
> 
> Ok, I am curious how to do it reliably. Non-null current and PF_KTHREAD and PF_WQ_WORKER not set?

If I'm not completely mistaken (current->flags & PF_KTHREAD) should do it, but I need to double check that as well.

>>> Even the fact opportunistic signalling needs to bypass the assert makes it sound like there isn't anything fundamentally wrong with signalling from task context.
>>>
>>
>> Opportunistic signaling can happen from everywhere. But when an implementation tries to signal it from an IOCTL that is certainly invalid.
>>
>>> The first patch also feels a bit too much if it has no purpose apart from checking the new asserts would otherwise trigger.
>>
>> The sw_sync code is is only used for testing and debugging. See the Kconfig of it:
>>
>>            A sync object driver that uses a 32bit counter to coordinate
>>            synchronization.  Useful when there is no hardware primitive backing
>>            the synchronization.
>>
>>            WARNING: improper use of this can result in deadlocking kernel
>>            drivers from userspace. Intended for test and debug only.
>>
> 
> But it is adding kernel code for a questionable benefit.

The whole sw_sync is questionable to begin with.

We had that for a couple of years already until we finally realized the problem with the infinite fences.

Today it should only be used for unit testing.

> What about calling the non-asserting version instead of adding complexity? It is kernel code so should be fine.

That would give other implementations both the possibility and impression that this is ok. And that is something I would really like to avoid.

> Because even with the worker sw_sync can still create infinite fences. Worker just defeats the asserts so I do not see the value in complicating it.

Yeah, I mean completely ripping out the sw_sync would be indeed better. But that would break existing unit tests.

Regards,
Christian.

> 
> Regards,
> 
> Tvrtko
> 
>>>
>>>> Signed-off-by: Christian König <ckoenig@able.fritz.box>
>>>> ---
>>>>    drivers/dma-buf/dma-fence.c | 59 +++++++++++++++++++++++++++----------
>>>>    include/linux/dma-fence.h   |  9 ++++--
>>>>    2 files changed, 51 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>>> index 3f78c56b58dc..2bce620eacac 100644
>>>> --- a/drivers/dma-buf/dma-fence.c
>>>> +++ b/drivers/dma-buf/dma-fence.c
>>>> @@ -345,33 +345,23 @@ void __dma_fence_might_wait(void)
>>>>    }
>>>>    #endif
>>>>    -
>>>>    /**
>>>> - * dma_fence_signal_timestamp_locked - signal completion of a fence
>>>> + * dma_fence_signal_internal - internal signal completion of a fence
>>>>     * @fence: the fence to signal
>>>>     * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
>>>>     *
>>>> - * Signal completion for software callbacks on a fence, this will unblock
>>>> - * dma_fence_wait() calls and run all the callbacks added with
>>>> - * dma_fence_add_callback(). Can be called multiple times, but since a fence
>>>> - * can only go from the unsignaled to the signaled state and not back, it will
>>>> - * only be effective the first time. Set the timestamp provided as the fence
>>>> - * signal timestamp.
>>>> - *
>>>> - * Unlike dma_fence_signal_timestamp(), this function must be called with
>>>> - * &dma_fence.lock held.
>>>> + * Internal signal the dma_fence without error checking. Should *NEVER* be used
>>>> + * by drivers or external code directly.
>>>>     *
>>>>     * Returns 0 on success and a negative error value when @fence has been
>>>>     * signalled already.
>>>>     */
>>>> -int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>>> -                      ktime_t timestamp)
>>>> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp)
>>>>    {
>>>>        struct dma_fence_cb *cur, *tmp;
>>>>        struct list_head cb_list;
>>>>          lockdep_assert_held(fence->lock);
>>>> -
>>>>        if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>>>>                          &fence->flags)))
>>>>            return -EINVAL;
>>>> @@ -390,7 +380,46 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>>>          return 0;
>>>>    }
>>>> -EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
>>>> +EXPORT_SYMBOL(dma_fence_signal_internal);
>>>> +
>>>> +/**
>>>> + * dma_fence_signal_timestamp_locked - signal completion of a fence
>>>> + * @fence: the fence to signal
>>>> + * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
>>>> + *
>>>> + * Signal completion for software callbacks on a fence, this will unblock
>>>> + * dma_fence_wait() calls and run all the callbacks added with
>>>> + * dma_fence_add_callback(). Can be called multiple times, but since a fence
>>>> + * can only go from the unsignaled to the signaled state and not back, it will
>>>> + * only be effective the first time. Set the timestamp provided as the fence
>>>> + * signal timestamp.
>>>> + *
>>>> + * Unlike dma_fence_signal_timestamp(), this function must be called with
>>>> + * &dma_fence.lock held.
>>>> + *
>>>> + * Returns 0 on success and a negative error value when @fence has been
>>>> + * signalled already.
>>>> + */
>>>> +int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>>> +                      ktime_t timestamp)
>>>> +{
>>>> +    /*
>>>> +     * We have the re-occurring problem that people try to invent a
>>>> +     * DMA-fences implementation which signals fences based on an userspace
>>>> +     * IOCTL.
>>>> +     *
>>>> +     * This is well known as source of hard to track down crashes and is
>>>> +     * documented to be an invalid approach. The problem is that it seems
>>>> +     * to work during initial testing and only long term tests points out
>>>> +     * why this can never work correctly.
>>>> +     *
>>>> +     * So give at least a warning when people try to signal a fence from
>>>> +     * task context and not from interrupts or a work item. This check is
>>>> +     * certainly not perfect but better than nothing.
>>>> +     */
>>>> +    WARN_ON_ONCE(!in_interrupt() && !current_work());
>>>> +    return dma_fence_signal_internal(fence, timestamp);
>>>> +}
>>>>      /**
>>>>     * dma_fence_signal_timestamp - signal completion of a fence
>>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>>> index 64639e104110..8dbcd66989b8 100644
>>>> --- a/include/linux/dma-fence.h
>>>> +++ b/include/linux/dma-fence.h
>>>> @@ -369,6 +369,7 @@ int dma_fence_signal_locked(struct dma_fence *fence);
>>>>    int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
>>>>    int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>>>                          ktime_t timestamp);
>>>> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp);
>>>>    signed long dma_fence_default_wait(struct dma_fence *fence,
>>>>                       bool intr, signed long timeout);
>>>>    int dma_fence_add_callback(struct dma_fence *fence,
>>>> @@ -422,7 +423,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>>>>            return true;
>>>>          if (fence->ops->signaled && fence->ops->signaled(fence)) {
>>>> -        dma_fence_signal_locked(fence);
>>>> +        dma_fence_signal_internal(fence, ktime_get());
>>>>            return true;
>>>>        }
>>>>    @@ -448,11 +449,15 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>>>>    static inline bool
>>>>    dma_fence_is_signaled(struct dma_fence *fence)
>>>>    {
>>>> +    unsigned long flags;
>>>> +
>>>>        if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>>>>            return true;
>>>>          if (fence->ops->signaled && fence->ops->signaled(fence)) {
>>>> -        dma_fence_signal(fence);
>>>> +        spin_lock_irqsave(fence->lock, flags);
>>>> +        dma_fence_signal_internal(fence, ktime_get());
>>>> +        spin_unlock_irqrestore(fence->lock, flags);
>>>>            return true;
>>>>        }
>>>>    
>>>
>>
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
  2025-08-20  7:27   ` kernel test robot
@ 2025-08-21 13:00     ` Christian König
  2025-09-01 11:14       ` Tvrtko Ursulin
  0 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2025-08-21 13:00 UTC (permalink / raw)
  To: simona.vetter
  Cc: oe-lkp, lkp, linux-media, dri-devel, linaro-mm-sig, ltp,
	tvrtko.ursulin, phasta, airlied, dakr, sumit.semwal

Hi Sima,

I need your opinion on this here. Adding the warning to not signal a fence from IOCTL context yielded it's first victim.

Skimming through the code what i915 does here is to signal a dma_fence from an error handling path. E.g. the dma_fence was allocated, initialized, the vma code finds that something is wrong and signals the fence and frees it's reference.

The function which does that has the following comment:

/**
 * dma_fence_work_commit_imm: Commit the fence, and if possible execute locally.
 * @f: the fenced worker
 *
 * Instead of always scheduling a worker to execute the callback (see
 * dma_fence_work_commit()), we try to execute the callback immediately in
 * the local context. It is required that the fence be committed before it
 * is published, and that no other threads try to tamper with the number
 * of asynchronous waits on the fence (or else the callback will be
 * executed in the wrong context, i.e. not the callers).
 */

This works technically, but I would say that this is absolutely not the best practice.

We have some oportunity to unify dma_fence_work logic between i915 and amdgpu here, but I'm pretty sure I don't want to allow such stuff in amdgpu even if it saves a few of CPU cycles in an error cleanup path.

What's your opinion on that?

Thanks,
Christian.

On 20.08.25 09:27, kernel test robot wrote:
> 
> 
> Hello,
> 
> kernel test robot noticed "WARNING:at_drivers/dma-buf/dma-fence.c:#dma_fence_signal" on:
> 
> commit: 409db68e04bdf052bc03f620e70339764b598ade ("[PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL")
> url: https://github.com/intel-lab-lkp/linux/commits/Christian-K-nig/dma-buf-add-warning-when-dma_fence-is-signaled-from-IOCTL/20250812-223543
> base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 53e760d8949895390e256e723e7ee46618310361
> patch link: https://lore.kernel.org/all/20250812143402.8619-2-christian.koenig@amd.com/
> patch subject: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
> 
> in testcase: ltp
> version: ltp-x86_64-9f512c1d8-1_20250809
> with following parameters:
> 
> 	test: syscalls-ipc-msgstress
> 
> 
> 
> config: x86_64-rhel-9.4-ltp
> compiler: gcc-12
> test machine: 8 threads 1 sockets Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz (Haswell) with 16G memory
> 
> (please refer to attached dmesg/kmsg for entire log/backtrace)
> 
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202508200843.8b006132-lkp@intel.com
> 
> 
> [   51.636268][  T218] ------------[ cut here ]------------
> [ 51.636273][ T218] WARNING: CPU: 3 PID: 218 at drivers/dma-buf/dma-fence.c:420 dma_fence_signal (drivers/dma-buf/dma-fence.c:420 drivers/dma-buf/dma-fence.c:502) 
> [   51.636292][  T218] Modules linked in: coretemp sd_mod snd_hda_codec_realtek_lib snd_hda_codec_generic sg ipmi_devintf kvm_intel snd_hda_intel ipmi_msghandler platform_profile i915(+) kvm snd_hda_codec intel_gtt dell_wmi snd_hda_core drm_buddy binfmt_misc dell_smbios snd_intel_dspcfg ttm dell_wmi_descriptor snd_intel_sdw_acpi snd_hwdep mei_wdt sparse_keymap irqbypass drm_display_helper ahci ghash_clmulni_intel snd_pcm libahci rfkill cec mei_me rapl intel_cstate dcdbas snd_timer drm_client_lib libata intel_uncore mei snd drm_kms_helper i2c_i801 i2c_smbus pcspkr lpc_ich soundcore video wmi fuse drm loop dm_mod
> [   51.636385][  T218] CPU: 3 UID: 0 PID: 218 Comm: (udev-worker) Not tainted 6.17.0-rc1-00006-g409db68e04bd #1 PREEMPT(voluntary)
> [   51.636395][  T218] Hardware name: Dell Inc. OptiPlex 9020/0DNKMN, BIOS A05 12/05/2013
> [ 51.636399][ T218] RIP: 0010:dma_fence_signal (drivers/dma-buf/dma-fence.c:420 drivers/dma-buf/dma-fence.c:502) 
> [ 51.636415][ T218] Code: 00 fc ff df 80 3c 02 00 75 36 48 8b 3b 4c 89 e6 e8 10 33 27 01 89 e8 5b 5d 41 5c c3 cc cc cc cc e8 b0 2e 77 fe 48 85 c0 75 bc <0f> 0b eb b8 0f 0b bd ea ff ff ff 5b 89 e8 5d 41 5c c3 cc cc cc cc
> All code
> ========
>    0:	00 fc                	add    %bh,%ah
>    2:	ff                   	(bad)
>    3:	df 80 3c 02 00 75    	filds  0x7500023c(%rax)
>    9:	36 48 8b 3b          	ss mov (%rbx),%rdi
>    d:	4c 89 e6             	mov    %r12,%rsi
>   10:	e8 10 33 27 01       	call   0x1273325
>   15:	89 e8                	mov    %ebp,%eax
>   17:	5b                   	pop    %rbx
>   18:	5d                   	pop    %rbp
>   19:	41 5c                	pop    %r12
>   1b:	c3                   	ret
>   1c:	cc                   	int3
>   1d:	cc                   	int3
>   1e:	cc                   	int3
>   1f:	cc                   	int3
>   20:	e8 b0 2e 77 fe       	call   0xfffffffffe772ed5
>   25:	48 85 c0             	test   %rax,%rax
>   28:	75 bc                	jne    0xffffffffffffffe6
>   2a:*	0f 0b                	ud2		<-- trapping instruction
>   2c:	eb b8                	jmp    0xffffffffffffffe6
>   2e:	0f 0b                	ud2
>   30:	bd ea ff ff ff       	mov    $0xffffffea,%ebp
>   35:	5b                   	pop    %rbx
>   36:	89 e8                	mov    %ebp,%eax
>   38:	5d                   	pop    %rbp
>   39:	41 5c                	pop    %r12
>   3b:	c3                   	ret
>   3c:	cc                   	int3
>   3d:	cc                   	int3
>   3e:	cc                   	int3
>   3f:	cc                   	int3
> 
> Code starting with the faulting instruction
> ===========================================
>    0:	0f 0b                	ud2
>    2:	eb b8                	jmp    0xffffffffffffffbc
>    4:	0f 0b                	ud2
>    6:	bd ea ff ff ff       	mov    $0xffffffea,%ebp
>    b:	5b                   	pop    %rbx
>    c:	89 e8                	mov    %ebp,%eax
>    e:	5d                   	pop    %rbp
>    f:	41 5c                	pop    %r12
>   11:	c3                   	ret
>   12:	cc                   	int3
>   13:	cc                   	int3
>   14:	cc                   	int3
>   15:	cc                   	int3
> [   51.636420][  T218] RSP: 0018:ffffc90000a9ed30 EFLAGS: 00010046
> [   51.636428][  T218] RAX: 0000000000000000 RBX: ffff88811750fc00 RCX: 0000000000000018
> [   51.636437][  T218] RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffff88810691512c
> [   51.636440][  T218] RBP: 0000000be56b1408 R08: 0000000000000001 R09: fffff52000153d9a
> [   51.636445][  T218] R10: 0000000000000003 R11: ffff888108145000 R12: 0000000000000246
> [   51.636452][  T218] R13: ffffffffc1c9b060 R14: ffff88810406ba0c R15: 1ffff92000153dc2
> [   51.636455][  T218] FS:  00007efd90c038c0(0000) GS:ffff8883e4077000(0000) knlGS:0000000000000000
> [   51.636459][  T218] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   51.636462][  T218] CR2: 00007f5bd8238c20 CR3: 000000040ed8a005 CR4: 00000000001726f0
> [   51.636466][  T218] Call Trace:
> [   51.636469][  T218]  <TASK>
> [ 51.636477][ T218] fence_work (include/linux/dma-fence.h:272 drivers/gpu/drm/i915/i915_sw_fence_work.c:23) i915 
> [ 51.637304][ T218] fence_notify (drivers/gpu/drm/i915/i915_sw_fence_work.c:39) i915 
> [ 51.637827][ T218] __i915_sw_fence_complete (drivers/gpu/drm/i915/i915_sw_fence.c:201) i915 
> [ 51.638300][ T218] i915_vma_pin_ww (drivers/gpu/drm/i915/i915_vma.c:1601) i915 
> [ 51.638763][ T218] ? __pfx_i915_vma_pin_ww (drivers/gpu/drm/i915/i915_vma.c:1434) i915 
> [ 51.639218][ T218] ? i915_gem_object_make_unshrinkable (include/linux/list.h:215 include/linux/list.h:287 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:500) i915 
> [ 51.639648][ T218] ? i915_vma_make_unshrinkable (drivers/gpu/drm/i915/i915_vma.c:2292) i915 
> [ 51.640091][ T218] ? intel_ring_pin (drivers/gpu/drm/i915/gt/intel_ring.c:73) i915 
> [ 51.640505][ T218] intel_ring_submission_setup (drivers/gpu/drm/i915/gt/intel_ring_submission.c:1290 drivers/gpu/drm/i915/gt/intel_ring_submission.c:1421) i915 
> [ 51.640918][ T218] ? __pfx_intel_ring_submission_setup (drivers/gpu/drm/i915/gt/intel_ring_submission.c:1349) i915 
> [   51.641232][   T65] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
> [ 51.641321][ T218] ? intel_engine_init_whitelist (drivers/gpu/drm/i915/gt/intel_workarounds.c:2104) i915 
> [ 51.641735][ T218] ? __intel_wakeref_init (arch/x86/include/asm/atomic.h:28 include/linux/atomic/atomic-arch-fallback.h:503 include/linux/atomic/atomic-instrumented.h:68 drivers/gpu/drm/i915/intel_wakeref.c:109) i915 
> [ 51.642126][ T218] intel_engines_init (drivers/gpu/drm/i915/gt/intel_engine_cs.c:1514) i915 
> [ 51.642521][ T218] ? i915_gem_object_make_unshrinkable (arch/x86/include/asm/atomic.h:93 include/linux/atomic/atomic-arch-fallback.h:667 include/linux/atomic/atomic-arch-fallback.h:1119 include/linux/atomic/atomic-instrumented.h:524 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:498) i915 
> [ 51.642929][ T218] ? __pfx_intel_ring_submission_setup (drivers/gpu/drm/i915/gt/intel_ring_submission.c:1349) i915 
> [ 51.643331][ T218] intel_gt_init (drivers/gpu/drm/i915/gt/intel_gt.c:719) i915 
> [ 51.643728][ T218] i915_gem_init (drivers/gpu/drm/i915/i915_gem.c:1191) i915 
> [ 51.644140][ T218] i915_driver_probe (drivers/gpu/drm/i915/i915_driver.c:831) i915 
> [ 51.644524][ T218] ? __pfx_i915_driver_probe (drivers/gpu/drm/i915/i915_driver.c:780) i915 
> [ 51.644903][ T218] ? drm_privacy_screen_get (drivers/gpu/drm/drm_privacy_screen.c:169) drm 
> [ 51.645047][ T218] ? intel_display_driver_probe_defer (drivers/gpu/drm/i915/display/intel_display_driver.c:84) i915 
> [ 51.645483][ T218] ? i915_pci_probe (drivers/gpu/drm/i915/i915_pci.c:995) i915 
> [ 51.645876][ T218] ? __pfx_i915_pci_probe (drivers/gpu/drm/i915/i915_pci.c:956) i915 
> [ 51.646267][ T218] local_pci_probe (drivers/pci/pci-driver.c:324) 
> [ 51.646283][ T218] pci_call_probe (drivers/pci/pci-driver.c:392) 
> [ 51.646295][ T218] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:107 include/linux/atomic/atomic-arch-fallback.h:2170 include/linux/atomic/atomic-instrumented.h:1302 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:187 include/linux/spinlock_api_smp.h:134 kernel/locking/spinlock.c:154) 
> [ 51.646308][ T218] ? __pfx_pci_call_probe (drivers/pci/pci-driver.c:352) 
> [ 51.646321][ T218] ? kernfs_add_one (fs/kernfs/dir.c:834) 
> [ 51.646337][ T218] ? pci_assign_irq (drivers/pci/irq.c:149) 
> [ 51.646350][ T218] ? pci_match_device (drivers/pci/pci-driver.c:159 (discriminator 1)) 
> [ 51.646362][ T218] ? kernfs_put (arch/x86/include/asm/atomic.h:67 (discriminator 1) include/linux/atomic/atomic-arch-fallback.h:2278 (discriminator 1) include/linux/atomic/atomic-instrumented.h:1384 (discriminator 1) fs/kernfs/dir.c:569 (discriminator 1)) 
> [ 51.646368][ T218] pci_device_probe (drivers/pci/pci-driver.c:452) 
> [ 51.646377][ T218] really_probe (drivers/base/dd.c:581 drivers/base/dd.c:659) 
> [ 51.646391][ T218] __driver_probe_device (drivers/base/dd.c:801) 
> [ 51.646404][ T218] driver_probe_device (drivers/base/dd.c:831) 
> [ 51.646416][ T218] __driver_attach (drivers/base/dd.c:1218) 
> [ 51.646424][ T218] ? __pfx___driver_attach (drivers/base/dd.c:1158) 
> [ 51.646428][ T218] bus_for_each_dev (drivers/base/bus.c:369) 
> [ 51.646441][ T218] ? __pfx_bus_for_each_dev (drivers/base/bus.c:358) 
> [ 51.646444][ T218] ? __kmalloc_cache_noprof (arch/x86/include/asm/jump_label.h:46 include/linux/memcontrol.h:1714 mm/slub.c:2210 mm/slub.c:4190 mm/slub.c:4229 mm/slub.c:4391) 
> [ 51.646456][ T218] ? klist_add_tail (include/linux/list.h:150 include/linux/list.h:183 lib/klist.c:104 lib/klist.c:137) 
> [ 51.646468][ T218] bus_add_driver (drivers/base/bus.c:678) 
> [ 51.646482][ T218] driver_register (drivers/base/driver.c:249) 
> [ 51.646490][ T218] i915_init (drivers/gpu/drm/i915/i915_driver.c:1428) i915 
> [ 51.646891][ T218] ? __pfx_i915_init (drivers/gpu/drm/i915/i915_config.c:13) i915 
> [   51.647101][   T67] sd 2:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
> [ 51.647277][ T218] do_one_initcall (init/main.c:1269) 
> [ 51.647292][ T218] ? kfree (mm/slub.c:4680 mm/slub.c:4879) 
> [ 51.647304][ T218] ? __pfx_do_one_initcall (init/main.c:1260) 
> [ 51.647315][ T218] ? kasan_unpoison (mm/kasan/shadow.c:156 mm/kasan/shadow.c:182) 
> [ 51.647327][ T218] ? __kasan_slab_alloc (mm/kasan/common.c:329 mm/kasan/common.c:356) 
> [ 51.647340][ T218] ? __kmalloc_cache_noprof (mm/slub.c:4180 mm/slub.c:4229 mm/slub.c:4391) 
> [ 51.647352][ T218] ? kasan_save_track (arch/x86/include/asm/current.h:25 mm/kasan/common.c:60 mm/kasan/common.c:69) 
> [ 51.647365][ T218] ? kasan_unpoison (mm/kasan/shadow.c:156 mm/kasan/shadow.c:182) 
> [ 51.647377][ T218] do_init_module (kernel/module/main.c:3039) 
> [ 51.647388][ T218] ? __pfx_do_init_module (kernel/module/main.c:3011) 
> [ 51.647402][ T218] ? kfree (mm/slub.c:4680 mm/slub.c:4879) 
> [ 51.647414][ T218] ? klp_module_coming (kernel/livepatch/core.c:1317) 
> [ 51.647426][ T218] ? load_module (kernel/module/main.c:2468 kernel/module/main.c:2463 kernel/module/main.c:3504) 
> [ 51.647441][ T218] load_module (kernel/module/main.c:3509) 
> [ 51.647449][ T218] ? ima_post_read_file (security/integrity/ima/ima_main.c:896 security/integrity/ima/ima_main.c:878) 
> [ 51.647466][ T218] ? __pfx_load_module (kernel/module/main.c:3353) 
> [ 51.647478][ T218] ? __pfx_kernel_read_file (fs/kernel_read_file.c:38) 
> [ 51.647489][ T218] ? init_module_from_file (kernel/module/main.c:3701) 
> [ 51.647499][ T218] init_module_from_file (kernel/module/main.c:3701) 
> [ 51.647514][ T218] ? __pfx_init_module_from_file (kernel/module/main.c:3677) 
> [ 51.647525][ T218] ? mm_get_unmapped_area (arch/x86/include/asm/bitops.h:206 arch/x86/include/asm/bitops.h:238 include/asm-generic/bitops/instrumented-non-atomic.h:142 mm/mmap.c:805 mm/mmap.c:871) 
> [ 51.647540][ T218] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:107 include/linux/atomic/atomic-arch-fallback.h:2170 include/linux/atomic/atomic-instrumented.h:1302 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:187 include/linux/spinlock_api_smp.h:134 kernel/locking/spinlock.c:154) 
> [ 51.647547][ T218] ? __pfx__raw_spin_lock (kernel/locking/spinlock.c:153) 
> [ 51.647560][ T218] idempotent_init_module (kernel/module/main.c:3713) 
> [ 51.647573][ T218] ? __pfx_idempotent_init_module (kernel/module/main.c:3705) 
> [ 51.647582][ T218] ? __pfx___seccomp_filter (kernel/seccomp.c:1244) 
> [ 51.647590][ T218] ? fdget (include/linux/atomic/atomic-arch-fallback.h:479 include/linux/atomic/atomic-instrumented.h:50 fs/file.c:1167 fs/file.c:1181) 
> [ 51.647607][ T218] ? security_capable (security/security.c:1142) 
> [ 51.647615][ T218] __x64_sys_finit_module (include/linux/file.h:62 include/linux/file.h:83 kernel/module/main.c:3736 kernel/module/main.c:3723 kernel/module/main.c:3723) 
> [ 51.647627][ T218] ? syscall_trace_enter (kernel/entry/syscall-common.c:44) 
> [ 51.647640][ T218] do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94) 
> [ 51.647657][ T218] ? fput (arch/x86/include/asm/atomic64_64.h:79 include/linux/atomic/atomic-arch-fallback.h:2913 include/linux/atomic/atomic-arch-fallback.h:3364 include/linux/atomic/atomic-long.h:698 include/linux/atomic/atomic-instrumented.h:3767 include/linux/file_ref.h:157 fs/file_table.c:544) 
> [ 51.647668][ T218] ? ksys_mmap_pgoff (mm/mmap.c:609) 
> [ 51.647682][ T218] ? do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94) 
> [ 51.647694][ T218] ? from_kgid_munged (kernel/user_namespace.c:535) 
> [ 51.647708][ T218] ? _copy_to_user (arch/x86/include/asm/uaccess_64.h:126 arch/x86/include/asm/uaccess_64.h:147 include/linux/uaccess.h:197 lib/usercopy.c:26) 
> [ 51.647722][ T218] ? cp_new_stat (fs/stat.c:471) 
> [ 51.647732][ T218] ? __pfx_cp_new_stat (fs/stat.c:471) 
> 
> 
> The kernel config and materials to reproduce are available at:
> https://download.01.org/0day-ci/archive/20250820/202508200843.8b006132-lkp@intel.com
> 
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
  2025-08-21 12:46         ` Christian König
@ 2025-08-22  8:08           ` Tvrtko Ursulin
  0 siblings, 0 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2025-08-22  8:08 UTC (permalink / raw)
  To: Christian König, simona.vetter, phasta, airlied, dakr,
	sumit.semwal, dri-devel, linux-media


On 21/08/2025 13:46, Christian König wrote:
> On 13.08.25 13:59, Tvrtko Ursulin wrote:
>>> Good point. Could be that someone is using a pure kernel thread for fence signaling. Going to check for that instead of a worker.
>>
>> Ok, I am curious how to do it reliably. Non-null current and PF_KTHREAD and PF_WQ_WORKER not set?
> 
> If I'm not completely mistaken (current->flags & PF_KTHREAD) should do it, but I need to double check that as well.
> 
>>>> Even the fact opportunistic signalling needs to bypass the assert makes it sound like there isn't anything fundamentally wrong with signalling from task context.
>>>>
>>>
>>> Opportunistic signaling can happen from everywhere. But when an implementation tries to signal it from an IOCTL that is certainly invalid.
>>>
>>>> The first patch also feels a bit too much if it has no purpose apart from checking the new asserts would otherwise trigger.
>>>
>>> The sw_sync code is is only used for testing and debugging. See the Kconfig of it:
>>>
>>>             A sync object driver that uses a 32bit counter to coordinate
>>>             synchronization.  Useful when there is no hardware primitive backing
>>>             the synchronization.
>>>
>>>             WARNING: improper use of this can result in deadlocking kernel
>>>             drivers from userspace. Intended for test and debug only.
>>>
>>
>> But it is adding kernel code for a questionable benefit.
> 
> The whole sw_sync is questionable to begin with.
> 
> We had that for a couple of years already until we finally realized the problem with the infinite fences.
> 
> Today it should only be used for unit testing.
> 
>> What about calling the non-asserting version instead of adding complexity? It is kernel code so should be fine.
> 
> That would give other implementations both the possibility and impression that this is ok. And that is something I would really like to avoid.

I think a "big fat" comment next to the (only?) caller should be enough 
and on balance better than complicating the code for a debug component, 
which even adds an artificial/misleading limitation. I mean a 
complication plus limitation, with the sole purpose of making the assert 
feasible does not sound like the best option.

Perhaps a patch to checkpatch too to flag "no new users of this api 
should be added"? I wonder if there is a generic way to tie checkpatch 
with a list of deprecated API and if not should one be proposed.

Regards,

Tvrtko

>> Because even with the worker sw_sync can still create infinite fences. Worker just defeats the asserts so I do not see the value in complicating it.
> 
> Yeah, I mean completely ripping out the sw_sync would be indeed better. But that would break existing unit tests.
> 
> Regards,
> Christian.
> 
>>
>> Regards,
>>
>> Tvrtko
>>
>>>>
>>>>> Signed-off-by: Christian König <ckoenig@able.fritz.box>
>>>>> ---
>>>>>     drivers/dma-buf/dma-fence.c | 59 +++++++++++++++++++++++++++----------
>>>>>     include/linux/dma-fence.h   |  9 ++++--
>>>>>     2 files changed, 51 insertions(+), 17 deletions(-)
>>>>>
>>>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>>>> index 3f78c56b58dc..2bce620eacac 100644
>>>>> --- a/drivers/dma-buf/dma-fence.c
>>>>> +++ b/drivers/dma-buf/dma-fence.c
>>>>> @@ -345,33 +345,23 @@ void __dma_fence_might_wait(void)
>>>>>     }
>>>>>     #endif
>>>>>     -
>>>>>     /**
>>>>> - * dma_fence_signal_timestamp_locked - signal completion of a fence
>>>>> + * dma_fence_signal_internal - internal signal completion of a fence
>>>>>      * @fence: the fence to signal
>>>>>      * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
>>>>>      *
>>>>> - * Signal completion for software callbacks on a fence, this will unblock
>>>>> - * dma_fence_wait() calls and run all the callbacks added with
>>>>> - * dma_fence_add_callback(). Can be called multiple times, but since a fence
>>>>> - * can only go from the unsignaled to the signaled state and not back, it will
>>>>> - * only be effective the first time. Set the timestamp provided as the fence
>>>>> - * signal timestamp.
>>>>> - *
>>>>> - * Unlike dma_fence_signal_timestamp(), this function must be called with
>>>>> - * &dma_fence.lock held.
>>>>> + * Internal signal the dma_fence without error checking. Should *NEVER* be used
>>>>> + * by drivers or external code directly.
>>>>>      *
>>>>>      * Returns 0 on success and a negative error value when @fence has been
>>>>>      * signalled already.
>>>>>      */
>>>>> -int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>>>> -                      ktime_t timestamp)
>>>>> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp)
>>>>>     {
>>>>>         struct dma_fence_cb *cur, *tmp;
>>>>>         struct list_head cb_list;
>>>>>           lockdep_assert_held(fence->lock);
>>>>> -
>>>>>         if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>>>>>                           &fence->flags)))
>>>>>             return -EINVAL;
>>>>> @@ -390,7 +380,46 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>>>>           return 0;
>>>>>     }
>>>>> -EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
>>>>> +EXPORT_SYMBOL(dma_fence_signal_internal);
>>>>> +
>>>>> +/**
>>>>> + * dma_fence_signal_timestamp_locked - signal completion of a fence
>>>>> + * @fence: the fence to signal
>>>>> + * @timestamp: fence signal timestamp in kernel's CLOCK_MONOTONIC time domain
>>>>> + *
>>>>> + * Signal completion for software callbacks on a fence, this will unblock
>>>>> + * dma_fence_wait() calls and run all the callbacks added with
>>>>> + * dma_fence_add_callback(). Can be called multiple times, but since a fence
>>>>> + * can only go from the unsignaled to the signaled state and not back, it will
>>>>> + * only be effective the first time. Set the timestamp provided as the fence
>>>>> + * signal timestamp.
>>>>> + *
>>>>> + * Unlike dma_fence_signal_timestamp(), this function must be called with
>>>>> + * &dma_fence.lock held.
>>>>> + *
>>>>> + * Returns 0 on success and a negative error value when @fence has been
>>>>> + * signalled already.
>>>>> + */
>>>>> +int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>>>> +                      ktime_t timestamp)
>>>>> +{
>>>>> +    /*
>>>>> +     * We have the re-occurring problem that people try to invent a
>>>>> +     * DMA-fences implementation which signals fences based on an userspace
>>>>> +     * IOCTL.
>>>>> +     *
>>>>> +     * This is well known as source of hard to track down crashes and is
>>>>> +     * documented to be an invalid approach. The problem is that it seems
>>>>> +     * to work during initial testing and only long term tests points out
>>>>> +     * why this can never work correctly.
>>>>> +     *
>>>>> +     * So give at least a warning when people try to signal a fence from
>>>>> +     * task context and not from interrupts or a work item. This check is
>>>>> +     * certainly not perfect but better than nothing.
>>>>> +     */
>>>>> +    WARN_ON_ONCE(!in_interrupt() && !current_work());
>>>>> +    return dma_fence_signal_internal(fence, timestamp);
>>>>> +}
>>>>>       /**
>>>>>      * dma_fence_signal_timestamp - signal completion of a fence
>>>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>>>> index 64639e104110..8dbcd66989b8 100644
>>>>> --- a/include/linux/dma-fence.h
>>>>> +++ b/include/linux/dma-fence.h
>>>>> @@ -369,6 +369,7 @@ int dma_fence_signal_locked(struct dma_fence *fence);
>>>>>     int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
>>>>>     int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>>>>                           ktime_t timestamp);
>>>>> +int dma_fence_signal_internal(struct dma_fence *fence, ktime_t timestamp);
>>>>>     signed long dma_fence_default_wait(struct dma_fence *fence,
>>>>>                        bool intr, signed long timeout);
>>>>>     int dma_fence_add_callback(struct dma_fence *fence,
>>>>> @@ -422,7 +423,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>>>>>             return true;
>>>>>           if (fence->ops->signaled && fence->ops->signaled(fence)) {
>>>>> -        dma_fence_signal_locked(fence);
>>>>> +        dma_fence_signal_internal(fence, ktime_get());
>>>>>             return true;
>>>>>         }
>>>>>     @@ -448,11 +449,15 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>>>>>     static inline bool
>>>>>     dma_fence_is_signaled(struct dma_fence *fence)
>>>>>     {
>>>>> +    unsigned long flags;
>>>>> +
>>>>>         if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>>>>>             return true;
>>>>>           if (fence->ops->signaled && fence->ops->signaled(fence)) {
>>>>> -        dma_fence_signal(fence);
>>>>> +        spin_lock_irqsave(fence->lock, flags);
>>>>> +        dma_fence_signal_internal(fence, ktime_get());
>>>>> +        spin_unlock_irqrestore(fence->lock, flags);
>>>>>             return true;
>>>>>         }
>>>>>     
>>>>
>>>
>>
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
  2025-08-21 13:00     ` Christian König
@ 2025-09-01 11:14       ` Tvrtko Ursulin
  0 siblings, 0 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2025-09-01 11:14 UTC (permalink / raw)
  To: Christian König, simona.vetter
  Cc: oe-lkp, lkp, linux-media, dri-devel, linaro-mm-sig, ltp,
	tvrtko.ursulin, phasta, airlied, dakr, sumit.semwal


On 21/08/2025 14:00, Christian König wrote:
> Hi Sima,
> 
> I need your opinion on this here. Adding the warning to not signal a fence from IOCTL context yielded it's first victim.
> 
> Skimming through the code what i915 does here is to signal a dma_fence from an error handling path. E.g. the dma_fence was allocated, initialized, the vma code finds that something is wrong and signals the fence and frees it's reference.
> 
> The function which does that has the following comment:
> 
> /**
>   * dma_fence_work_commit_imm: Commit the fence, and if possible execute locally.
>   * @f: the fenced worker
>   *
>   * Instead of always scheduling a worker to execute the callback (see
>   * dma_fence_work_commit()), we try to execute the callback immediately in
>   * the local context. It is required that the fence be committed before it
>   * is published, and that no other threads try to tamper with the number
>   * of asynchronous waits on the fence (or else the callback will be
>   * executed in the wrong context, i.e. not the callers).
>   */
> 
> This works technically, but I would say that this is absolutely not the best practice.
> 
> We have some oportunity to unify dma_fence_work logic between i915 and amdgpu here, but I'm pretty sure I don't want to allow such stuff in amdgpu even if it saves a few of CPU cycles in an error cleanup path.

Jumping in a bit. I had a quick look to remind myself of this area. 
Unfortunately it is not an error cleanup path but the goto labels are a 
bit misleadingly named. The immediate path triggers quite a lot in 
practice so to remove this would need some testing to ensure no regressions.

Regards,

Tvrtko

> 
> What's your opinion on that?
> 
> Thanks,
> Christian.
> 
> On 20.08.25 09:27, kernel test robot wrote:
>>
>>
>> Hello,
>>
>> kernel test robot noticed "WARNING:at_drivers/dma-buf/dma-fence.c:#dma_fence_signal" on:
>>
>> commit: 409db68e04bdf052bc03f620e70339764b598ade ("[PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL")
>> url: https://github.com/intel-lab-lkp/linux/commits/Christian-K-nig/dma-buf-add-warning-when-dma_fence-is-signaled-from-IOCTL/20250812-223543
>> base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 53e760d8949895390e256e723e7ee46618310361
>> patch link: https://lore.kernel.org/all/20250812143402.8619-2-christian.koenig@amd.com/
>> patch subject: [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL
>>
>> in testcase: ltp
>> version: ltp-x86_64-9f512c1d8-1_20250809
>> with following parameters:
>>
>> 	test: syscalls-ipc-msgstress
>>
>>
>>
>> config: x86_64-rhel-9.4-ltp
>> compiler: gcc-12
>> test machine: 8 threads 1 sockets Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz (Haswell) with 16G memory
>>
>> (please refer to attached dmesg/kmsg for entire log/backtrace)
>>
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <oliver.sang@intel.com>
>> | Closes: https://lore.kernel.org/oe-lkp/202508200843.8b006132-lkp@intel.com
>>
>>
>> [   51.636268][  T218] ------------[ cut here ]------------
>> [ 51.636273][ T218] WARNING: CPU: 3 PID: 218 at drivers/dma-buf/dma-fence.c:420 dma_fence_signal (drivers/dma-buf/dma-fence.c:420 drivers/dma-buf/dma-fence.c:502)
>> [   51.636292][  T218] Modules linked in: coretemp sd_mod snd_hda_codec_realtek_lib snd_hda_codec_generic sg ipmi_devintf kvm_intel snd_hda_intel ipmi_msghandler platform_profile i915(+) kvm snd_hda_codec intel_gtt dell_wmi snd_hda_core drm_buddy binfmt_misc dell_smbios snd_intel_dspcfg ttm dell_wmi_descriptor snd_intel_sdw_acpi snd_hwdep mei_wdt sparse_keymap irqbypass drm_display_helper ahci ghash_clmulni_intel snd_pcm libahci rfkill cec mei_me rapl intel_cstate dcdbas snd_timer drm_client_lib libata intel_uncore mei snd drm_kms_helper i2c_i801 i2c_smbus pcspkr lpc_ich soundcore video wmi fuse drm loop dm_mod
>> [   51.636385][  T218] CPU: 3 UID: 0 PID: 218 Comm: (udev-worker) Not tainted 6.17.0-rc1-00006-g409db68e04bd #1 PREEMPT(voluntary)
>> [   51.636395][  T218] Hardware name: Dell Inc. OptiPlex 9020/0DNKMN, BIOS A05 12/05/2013
>> [ 51.636399][ T218] RIP: 0010:dma_fence_signal (drivers/dma-buf/dma-fence.c:420 drivers/dma-buf/dma-fence.c:502)
>> [ 51.636415][ T218] Code: 00 fc ff df 80 3c 02 00 75 36 48 8b 3b 4c 89 e6 e8 10 33 27 01 89 e8 5b 5d 41 5c c3 cc cc cc cc e8 b0 2e 77 fe 48 85 c0 75 bc <0f> 0b eb b8 0f 0b bd ea ff ff ff 5b 89 e8 5d 41 5c c3 cc cc cc cc
>> All code
>> ========
>>     0:	00 fc                	add    %bh,%ah
>>     2:	ff                   	(bad)
>>     3:	df 80 3c 02 00 75    	filds  0x7500023c(%rax)
>>     9:	36 48 8b 3b          	ss mov (%rbx),%rdi
>>     d:	4c 89 e6             	mov    %r12,%rsi
>>    10:	e8 10 33 27 01       	call   0x1273325
>>    15:	89 e8                	mov    %ebp,%eax
>>    17:	5b                   	pop    %rbx
>>    18:	5d                   	pop    %rbp
>>    19:	41 5c                	pop    %r12
>>    1b:	c3                   	ret
>>    1c:	cc                   	int3
>>    1d:	cc                   	int3
>>    1e:	cc                   	int3
>>    1f:	cc                   	int3
>>    20:	e8 b0 2e 77 fe       	call   0xfffffffffe772ed5
>>    25:	48 85 c0             	test   %rax,%rax
>>    28:	75 bc                	jne    0xffffffffffffffe6
>>    2a:*	0f 0b                	ud2		<-- trapping instruction
>>    2c:	eb b8                	jmp    0xffffffffffffffe6
>>    2e:	0f 0b                	ud2
>>    30:	bd ea ff ff ff       	mov    $0xffffffea,%ebp
>>    35:	5b                   	pop    %rbx
>>    36:	89 e8                	mov    %ebp,%eax
>>    38:	5d                   	pop    %rbp
>>    39:	41 5c                	pop    %r12
>>    3b:	c3                   	ret
>>    3c:	cc                   	int3
>>    3d:	cc                   	int3
>>    3e:	cc                   	int3
>>    3f:	cc                   	int3
>>
>> Code starting with the faulting instruction
>> ===========================================
>>     0:	0f 0b                	ud2
>>     2:	eb b8                	jmp    0xffffffffffffffbc
>>     4:	0f 0b                	ud2
>>     6:	bd ea ff ff ff       	mov    $0xffffffea,%ebp
>>     b:	5b                   	pop    %rbx
>>     c:	89 e8                	mov    %ebp,%eax
>>     e:	5d                   	pop    %rbp
>>     f:	41 5c                	pop    %r12
>>    11:	c3                   	ret
>>    12:	cc                   	int3
>>    13:	cc                   	int3
>>    14:	cc                   	int3
>>    15:	cc                   	int3
>> [   51.636420][  T218] RSP: 0018:ffffc90000a9ed30 EFLAGS: 00010046
>> [   51.636428][  T218] RAX: 0000000000000000 RBX: ffff88811750fc00 RCX: 0000000000000018
>> [   51.636437][  T218] RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffff88810691512c
>> [   51.636440][  T218] RBP: 0000000be56b1408 R08: 0000000000000001 R09: fffff52000153d9a
>> [   51.636445][  T218] R10: 0000000000000003 R11: ffff888108145000 R12: 0000000000000246
>> [   51.636452][  T218] R13: ffffffffc1c9b060 R14: ffff88810406ba0c R15: 1ffff92000153dc2
>> [   51.636455][  T218] FS:  00007efd90c038c0(0000) GS:ffff8883e4077000(0000) knlGS:0000000000000000
>> [   51.636459][  T218] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [   51.636462][  T218] CR2: 00007f5bd8238c20 CR3: 000000040ed8a005 CR4: 00000000001726f0
>> [   51.636466][  T218] Call Trace:
>> [   51.636469][  T218]  <TASK>
>> [ 51.636477][ T218] fence_work (include/linux/dma-fence.h:272 drivers/gpu/drm/i915/i915_sw_fence_work.c:23) i915
>> [ 51.637304][ T218] fence_notify (drivers/gpu/drm/i915/i915_sw_fence_work.c:39) i915
>> [ 51.637827][ T218] __i915_sw_fence_complete (drivers/gpu/drm/i915/i915_sw_fence.c:201) i915
>> [ 51.638300][ T218] i915_vma_pin_ww (drivers/gpu/drm/i915/i915_vma.c:1601) i915
>> [ 51.638763][ T218] ? __pfx_i915_vma_pin_ww (drivers/gpu/drm/i915/i915_vma.c:1434) i915
>> [ 51.639218][ T218] ? i915_gem_object_make_unshrinkable (include/linux/list.h:215 include/linux/list.h:287 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:500) i915
>> [ 51.639648][ T218] ? i915_vma_make_unshrinkable (drivers/gpu/drm/i915/i915_vma.c:2292) i915
>> [ 51.640091][ T218] ? intel_ring_pin (drivers/gpu/drm/i915/gt/intel_ring.c:73) i915
>> [ 51.640505][ T218] intel_ring_submission_setup (drivers/gpu/drm/i915/gt/intel_ring_submission.c:1290 drivers/gpu/drm/i915/gt/intel_ring_submission.c:1421) i915
>> [ 51.640918][ T218] ? __pfx_intel_ring_submission_setup (drivers/gpu/drm/i915/gt/intel_ring_submission.c:1349) i915
>> [   51.641232][   T65] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
>> [ 51.641321][ T218] ? intel_engine_init_whitelist (drivers/gpu/drm/i915/gt/intel_workarounds.c:2104) i915
>> [ 51.641735][ T218] ? __intel_wakeref_init (arch/x86/include/asm/atomic.h:28 include/linux/atomic/atomic-arch-fallback.h:503 include/linux/atomic/atomic-instrumented.h:68 drivers/gpu/drm/i915/intel_wakeref.c:109) i915
>> [ 51.642126][ T218] intel_engines_init (drivers/gpu/drm/i915/gt/intel_engine_cs.c:1514) i915
>> [ 51.642521][ T218] ? i915_gem_object_make_unshrinkable (arch/x86/include/asm/atomic.h:93 include/linux/atomic/atomic-arch-fallback.h:667 include/linux/atomic/atomic-arch-fallback.h:1119 include/linux/atomic/atomic-instrumented.h:524 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:498) i915
>> [ 51.642929][ T218] ? __pfx_intel_ring_submission_setup (drivers/gpu/drm/i915/gt/intel_ring_submission.c:1349) i915
>> [ 51.643331][ T218] intel_gt_init (drivers/gpu/drm/i915/gt/intel_gt.c:719) i915
>> [ 51.643728][ T218] i915_gem_init (drivers/gpu/drm/i915/i915_gem.c:1191) i915
>> [ 51.644140][ T218] i915_driver_probe (drivers/gpu/drm/i915/i915_driver.c:831) i915
>> [ 51.644524][ T218] ? __pfx_i915_driver_probe (drivers/gpu/drm/i915/i915_driver.c:780) i915
>> [ 51.644903][ T218] ? drm_privacy_screen_get (drivers/gpu/drm/drm_privacy_screen.c:169) drm
>> [ 51.645047][ T218] ? intel_display_driver_probe_defer (drivers/gpu/drm/i915/display/intel_display_driver.c:84) i915
>> [ 51.645483][ T218] ? i915_pci_probe (drivers/gpu/drm/i915/i915_pci.c:995) i915
>> [ 51.645876][ T218] ? __pfx_i915_pci_probe (drivers/gpu/drm/i915/i915_pci.c:956) i915
>> [ 51.646267][ T218] local_pci_probe (drivers/pci/pci-driver.c:324)
>> [ 51.646283][ T218] pci_call_probe (drivers/pci/pci-driver.c:392)
>> [ 51.646295][ T218] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:107 include/linux/atomic/atomic-arch-fallback.h:2170 include/linux/atomic/atomic-instrumented.h:1302 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:187 include/linux/spinlock_api_smp.h:134 kernel/locking/spinlock.c:154)
>> [ 51.646308][ T218] ? __pfx_pci_call_probe (drivers/pci/pci-driver.c:352)
>> [ 51.646321][ T218] ? kernfs_add_one (fs/kernfs/dir.c:834)
>> [ 51.646337][ T218] ? pci_assign_irq (drivers/pci/irq.c:149)
>> [ 51.646350][ T218] ? pci_match_device (drivers/pci/pci-driver.c:159 (discriminator 1))
>> [ 51.646362][ T218] ? kernfs_put (arch/x86/include/asm/atomic.h:67 (discriminator 1) include/linux/atomic/atomic-arch-fallback.h:2278 (discriminator 1) include/linux/atomic/atomic-instrumented.h:1384 (discriminator 1) fs/kernfs/dir.c:569 (discriminator 1))
>> [ 51.646368][ T218] pci_device_probe (drivers/pci/pci-driver.c:452)
>> [ 51.646377][ T218] really_probe (drivers/base/dd.c:581 drivers/base/dd.c:659)
>> [ 51.646391][ T218] __driver_probe_device (drivers/base/dd.c:801)
>> [ 51.646404][ T218] driver_probe_device (drivers/base/dd.c:831)
>> [ 51.646416][ T218] __driver_attach (drivers/base/dd.c:1218)
>> [ 51.646424][ T218] ? __pfx___driver_attach (drivers/base/dd.c:1158)
>> [ 51.646428][ T218] bus_for_each_dev (drivers/base/bus.c:369)
>> [ 51.646441][ T218] ? __pfx_bus_for_each_dev (drivers/base/bus.c:358)
>> [ 51.646444][ T218] ? __kmalloc_cache_noprof (arch/x86/include/asm/jump_label.h:46 include/linux/memcontrol.h:1714 mm/slub.c:2210 mm/slub.c:4190 mm/slub.c:4229 mm/slub.c:4391)
>> [ 51.646456][ T218] ? klist_add_tail (include/linux/list.h:150 include/linux/list.h:183 lib/klist.c:104 lib/klist.c:137)
>> [ 51.646468][ T218] bus_add_driver (drivers/base/bus.c:678)
>> [ 51.646482][ T218] driver_register (drivers/base/driver.c:249)
>> [ 51.646490][ T218] i915_init (drivers/gpu/drm/i915/i915_driver.c:1428) i915
>> [ 51.646891][ T218] ? __pfx_i915_init (drivers/gpu/drm/i915/i915_config.c:13) i915
>> [   51.647101][   T67] sd 2:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
>> [ 51.647277][ T218] do_one_initcall (init/main.c:1269)
>> [ 51.647292][ T218] ? kfree (mm/slub.c:4680 mm/slub.c:4879)
>> [ 51.647304][ T218] ? __pfx_do_one_initcall (init/main.c:1260)
>> [ 51.647315][ T218] ? kasan_unpoison (mm/kasan/shadow.c:156 mm/kasan/shadow.c:182)
>> [ 51.647327][ T218] ? __kasan_slab_alloc (mm/kasan/common.c:329 mm/kasan/common.c:356)
>> [ 51.647340][ T218] ? __kmalloc_cache_noprof (mm/slub.c:4180 mm/slub.c:4229 mm/slub.c:4391)
>> [ 51.647352][ T218] ? kasan_save_track (arch/x86/include/asm/current.h:25 mm/kasan/common.c:60 mm/kasan/common.c:69)
>> [ 51.647365][ T218] ? kasan_unpoison (mm/kasan/shadow.c:156 mm/kasan/shadow.c:182)
>> [ 51.647377][ T218] do_init_module (kernel/module/main.c:3039)
>> [ 51.647388][ T218] ? __pfx_do_init_module (kernel/module/main.c:3011)
>> [ 51.647402][ T218] ? kfree (mm/slub.c:4680 mm/slub.c:4879)
>> [ 51.647414][ T218] ? klp_module_coming (kernel/livepatch/core.c:1317)
>> [ 51.647426][ T218] ? load_module (kernel/module/main.c:2468 kernel/module/main.c:2463 kernel/module/main.c:3504)
>> [ 51.647441][ T218] load_module (kernel/module/main.c:3509)
>> [ 51.647449][ T218] ? ima_post_read_file (security/integrity/ima/ima_main.c:896 security/integrity/ima/ima_main.c:878)
>> [ 51.647466][ T218] ? __pfx_load_module (kernel/module/main.c:3353)
>> [ 51.647478][ T218] ? __pfx_kernel_read_file (fs/kernel_read_file.c:38)
>> [ 51.647489][ T218] ? init_module_from_file (kernel/module/main.c:3701)
>> [ 51.647499][ T218] init_module_from_file (kernel/module/main.c:3701)
>> [ 51.647514][ T218] ? __pfx_init_module_from_file (kernel/module/main.c:3677)
>> [ 51.647525][ T218] ? mm_get_unmapped_area (arch/x86/include/asm/bitops.h:206 arch/x86/include/asm/bitops.h:238 include/asm-generic/bitops/instrumented-non-atomic.h:142 mm/mmap.c:805 mm/mmap.c:871)
>> [ 51.647540][ T218] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:107 include/linux/atomic/atomic-arch-fallback.h:2170 include/linux/atomic/atomic-instrumented.h:1302 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:187 include/linux/spinlock_api_smp.h:134 kernel/locking/spinlock.c:154)
>> [ 51.647547][ T218] ? __pfx__raw_spin_lock (kernel/locking/spinlock.c:153)
>> [ 51.647560][ T218] idempotent_init_module (kernel/module/main.c:3713)
>> [ 51.647573][ T218] ? __pfx_idempotent_init_module (kernel/module/main.c:3705)
>> [ 51.647582][ T218] ? __pfx___seccomp_filter (kernel/seccomp.c:1244)
>> [ 51.647590][ T218] ? fdget (include/linux/atomic/atomic-arch-fallback.h:479 include/linux/atomic/atomic-instrumented.h:50 fs/file.c:1167 fs/file.c:1181)
>> [ 51.647607][ T218] ? security_capable (security/security.c:1142)
>> [ 51.647615][ T218] __x64_sys_finit_module (include/linux/file.h:62 include/linux/file.h:83 kernel/module/main.c:3736 kernel/module/main.c:3723 kernel/module/main.c:3723)
>> [ 51.647627][ T218] ? syscall_trace_enter (kernel/entry/syscall-common.c:44)
>> [ 51.647640][ T218] do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
>> [ 51.647657][ T218] ? fput (arch/x86/include/asm/atomic64_64.h:79 include/linux/atomic/atomic-arch-fallback.h:2913 include/linux/atomic/atomic-arch-fallback.h:3364 include/linux/atomic/atomic-long.h:698 include/linux/atomic/atomic-instrumented.h:3767 include/linux/file_ref.h:157 fs/file_table.c:544)
>> [ 51.647668][ T218] ? ksys_mmap_pgoff (mm/mmap.c:609)
>> [ 51.647682][ T218] ? do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
>> [ 51.647694][ T218] ? from_kgid_munged (kernel/user_namespace.c:535)
>> [ 51.647708][ T218] ? _copy_to_user (arch/x86/include/asm/uaccess_64.h:126 arch/x86/include/asm/uaccess_64.h:147 include/linux/uaccess.h:197 lib/usercopy.c:26)
>> [ 51.647722][ T218] ? cp_new_stat (fs/stat.c:471)
>> [ 51.647732][ T218] ? __pfx_cp_new_stat (fs/stat.c:471)
>>
>>
>> The kernel config and materials to reproduce are available at:
>> https://download.01.org/0day-ci/archive/20250820/202508200843.8b006132-lkp@intel.com
>>
>>
>>
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-09-01 11:14 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-12 14:34 [PATCH 1/2] dma-buf/sw_sync: put fence signaling into work item Christian König
2025-08-12 14:34 ` [PATCH 2/2] dma-buf: add warning when dma_fence is signaled from IOCTL Christian König
2025-08-13  8:16   ` Philipp Stanner
2025-08-13  9:18     ` Christian König
2025-08-13  8:20   ` Tvrtko Ursulin
2025-08-13 11:10     ` Christian König
2025-08-13 11:59       ` Tvrtko Ursulin
2025-08-21 12:46         ` Christian König
2025-08-22  8:08           ` Tvrtko Ursulin
2025-08-20  7:27   ` kernel test robot
2025-08-21 13:00     ` Christian König
2025-09-01 11:14       ` Tvrtko Ursulin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).