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@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.vetter@ffwll.ch
dma_fence_enable_sw_signaling(fence);
}
- dma_resv_iter_end(&cursor); rcu_read_unlock();
} -- 2.25.1