Re: [PATCH 1/7] dma-buf: add WARN_ON() illegal dma-fence signaling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 11.09.24 um 11:44 schrieb Philipp Stanner:
On Wed, 2024-09-11 at 10:58 +0200, Christian König wrote:
Calling the signaling a NULL fence is obviously a coding error in a
driver. Those functions unfortunately just returned silently without
raising a warning.
Good catch

Signed-off-by: Christian König <christian.koenig@xxxxxxx>
---
  drivers/dma-buf/dma-fence.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-
fence.c
index 0393a9bba3a8..325a263ac798 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -412,7 +412,7 @@ int dma_fence_signal_timestamp(struct dma_fence
*fence, ktime_t timestamp)
  	unsigned long flags;
  	int ret;
- if (!fence)
+	if (WARN_ON(!fence))
  		return -EINVAL;
While one can do that, as far as I can see there are only a hand full
of users of that function anyways.

The dma_fence_signal() function has tons of users, it's basically the core of the DMA-buf framework.

Couldn't one (additionally) add the error check of
dma_fenc_signal_timestapm() to those? Like in
dma_fenc_allocate_private_stub().

It seems some of them are void functions, though. Hm.
There is also the attribute __must_check that could be considered now
or in the future for such functions.

I actually want to remove the error return from dma_fence_signal() and the other variants. There is no valid reason that those functions should fail.

The only user is some obscure use case in AMDs KFD driver and I would rather like to clean that one up.

Regards,
Christian.


Regards,
P.


  spin_lock_irqsave(fence->lock, flags);
@@ -464,7 +464,7 @@ int dma_fence_signal(struct dma_fence *fence)
  	int ret;
  	bool tmp;
- if (!fence)
+	if (WARN_ON(!fence))
  		return -EINVAL;
  tmp = dma_fence_begin_signalling();




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux