| Message ID | 20210917123513.1106-8-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/26] dma-buf: add dma_resv_for_each_fence_unlocked v2 | expand |
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 >
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) dma_fence_enable_sw_signaling(fence); } + dma_resv_iter_end(&cursor); rcu_read_unlock(); }
This is probably a fix since we didn't even grabed a reference to the fences. 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(-)