On Tue, 12 Jul 2022 at 12:28, Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
>
> This reverts commit 8f61973718485f3e89bc4f408f929048b7b47c83.
>
> It turned out that this is not correct. Especially the sync_file info
> IOCTL needs to see even signaled fences to correctly report back their
> status to userspace.
>
> Instead add the filter in the merge function again where it makes sense.
>
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> ---
> drivers/dma-buf/dma-fence-unwrap.c | 3 ++-
> include/linux/dma-fence-unwrap.h | 6 +-----
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c
> index 502a65ea6d44..7002bca792ff 100644
> --- a/drivers/dma-buf/dma-fence-unwrap.c
> +++ b/drivers/dma-buf/dma-fence-unwrap.c
> @@ -72,7 +72,8 @@ struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences,
> count = 0;
> for (i = 0; i < num_fences; ++i) {
> dma_fence_unwrap_for_each(tmp, &iter[i], fences[i])
> - ++count;
> + if (!dma_fence_is_signaled(tmp))
> + ++count;
> }
>
> if (count == 0)
> diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h
> index 390de1ee9d35..66b1e56fbb81 100644
> --- a/include/linux/dma-fence-unwrap.h
> +++ b/include/linux/dma-fence-unwrap.h
> @@ -43,14 +43,10 @@ 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)) \
> - if (!dma_fence_is_signaled(fence))
> + fence = dma_fence_unwrap_next(cursor))
Not sure it's worth it, but could we still filter but keep the fence
if there's an error?
I'm honestly also not entirely sure whether error propagation is a
terrific idea, since every single use-case I've seen in i915 was a
mis-design and not good at all. So burning it all down and declaring
the testcases invalid might be the right thing to do here.
-Daniel
>
> struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences,
> struct dma_fence **fences,
> --
> 2.25.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
