From: Monk Liu <monk.liu@xxxxxxx> there is race issue between two threads on amdgpu_bo_free and amdgpu_bo_import, this patch tend to fix it by moving the pthread_mutex_lock out of bo_internal and cover the update_reference function. besides the mutex_unlock in bo_import should also cover bo refcount increasement. Change-Id: I8ec5a2879dda0e6468864fc64e6b46059482998b Find-by: Deng hui <hui.deng at amd.com> Signed-off-by: Monk Liu <monk.liu at amd.com> --- amdgpu/amdgpu_bo.c | 13 +++++-------- amdgpu/amdgpu_internal.h | 17 +++++++++++++++-- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c index ec99488..10a3efe 100644 --- a/amdgpu/amdgpu_bo.c +++ b/amdgpu/amdgpu_bo.c @@ -52,27 +52,22 @@ static void amdgpu_close_kms_handle(amdgpu_device_handle dev, drmIoctl(dev->fd, DRM_IOCTL_GEM_CLOSE, &args); } -void amdgpu_bo_free_internal(amdgpu_bo_handle bo) +drm_private 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) { bo->cpu_map_count = 1; amdgpu_bo_cpu_unmap(bo); } - amdgpu_close_kms_handle(bo->dev, bo->handle); - pthread_mutex_destroy(&bo->cpu_access_mutex); - free(bo); } int amdgpu_bo_alloc(amdgpu_device_handle dev, @@ -297,6 +292,7 @@ int amdgpu_bo_import(amdgpu_device_handle dev, /* Get a KMS handle. */ r = drmPrimeFDToHandle(dev->fd, shared_handle, &handle); if (r) { + pthread_mutex_unlock(&dev->bo_table_mutex); return r; } @@ -339,10 +335,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; @@ -466,6 +461,8 @@ int amdgpu_bo_cpu_map(amdgpu_bo_handle bo, void **cpu) bo->cpu_map_count = 1; pthread_mutex_unlock(&bo->cpu_access_mutex); + amdgpu_add_handle_to_table(bo); + *cpu = ptr; return 0; } diff --git a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h index 0508131..e1a6559 100644 --- a/amdgpu/amdgpu_internal.h +++ b/amdgpu/amdgpu_internal.h @@ -30,6 +30,9 @@ #include <assert.h> #include <pthread.h> +#include <stdlib.h> + +#include "libdrm_macros.h" #include "xf86atomic.h" #include "amdgpu.h" #include "util_double_list.h" @@ -193,8 +196,18 @@ 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); + struct amdgpu_bo* bo = *dst; + assert(bo!=NULL); + + pthread_mutex_lock(&bo->dev->bo_table_mutex); + + if (update_references(&bo->refcount, src?&src->refcount:NULL)) { + amdgpu_bo_free_internal(bo); + pthread_mutex_destroy(&bo->cpu_access_mutex); + pthread_mutex_unlock(&bo->dev->bo_table_mutex); + free(bo); + } else + pthread_mutex_unlock(&bo->dev->bo_table_mutex); *dst = src; } -- 2.7.4