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();