Re: [Linaro-mm-sig] [PATCH 06/26] dma-buf: use new iterator in dma_resv_test_signaled

On Fri, Sep 17, 2021 at 02:34:53PM +0200, Christian König wrote:

This makes the function much simpler since the complex retry logic is now handled elsewhere.

Signed-off-by: Christian König christian.koenig@amd.com

drivers/dma-buf/dma-resv.c | 56 ++++++-------------------------------- 1 file changed, 9 insertions(+), 47 deletions(-)

diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index c7db553ab115..d8f428ddaedd 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -593,22 +593,6 @@ long dma_resv_wait_timeout(struct dma_resv *obj, bool wait_all, bool intr, EXPORT_SYMBOL_GPL(dma_resv_wait_timeout); -static inline int dma_resv_test_signaled_single(struct dma_fence *passed_fence) -{

• struct dma_fence *fence, *lfence = passed_fence;
• int ret = 1;
• if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &lfence->flags)) {

• fence = dma_fence_get_rcu(lfence);
  

• if (!fence)
  

• 	return -1;
  

• ret = !!dma_fence_is_signaled(fence);
  

• dma_fence_put(fence);
  

• }
• return ret;

-}

/**

• dma_resv_test_signaled - Test if a reservation object's fences have been
• signaled.

@@ -625,43 +609,21 @@ static inline int dma_resv_test_signaled_single(struct dma_fence *passed_fence) */ bool dma_resv_test_signaled(struct dma_resv *obj, bool test_all) {

• struct dma_resv_iter cursor; struct dma_fence *fence;

• unsigned int seq;
• int ret;

rcu_read_lock(); -retry:

• ret = true;
• seq = read_seqcount_begin(&obj->seq);
• if (test_all) {

• struct dma_resv_list *fobj = dma_resv_shared_list(obj);
  

• unsigned int i, shared_count;
  

• shared_count = fobj ? fobj->shared_count : 0;
  

• for (i = 0; i < shared_count; ++i) {
  

• 	fence = rcu_dereference(fobj->shared[i]);
  

• 	ret = dma_resv_test_signaled_single(fence);
  

• 	if (ret < 0)
  

• 		goto retry;
  

• 	else if (!ret)
  

• 		break;
  

• dma_resv_iter_begin(&cursor, obj, test_all);
• dma_resv_for_each_fence_unlocked(&cursor, fence) {

• if (!dma_fence_is_signaled(fence)) {
  

Should we be extremely clever and document that the iterator already filters out unsignalled fences? We could rely on that here :-) Otoh we don't want to make the full is_signalled check in that iterator, so this makes sense.

Again rcu_read_lock into the iterators pls. With that:

Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch

• 	dma_resv_iter_end(&cursor);
  

• 	rcu_read_unlock();
  

• 	return false;
  

  } }

• fence = dma_resv_excl_fence(obj);
• if (ret && fence) {

• ret = dma_resv_test_signaled_single(fence);
  

• if (ret < 0)
  

• 	goto retry;
  

• }
• if (read_seqcount_retry(&obj->seq, seq))

• goto retry;
  

• dma_resv_iter_end(&cursor); rcu_read_unlock();

• return ret;

• return true;

} EXPORT_SYMBOL_GPL(dma_resv_test_signaled); -- 2.25.1