Hi Christian,
     Good suggestion. Put the barrier() before the kmem_cache_destroy() will be 
better. I will send a v2 patch later. Thanks

Best Wishes,
Emily Deng

> -----Original Message-----
> From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com]
> Sent: Thursday, May 17, 2018 4:39 PM
> To: Deng, Emily <emily.d...@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: add rcu_barrier after entity fini
> 
> Am 17.05.2018 um 05:05 schrieb Emily Deng:
> > To free the fence from the amdgpu_fence_slab, need twice call_rcu, to
> > avoid the amdgpu_fence_slab_fini call
> > kmem_cache_destroy(amdgpu_fence_slab) before
> kmem_cache_free(amdgpu_fence_slab, fence), add rcu_barrier after
> drm_sched_entity_fini.
> >
> > The kmem_cache_free(amdgpu_fence_slab, fence)'s call trace as below:
> > 1.drm_sched_entity_fini ->
> > drm_sched_entity_cleanup ->
> > dma_fence_put(entity->last_scheduled) ->
> > drm_sched_fence_release_finished ->
> drm_sched_fence_release_scheduled
> > -> call_rcu(&fence->finished.rcu, drm_sched_fence_free)
> >
> > 2.drm_sched_fence_free ->
> > dma_fence_put(fence->parent) ->
> > amdgpu_fence_release ->
> > call_rcu(&f->rcu, amdgpu_fence_free) ->
> > kmem_cache_free(amdgpu_fence_slab, fence);
> >
> > Change-Id: I8dcadd3372f97e72461bf46b41cc26d90f09b8df
> > Signed-off-by: Emily Deng <emily.d...@amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index cc3b067..07b2e10 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -134,6 +134,7 @@ static void amdgpu_ttm_global_fini(struct
> amdgpu_device *adev)
> >     if (adev->mman.mem_global_referenced) {
> >             drm_sched_entity_fini(adev->mman.entity.sched,
> >                                   &adev->mman.entity);
> > +           rcu_barrier();
> 
> Good catch, but why don't you put the barrier() before the
> kmem_cache_destroy()?
> 
> That looks like it would make more sense.
> 
> Christian.
> 
> >             mutex_destroy(&adev->mman.gtt_window_lock);
> >             drm_global_item_unref(&adev->mman.bo_global_ref.ref);
> >             drm_global_item_unref(&adev->mman.mem_global_ref);

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to