diff mbox series

[3/3] dma-buf: Check status of enable-signaling bit on debug

Message ID 20220927172409.484061-4-Arvind.Yadav@amd.com (mailing list archive)
State New, archived
Headers show
Series dma-buf: Check status of enable-signaling bit on debug | expand

Commit Message

Arvind Yadav Sept. 27, 2022, 5:24 p.m. UTC
Fence signaling must be enabled to make sure that
the dma_fence_is_signaled_locked() function ever returns true.
Since drivers and implementations sometimes mess this up,
this ensures correct behaviour when DMABUF_DEBUG_ENABLE_SIGNALING
is used during debugging.
This should make any implementation bugs resulting in not
signaled fences much more obvious.

Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
---
 include/linux/dma-fence.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Christian König Sept. 29, 2022, 6:18 p.m. UTC | #1
Am 27.09.22 um 19:24 schrieb Arvind Yadav:
> Fence signaling must be enabled to make sure that
> the dma_fence_is_signaled_locked() function ever returns true.
> Since drivers and implementations sometimes mess this up,
> this ensures correct behaviour when DMABUF_DEBUG_ENABLE_SIGNALING
> is used during debugging.
> This should make any implementation bugs resulting in not
> signaled fences much more obvious.

Are all IGT tests now passing with this? That would be a bit unfortunate 
because it means we still have missed the bug in drm_syncobj.

Christian.

