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.leichtzumerken at gmail.com]
> Sent: Thursday, May 17, 2018 4:39 PM
> To: Deng, Emily <Emily.Deng at amd.com>; amd-gfx at 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.Deng at 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);
