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.koe...@amd.com>
---
  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();

Reply via email to