On Wed, May 04, 2022 at 02:22:54PM +0200, Christian König wrote:
> dma_fence_chain containers cleanup signaled fences automatically, so
> filter those out from arrays as well.
>
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> ---
> drivers/dma-buf/dma-fence-unwrap.c | 23 ++++++++++++++++-------
> include/linux/dma-fence-unwrap.h | 4 ++--
> 2 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c
> index 711be125428c..7b0b91086ded 100644
> --- a/drivers/dma-buf/dma-fence-unwrap.c
> +++ b/drivers/dma-buf/dma-fence-unwrap.c
> @@ -32,8 +32,13 @@ __dma_fence_unwrap_array(struct dma_fence_unwrap *cursor)
> struct dma_fence *dma_fence_unwrap_first(struct dma_fence *head,
> struct dma_fence_unwrap *cursor)
> {
> + struct dma_fence *tmp;
> +
> cursor->chain = dma_fence_get(head);
> - return __dma_fence_unwrap_array(cursor);
> + tmp = __dma_fence_unwrap_array(cursor);
> + if (tmp && dma_fence_is_signaled(tmp))
> + tmp = dma_fence_unwrap_next(cursor);
> + return tmp;
> }
> EXPORT_SYMBOL_GPL(dma_fence_unwrap_first);
>
> @@ -48,12 +53,16 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor)
> {
> struct dma_fence *tmp;
>
> - ++cursor->index;
> - tmp = dma_fence_array_next(cursor->array, cursor->index);
> - if (tmp)
> - return tmp;
> + do {
> + ++cursor->index;
> + tmp = dma_fence_array_next(cursor->array, cursor->index);
> + if (tmp && !dma_fence_is_signaled(tmp))
> + return tmp;
Don't do need a do {} while here too to first walk through the array
before going to the next one in the chain? Maybe add a testcase for this?
> +
> + cursor->chain = dma_fence_chain_walk(cursor->chain);
> + tmp = __dma_fence_unwrap_array(cursor);
> + } while (tmp && dma_fence_is_signaled(tmp));
>
> - cursor->chain = dma_fence_chain_walk(cursor->chain);
> - return __dma_fence_unwrap_array(cursor);
> + return tmp;
> }
> EXPORT_SYMBOL_GPL(dma_fence_unwrap_next);
> diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h
> index e7c219da4ed7..e9d114637294 100644
> --- a/include/linux/dma-fence-unwrap.h
> +++ b/include/linux/dma-fence-unwrap.h
> @@ -41,8 +41,8 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor);
> * @head: starting point for the iterator
> *
> * 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.
> + * potential none signaled fences in them. If @head is just a normal fence only
> + * that one is returned.
I think I get what you want to say, but it reads garbled. What about
leaving the current text as-is and adding something like
"Note that signalled fences are opportunistically filtered out, which
means the iteration is potentially over no fence at all"
Or something like that? I think smashing this all into one sentence
doesn't work well.
Then please also add this same sentence to unwrap_first/next() for
completeness.
-Daniel
> */
> #define dma_fence_unwrap_for_each(fence, cursor, head) \
> for (fence = dma_fence_unwrap_first(head, cursor); fence; \
> --
> 2.25.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
