On Fri, Sep 17, 2021 at 02:34:54PM +0200, Christian König wrote:
> This is probably a fix since we didn't even grabed a reference to the
> fences.

It's rcu protected, and we only care about speeding things up a bit. I
think this wont have any impact on correctness, and I don't think any
driver could blow up?

But yeah maybe we should have a few assert sprinkled into various
dma_fence functions to make sure we never call them when the refcount has
dropped to 0. That would catch stuff like this, and help lock down the
dma-fence api quite a bit.
> 
> Signed-off-by: Christian König <christian.koe...@amd.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 3b22c0013dbf..7d804c0c69b0 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -269,22 +269,16 @@ static int ttm_bo_individualize_resv(struct 
> ttm_buffer_object *bo)
>  static void ttm_bo_flush_all_fences(struct ttm_buffer_object *bo)
>  {
>       struct dma_resv *resv = &bo->base._resv;
> -     struct dma_resv_list *fobj;
> +     struct dma_resv_iter cursor;
>       struct dma_fence *fence;
> -     int i;
>  
>       rcu_read_lock();
> -     fobj = dma_resv_shared_list(resv);
> -     fence = dma_resv_excl_fence(resv);
> -     if (fence && !fence->ops->signaled)
> -             dma_fence_enable_sw_signaling(fence);
> -
> -     for (i = 0; fobj && i < fobj->shared_count; ++i) {
> -             fence = rcu_dereference(fobj->shared[i]);
> -
> +     dma_resv_iter_begin(&cursor, resv, true);
> +     dma_resv_for_each_fence_unlocked(&cursor, fence) {
>               if (!fence->ops->signaled)

Imo delete this check here. If that really matters for performance we
should have it in the core dma_fence function, not replicated all over the
place like this. Noodling around in dma_fence internals like this isn't
cool.

With that removal included:

Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>

>                       dma_fence_enable_sw_signaling(fence);
>       }
> +     dma_resv_iter_end(&cursor);
>       rcu_read_unlock();
>  }
>  
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Reply via email to