Re: [PATCH libdrm 2/3] amdgpu: don't mess with shared_handle if amdgpu_bo_import() fails
On 23 January 2017 at 16:14, Nicolai Hähnlewrote: > I don't think is correct. The incoming handle is in shared_handle, not in > handle. Once the code block around line 310 has executed, shared_handle is > the handle produced by drmPrimeFDToHandle, and closing it on error (as the > code currently does) should be the correct thing to do. > I've got confused with the shared_handle <> handle naming - read the wrong one where I shouldn't have ;-) You're spot on. Thanks for the correction. Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 2/3] amdgpu: don't mess with shared_handle if amdgpu_bo_import() fails
I don't think is correct. The incoming handle is in shared_handle, not in handle. Once the code block around line 310 has executed, shared_handle is the handle produced by drmPrimeFDToHandle, and closing it on error (as the code currently does) should be the correct thing to do. The only possible issue is that I'm seeing is that perhaps the handle should be closed _before_ the unlock. Maybe someone else knows the details. Cheers, Nicolai On 22.01.2017 19:48, Emil Velikov wrote: Do not close the handle if someone else has created it. Afaict there's no change of ownership implied if the function fails. Thus the caller is responsible to doing the right thing - trying again, closing the handle and/or other. Cc: amd-...@lists.freedesktop.org Signed-off-by: Emil Velikov--- Not 100% sure if it's the correct thing. --- amdgpu/amdgpu_bo.c | 4 1 file changed, 4 deletions(-) diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c index c9f31587..24fa8880 100644 --- a/amdgpu/amdgpu_bo.c +++ b/amdgpu/amdgpu_bo.c @@ -310,7 +310,6 @@ int amdgpu_bo_import(amdgpu_device_handle dev, size = lseek(shared_handle, 0, SEEK_END); if (size == (off_t)-1) { pthread_mutex_unlock(>bo_table_mutex); - amdgpu_close_kms_handle(dev, handle); return -errno; } lseek(shared_handle, 0, SEEK_SET); @@ -355,9 +354,6 @@ int amdgpu_bo_import(amdgpu_device_handle dev, bo = calloc(1, sizeof(struct amdgpu_bo)); if (!bo) { pthread_mutex_unlock(>bo_table_mutex); - if (type == amdgpu_bo_handle_type_dma_buf_fd) { - amdgpu_close_kms_handle(dev, shared_handle); - } return -ENOMEM; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 2/3] amdgpu: don't mess with shared_handle if amdgpu_bo_import() fails
Do not close the handle if someone else has created it. Afaict there's no change of ownership implied if the function fails. Thus the caller is responsible to doing the right thing - trying again, closing the handle and/or other. Cc: amd-...@lists.freedesktop.org Signed-off-by: Emil Velikov--- Not 100% sure if it's the correct thing. --- amdgpu/amdgpu_bo.c | 4 1 file changed, 4 deletions(-) diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c index c9f31587..24fa8880 100644 --- a/amdgpu/amdgpu_bo.c +++ b/amdgpu/amdgpu_bo.c @@ -310,7 +310,6 @@ int amdgpu_bo_import(amdgpu_device_handle dev, size = lseek(shared_handle, 0, SEEK_END); if (size == (off_t)-1) { pthread_mutex_unlock(>bo_table_mutex); - amdgpu_close_kms_handle(dev, handle); return -errno; } lseek(shared_handle, 0, SEEK_SET); @@ -355,9 +354,6 @@ int amdgpu_bo_import(amdgpu_device_handle dev, bo = calloc(1, sizeof(struct amdgpu_bo)); if (!bo) { pthread_mutex_unlock(>bo_table_mutex); - if (type == amdgpu_bo_handle_type_dma_buf_fd) { - amdgpu_close_kms_handle(dev, shared_handle); - } return -ENOMEM; } -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel