>Please add some spaces in "src?&src->refcount:NULL" after each operator.

>With that fixed the patch is Reviewed-by: Christian König 
><christian.koe...@amd.com>.

Actually this " src?&src->refcount:NULL" is totally removed in second cleanup 
patch ... 

BR Monk

-----Original Message-----
From: Christian König [mailto:deathsim...@vodafone.de] 
Sent: Tuesday, August 8, 2017 3:44 PM
To: Liu, Monk <monk....@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH libdrm 1/2] drm:fix race issue between two bo functions(v2)

Am 08.08.2017 um 09:34 schrieb Monk Liu:
> From: Monk Liu <monk....@amd.com>
>
> there is race issue between two threads on amdgpu_bo_reference and 
> amdgpu_bo_import, this patch tends to fix it by moving the 
> pthread_mutex_lock out of bo_free_internal and move to bo_reference to 
> cover the update_reference part.
>
> The mutex_unlock in bo_import should also cover bo refcount 
> increasement.
>
> Change-Id: I1f65eacf74cd28cc0d3a71ef2f7a19b890d63c29
> Signed-off-by: Monk Liu <monk....@amd.com>
> ---
>   amdgpu/amdgpu_bo.c       |  5 +----
>   amdgpu/amdgpu_internal.h | 15 ++++++++++++---
>   2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c index 
> 07eb743..82f38c0 100644
> --- a/amdgpu/amdgpu_bo.c
> +++ b/amdgpu/amdgpu_bo.c
> @@ -55,14 +55,12 @@ static void amdgpu_close_kms_handle(amdgpu_device_handle 
> dev,
>   void amdgpu_bo_free_internal(amdgpu_bo_handle bo)
>   {
>       /* Remove the buffer from the hash tables. */
> -     pthread_mutex_lock(&bo->dev->bo_table_mutex);
>       util_hash_table_remove(bo->dev->bo_handles,
>                              (void*)(uintptr_t)bo->handle);
>       if (bo->flink_name) {
>               util_hash_table_remove(bo->dev->bo_flink_names,
>                                      (void*)(uintptr_t)bo->flink_name);
>       }
> -     pthread_mutex_unlock(&bo->dev->bo_table_mutex);
>   
>       /* Release CPU access. */
>       if (bo->cpu_map_count > 0) {
> @@ -340,10 +338,9 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
>       }
>   
>       if (bo) {
> -             pthread_mutex_unlock(&dev->bo_table_mutex);
> -
>               /* The buffer already exists, just bump the refcount. */
>               atomic_inc(&bo->refcount);
> +             pthread_mutex_unlock(&dev->bo_table_mutex);
>   
>               output->buf_handle = bo;
>               output->alloc_size = bo->alloc_size; diff --git 
> a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h index 
> 0508131..79da0e7 100644
> --- a/amdgpu/amdgpu_internal.h
> +++ b/amdgpu/amdgpu_internal.h
> @@ -143,7 +143,7 @@ void amdgpu_vamgr_deinit(struct amdgpu_bo_va_mgr *mgr);
>   uint64_t amdgpu_vamgr_find_va(struct amdgpu_bo_va_mgr *mgr, uint64_t size,
>                               uint64_t alignment, uint64_t base_required);
>   
> -void amdgpu_vamgr_free_va(struct amdgpu_bo_va_mgr *mgr, uint64_t va,
> +void amdgpu_vamgr_free_va(struct amdgpu_bo_va_mgr *mgr, uint64_t va,
>                               uint64_t size);
>   
>   int amdgpu_query_gpu_info_init(amdgpu_device_handle dev); @@ -193,8 
> +193,17 @@ static inline bool update_references(atomic_t *dst, atomic_t *src)
>   static inline void amdgpu_bo_reference(struct amdgpu_bo **dst,
>                                       struct amdgpu_bo *src)
>   {
> -     if (update_references(&(*dst)->refcount, &src->refcount))
> -             amdgpu_bo_free_internal(*dst);
> +     pthread_mutex_t *mlock;
> +     struct amdgpu_bo* bo = *dst;
> +
> +     assert(bo != NULL);
> +     mlock = &bo->dev->bo_table_mutex;
> +     pthread_mutex_lock(mlock);
> +
> +     if (update_references(&bo->refcount, src?&src->refcount:NULL))

Please add some spaces in "src?&src->refcount:NULL" after each operator.

With that fixed the patch is Reviewed-by: Christian König 
<christian.koe...@amd.com>.

Regards,
Christian.

> +             amdgpu_bo_free_internal(bo);
> +
> +     pthread_mutex_unlock(mlock);
>       *dst = src;
>   }
>   


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

Reply via email to