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

Christian König ckoenig.leichtzumerken at gmail.com
Thu Sep 12 14:55:42 UTC 2024


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



More information about the dri-devel mailing list