Re: [Intel-gfx] [PATCH libdrm] intel: Serialize drmPrimeFDToHandle with struct_mutex
On Fri, Jul 24, 2015 at 11:51:01AM +0100, Chris Wilson wrote: On Fri, Jul 24, 2015 at 11:22:34AM +0200, Michał Winiarski wrote: From: Rafał Sapała rafal.a.sap...@intel.com It is possible to hit a race condition in create_from_prime, when trying to import a BO that's currently being freed. In case of prime sharing we'll succesfully get a handle, but fail on get_tiling call, potentially confusing the caller (and requiring different locking scheme than with sharing using flink). Wrap fd_to_handle with struct_mutex to force a more consistent behaviour between prime/flink, convert fprintf to DBG when handling errors. The race is that the kernel returns us the same file-private handle as the first thread, but that first thread is about to call gem_close (thereby removing the handle from the file completely) and does so between us acquiring the handle and taking the mutex. If we take the mutex, then we acquire the refcnt on the bo prior to the first thread completing its unref (and so preventing the early close). Or we acquire the handle after the earlier close, in which case we are the new owner. Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk Thanks for the patch review, pushed. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH libdrm] intel: Serialize drmPrimeFDToHandle with struct_mutex
From: Rafał Sapała rafal.a.sap...@intel.com
It is possible to hit a race condition in create_from_prime, when trying
to import a BO that's currently being freed. In case of prime sharing
we'll succesfully get a handle, but fail on get_tiling call, potentially
confusing the caller (and requiring different locking scheme than with
sharing using flink). Wrap fd_to_handle with struct_mutex to force
a more consistent behaviour between prime/flink, convert fprintf to DBG
when handling errors.
Testcase: igt/drm_import_export/import-close-race-prime
Signed-off-by: Rafał Sapała rafal.a.sap...@intel.com
Signed-off-by: Michał Winiarski michal.winiar...@intel.com
---
intel/intel_bufmgr_gem.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index b1c3b3a..ed4ffd2 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -2728,14 +2728,19 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr
*bufmgr, int prime_fd, int s
struct drm_i915_gem_get_tiling get_tiling;
drmMMListHead *list;
+ pthread_mutex_lock(bufmgr_gem-lock);
ret = drmPrimeFDToHandle(bufmgr_gem-fd, prime_fd, handle);
+ if (ret) {
+ DBG(create_from_prime: failed to obtain handle from fd: %s\n,
strerror(errno));
+ pthread_mutex_unlock(bufmgr_gem-lock);
+ return NULL;
+ }
/*
* See if the kernel has already returned this buffer to us. Just as
* for named buffers, we must not create two bo's pointing at the same
* kernel object
*/
- pthread_mutex_lock(bufmgr_gem-lock);
for (list = bufmgr_gem-named.next;
list != bufmgr_gem-named;
list = list-next) {
@@ -2747,12 +2752,6 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr
*bufmgr, int prime_fd, int s
}
}
- if (ret) {
- fprintf(stderr,ret is %d %d\n, ret, errno);
- pthread_mutex_unlock(bufmgr_gem-lock);
- return NULL;
- }
-
bo_gem = calloc(1, sizeof(*bo_gem));
if (!bo_gem) {
pthread_mutex_unlock(bufmgr_gem-lock);
@@ -2793,6 +2792,7 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr
*bufmgr, int prime_fd, int s
DRM_IOCTL_I915_GEM_GET_TILING,
get_tiling);
if (ret != 0) {
+ DBG(create_from_prime: failed to get tiling: %s\n,
strerror(errno));
drm_intel_gem_bo_unreference(bo_gem-bo);
return NULL;
}
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH libdrm] intel: Serialize drmPrimeFDToHandle with struct_mutex
On Fri, Jul 24, 2015 at 11:22:34AM +0200, Michał Winiarski wrote: From: Rafał Sapała rafal.a.sap...@intel.com It is possible to hit a race condition in create_from_prime, when trying to import a BO that's currently being freed. In case of prime sharing we'll succesfully get a handle, but fail on get_tiling call, potentially confusing the caller (and requiring different locking scheme than with sharing using flink). Wrap fd_to_handle with struct_mutex to force a more consistent behaviour between prime/flink, convert fprintf to DBG when handling errors. The race is that the kernel returns us the same file-private handle as the first thread, but that first thread is about to call gem_close (thereby removing the handle from the file completely) and does so between us acquiring the handle and taking the mutex. If we take the mutex, then we acquire the refcnt on the bo prior to the first thread completing its unref (and so preventing the early close). Or we acquire the handle after the earlier close, in which case we are the new owner. Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
