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.koenig@xxxxxxx>
> ---
> 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.vetter@xxxxxxxx>
> 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
