On Thu, 2024-09-12 at 16:55 +0200, Christian König wrote: > 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. I meant dma_fence_signal_timestamp() itself. > > > 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. Makes sense to me. +1 P. > > 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(); >
