Re: [PATCH 3/5] dma-buf: return only unsignaled fences in dma_fence_unwrap_for_each v3

From: "Christian König" <christian.koenig@amd.com>
To: "Karolina Drobnik" <karolina.drobnik@intel.com>,
	"Christian König" <ckoenig.leichtzumerken@gmail.com>
Cc: linaro-mm-sig@lists.linaro.org, dri-devel@lists.freedesktop.org,
	linux-media@vger.kernel.org
Subject: Re: [PATCH 3/5] dma-buf: return only unsignaled fences in dma_fence_unwrap_for_each v3
Date: Mon, 11 Jul 2022 11:57:31 +0200	[thread overview]
Message-ID: <4c9598f0-4ba7-c18e-2ccd-f508769a72e9@amd.com> (raw)
In-Reply-To: <f67d23e2-3953-7717-9c41-65075929525c@intel.com>

Hi Karolina,

Am 11.07.22 um 11:44 schrieb Karolina Drobnik:
> Hi Christian,
>
> I'm sorry for digging this one out so late.
>
> On 06.05.2022 16:10, Christian König wrote:
>> dma_fence_chain containers cleanup signaled fences automatically, so
>> filter those out from arrays as well.
>>
>> v2: fix missing walk over the array
>> v3: massively simplify the patch and actually update the description.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   include/linux/dma-fence-unwrap.h | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/dma-fence-unwrap.h 
>> b/include/linux/dma-fence-unwrap.h
>> index e7c219da4ed7..a4d342fef8e0 100644
>> --- a/include/linux/dma-fence-unwrap.h
>> +++ b/include/linux/dma-fence-unwrap.h
>> @@ -43,9 +43,13 @@ struct dma_fence *dma_fence_unwrap_next(struct 
>> dma_fence_unwrap *cursor);
>>    * Unwrap dma_fence_chain and dma_fence_array containers and deep 
>> dive into all
>>    * potential fences in them. If @head is just a normal fence only 
>> that one is
>>    * returned.
>> + *
>> + * Note that signalled fences are opportunistically filtered out, which
>> + * means the iteration is potentially over no fence at all.
>>    */
>>   #define dma_fence_unwrap_for_each(fence, cursor, head)            \
>>       for (fence = dma_fence_unwrap_first(head, cursor); fence;    \
>> -         fence = dma_fence_unwrap_next(cursor))
>> +         fence = dma_fence_unwrap_next(cursor))            \
>> +        if (!dma_fence_is_signaled(fence))
>>     #endif
>
> It looks like this particular patch affects merging Sync Fences, which 
> is reflected by failing IGT test (igt@sw_sync)[1]. The failing 
> subtests are:
>   - sync_merge - merging different fences on the same timeline, neither
>          single nor merged fences are signaled
>
>   - sync_merge_same - merging the fence with itself on the same
>          timeline, the fence didn't signal at all
>
>   - sync_multi_timeline_wait - merging different fences on different
>          timelines; the subtest checks if counting fences of
>          various states works. Currently, it can only see 2
>          active fences, 0 signaling (should be 2 active,
>          1 signaling)
>
> Reverting this commit on the top of drm-tip fixes the issue, but I'm 
> not sure if it wouldn't impact other places in the code. Please let me 
> know if I can be of any help.

Thanks for letting me know. Not sure what's going on here, but I can 
take a look today if time permits.

Do you have a description how to easy reproduce this? E.g. how to run 
just those specific igts?

Thanks,
Christian.

>
> All the best,
> Karolina
>
> ---------------------
> [1] - reproducible locally, but can be also seen in the CI:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fintel-gfx-ci.01.org%2Ftree%2Fdrm-tip%2Findex.html%3Ftestfilter%3Digt%40sw_sync&amp;data=05%7C01%7Cchristian.koenig%40amd.com%7C2af59c808f664f0cf04908da6321e708%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637931294507736831%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=NTI04OpP7kMsCu%2BDWsJ0%2FRIVJGJbxy36tJBImD2MQDU%3D&amp;reserved=0 
>