AI/rocm-systems
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libhsakmt: Fix deadlocks in __fmm_release

Browse Source 
__fmm_release is sometimes called with the aperture lock, and sometimes
without. Consistently call it with the aperture lock held and remove the
lock/unlock calls from this function.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
Change-Id: I80dddc64cc0703e5eed8e9f1eb65b75a2c7ae2eb


[ROCm/ROCR-Runtime commit: 5fac7dcc3b]
This commit is contained in:
Felix Kuehling
2021-07-09 20:14:23 -04:00
parent 559bb50c6a
commit 14d821391e
1 changed files with 24 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
projects/rocr-runtime/src/fmm.c
+24 -29
View File
@@ -287,6 +287,9 @@ static inline HsaSharedMemoryHandle *to_hsa_shared_memory_handle(
extern int debug_get_reg_status(uint32_t node_id, bool *is_debugged);
static int __fmm_release(vm_object_t *object, manageable_aperture_t *aperture);
static int _fmm_map_to_gpu(manageable_aperture_t *aperture,
void *address, uint64_t size, vm_object_t *obj,
uint32_t *nodes_to_map, uint32_t nodes_array_size);
static int _fmm_unmap_from_gpu_scratch(uint32_t gpu_id,
manageable_aperture_t *aperture,
void *address);
@@ -1345,8 +1348,6 @@ void *fmm_allocate_device(uint32_t gpu_id, void *address, uint64_t MemorySizeInB
gpuid_to_nodeid(gpu_id, &vm_obj->node_id);
}
pthread_mutex_unlock(&aperture->fmm_mutex);
if (mem) {
int map_fd = gpu_mem[gpu_mem_id].drm_render_fd;
int prot = flags.ui32.HostAccess ? PROT_READ | PROT_WRITE :
@@ -1358,7 +1359,8 @@ void *fmm_allocate_device(uint32_t gpu_id, void *address, uint64_t MemorySizeInB
if (ret == MAP_FAILED) {
__fmm_release(vm_obj, aperture);
return NULL;
mem = NULL;
goto out_unlock;
}
/*
* This madvise() call is needed to avoid additional references
@@ -1369,6 +1371,9 @@ void *fmm_allocate_device(uint32_t gpu_id, void *address, uint64_t MemorySizeInB
madvise(mem, MemorySizeInBytes, MADV_DONTFORK);
}
out_unlock:
pthread_mutex_unlock(&aperture->fmm_mutex);
return mem;
}
@@ -1410,8 +1415,6 @@ void *fmm_allocate_doorbell(uint32_t gpu_id, uint64_t MemorySizeInBytes,
gpuid_to_nodeid(gpu_id, &vm_obj->node_id);
}
pthread_mutex_unlock(&aperture->fmm_mutex);
if (mem) {
void *ret = mmap(mem, MemorySizeInBytes,
PROT_READ | PROT_WRITE,
@@ -1419,10 +1422,12 @@ void *fmm_allocate_doorbell(uint32_t gpu_id, uint64_t MemorySizeInBytes,
doorbell_mmap_offset);
if (ret == MAP_FAILED) {
__fmm_release(vm_obj, aperture);
return NULL;
mem = NULL;
}
}
pthread_mutex_unlock(&aperture->fmm_mutex);
return mem;
}
@@ -1652,23 +1657,18 @@ static int __fmm_release(vm_object_t *object, manageable_aperture_t *aperture)
if (!object)
return -EINVAL;
pthread_mutex_lock(&aperture->fmm_mutex);
/* If memory is user memory and it's still GPU mapped, munmap
* would cause an eviction. If the restore happens quickly
* enough, restore would also fail with an error message. So
* free the BO before unmapping the pages.
*/
args.handle = object->handle;
if (kmtIoctl(kfd_fd, AMDKFD_IOC_FREE_MEMORY_OF_GPU, &args)) {
pthread_mutex_unlock(&aperture->fmm_mutex);
if (kmtIoctl(kfd_fd, AMDKFD_IOC_FREE_MEMORY_OF_GPU, &args))
return -errno;
}
aperture_release_area(aperture, object->start, object->size);
vm_remove_object(aperture, object);
pthread_mutex_unlock(&aperture->fmm_mutex);
return 0;
}
@@ -1701,9 +1701,10 @@ HSAKMT_STATUS fmm_release(void *address)
pthread_mutex_unlock(&aperture->fmm_mutex);
munmap(address, size);
} else {
pthread_mutex_unlock(&aperture->fmm_mutex);
int r = __fmm_release(object, aperture);
if (__fmm_release(object, aperture))
pthread_mutex_unlock(&aperture->fmm_mutex);
if (r)
return HSAKMT_STATUS_ERROR;
if (!aperture->is_cpu_accessible)
@@ -2088,7 +2089,6 @@ static void *map_mmio(uint32_t node_id, uint32_t gpu_id, int mmap_fd)
flags.ui32.Reserved = 0;
vm_obj->flags = flags.Value;
vm_obj->node_id = node_id;
pthread_mutex_unlock(&aperture->fmm_mutex);
/* Map for CPU access*/
ret = mmap(mem, PAGE_SIZE,
@@ -2097,14 +2097,14 @@ static void *map_mmio(uint32_t node_id, uint32_t gpu_id, int mmap_fd)
mmap_offset);
if (ret == MAP_FAILED) {
__fmm_release(vm_obj, aperture);
return NULL;
mem = NULL;
/* Map for GPU access*/
} else if (_fmm_map_to_gpu(aperture, mem, PAGE_SIZE, vm_obj, NULL, 0)) {
__fmm_release(vm_obj, aperture);
mem = NULL;
}
/* Map for GPU access*/
if (fmm_map_to_gpu(mem, PAGE_SIZE, NULL)) {
__fmm_release(vm_obj, aperture);
return NULL;
}
pthread_mutex_unlock(&aperture->fmm_mutex);
return mem;
}
@@ -2888,13 +2888,8 @@ static int _fmm_unmap_from_gpu_scratch(uint32_t gpu_id,
free(object->mapped_node_id_array);
object->mapped_node_id_array = NULL;
if (ret)
goto err;
pthread_mutex_unlock(&aperture->fmm_mutex);
/* free object in scratch backing aperture */
return __fmm_release(object, aperture);
if (!ret)
ret = __fmm_release(object, aperture);
err:
pthread_mutex_unlock(&aperture->fmm_mutex);
@@ -3398,8 +3393,8 @@ HSAKMT_STATUS fmm_deregister_memory(void *address)
* buffer. Deregistering imported graphics buffers or
* userptrs means releasing the BO.
*/
pthread_mutex_unlock(&aperture->fmm_mutex);
__fmm_release(object, aperture);
pthread_mutex_unlock(&aperture->fmm_mutex);
return HSAKMT_STATUS_SUCCESS;
}