>
> Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
> ---
>   include/linux/dma-fence.h | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 775cdc0b4f24..5156dc6be0a6 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -398,6 +398,11 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence);
>   static inline bool
>   dma_fence_is_signaled_locked(struct dma_fence *fence)
>   {
> +#ifdef CONFIG_DMABUF_DEBUG_ENABLE_SIGNALING
> +	if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
> +		return false;
> +#endif
> +
>   	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>   		return true;
>
Yadav, Arvind Sept. 29, 2022, 6:30 p.m. UTC | #2
On 9/29/2022 11:48 PM, Christian König wrote:
> Am 27.09.22 um 19:24 schrieb Arvind Yadav:
>> Fence signaling must be enabled to make sure that
>> the dma_fence_is_signaled_locked() function ever returns true.
>> Since drivers and implementations sometimes mess this up,
>> this ensures correct behaviour when DMABUF_DEBUG_ENABLE_SIGNALING
>> is used during debugging.
>> This should make any implementation bugs resulting in not
>> signaled fences much more obvious.
>
> Are all IGT tests now passing with this? That would be a bit 
> unfortunate because it means we still have missed the bug in drm_syncobj.
>
IGT has these test cases related to syncobj (syncobj_basic, 
syncobj_timeline and syncobj_wait)and all are passing.

I will check syncobj and let you know.

~Arvind

> Christian.
>
>>
>> Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
>> ---
>>   include/linux/dma-fence.h | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>> index 775cdc0b4f24..5156dc6be0a6 100644
>> --- a/include/linux/dma-fence.h
>> +++ b/include/linux/dma-fence.h
>> @@ -398,6 +398,11 @@ void dma_fence_enable_sw_signaling(struct 
>> dma_fence *fence);
>>   static inline bool
>>   dma_fence_is_signaled_locked(struct dma_fence *fence)
>>   {
>> +#ifdef CONFIG_DMABUF_DEBUG_ENABLE_SIGNALING
>> +    if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
>> +        return false;
>> +#endif
>> +
>>       if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>>           return true;
>
Christian König Sept. 29, 2022, 6:32 p.m. UTC | #3
Am 29.09.22 um 20:30 schrieb Yadav, Arvind:
>
> On 9/29/2022 11:48 PM, Christian König wrote:
>> Am 27.09.22 um 19:24 schrieb Arvind Yadav:
>>> Fence signaling must be enabled to make sure that
>>> the dma_fence_is_signaled_locked() function ever returns true.
>>> Since drivers and implementations sometimes mess this up,
>>> this ensures correct behaviour when DMABUF_DEBUG_ENABLE_SIGNALING
>>> is used during debugging.
>>> This should make any implementation bugs resulting in not
>>> signaled fences much more obvious.
>>
>> Are all IGT tests now passing with this? That would be a bit 
>> unfortunate because it means we still have missed the bug in 
>> drm_syncobj.
>>
> IGT has these test cases related to syncobj (syncobj_basic, 
> syncobj_timeline and syncobj_wait)and all are passing.
>
> I will check syncobj and let you know.

Maybe CC the Intel list and let their CI systems take a look. That's 
usually rather valuable.

Thanks,
Christian.

>
> ~Arvind
>
>> Christian.
>>
>>>
>>> Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
>>> ---
>>>   include/linux/dma-fence.h | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>> index 775cdc0b4f24..5156dc6be0a6 100644
>>> --- a/include/linux/dma-fence.h
>>> +++ b/include/linux/dma-fence.h
>>> @@ -398,6 +398,11 @@ void dma_fence_enable_sw_signaling(struct 
>>> dma_fence *fence);
>>>   static inline bool
>>>   dma_fence_is_signaled_locked(struct dma_fence *fence)
>>>   {
>>> +#ifdef CONFIG_DMABUF_DEBUG_ENABLE_SIGNALING
>>> +    if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
>>> +        return false;
>>> +#endif
>>> +
>>>       if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>>>           return true;
>>
Yadav, Arvind Sept. 30, 2022, 7:32 a.m. UTC | #4
On 9/30/2022 12:02 AM, Christian König wrote:
> Am 29.09.22 um 20:30 schrieb Yadav, Arvind:
>>
>> On 9/29/2022 11:48 PM, Christian König wrote:
>>> Am 27.09.22 um 19:24 schrieb Arvind Yadav:
>>>> Fence signaling must be enabled to make sure that
>>>> the dma_fence_is_signaled_locked() function ever returns true.
>>>> Since drivers and implementations sometimes mess this up,
>>>> this ensures correct behaviour when DMABUF_DEBUG_ENABLE_SIGNALING
>>>> is used during debugging.
>>>> This should make any implementation bugs resulting in not
>>>> signaled fences much more obvious.
>>>
>>> Are all IGT tests now passing with this? That would be a bit 
>>> unfortunate because it means we still have missed the bug in 
>>> drm_syncobj.
>>>
>> IGT has these test cases related to syncobj (syncobj_basic, 
>> syncobj_timeline and syncobj_wait)and all are passing.
>>
>> I will check syncobj and let you know.
>
> Maybe CC the Intel list and let their CI systems take a look. That's 
> usually rather valuable.

There is one IGT subtest is failing which is related to syncobj. I will 
fix this and submit the patch.

  igt_subtest("host-signal-points")
                 test_host_signal_points(fd);

>
> Thanks,
> Christian.
>
>>
>> ~Arvind
>>
>>> Christian.
>>>
>>>>
>>>> Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
>>>> ---
>>>>   include/linux/dma-fence.h | 5 +++++
>>>>   1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>>> index 775cdc0b4f24..5156dc6be0a6 100644
>>>> --- a/include/linux/dma-fence.h
>>>> +++ b/include/linux/dma-fence.h
>>>> @@ -398,6 +398,11 @@ void dma_fence_enable_sw_signaling(struct 
>>>> dma_fence *fence);
>>>>   static inline bool
>>>>   dma_fence_is_signaled_locked(struct dma_fence *fence)
>>>>   {
>>>> +#ifdef CONFIG_DMABUF_DEBUG_ENABLE_SIGNALING
>>>> +    if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
>>>> +        return false;
>>>> +#endif
>>>> +
>>>>       if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>>>>           return true;
>>>
>
diff mbox series

Patch

diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 775cdc0b4f24..5156dc6be0a6 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -398,6 +398,11 @@  void dma_fence_enable_sw_signaling(struct dma_fence *fence);
 static inline bool
 dma_fence_is_signaled_locked(struct dma_fence *fence)
 {
+#ifdef CONFIG_DMABUF_DEBUG_ENABLE_SIGNALING
+	if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags))
+		return false;
+#endif
+
 	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
 		return true;