drm: Release driver references to handle before making it available again

ickle · danvet · commit f6cd7daecff5 · 2016-04-15T14:39:18.000+02:00
When userspace closes a handle, we remove it from the file->object_idr and then tell the driver to drop its references to that file/handle. However, as the file/handle is already available again for reuse, it may be reallocated back to userspace and active on a new object before the driver has had a chance to drop the old file/handle references. Whilst calling back into the driver, we have to drop the file->table_lock spinlock and so to prevent reusing the closed handle we mark that handle as stale in the idr, perform the callback and then remove the handle. We set the stale handle to point to the NULL object, then any idr_find() whilst the driver is removing the handle will return NULL, just as if the handle is already removed from idr. Note: This will be used to have a direct handle -> vma lookup table, instead of first a handle -> obj lookup, and then an (obj, vm) -> vma lookup. v2: Use NULL rather than an ERR_PTR to avoid having to adjust callers. idr_alloc() tracks existing handles using an internal bitmap, so we are free to use the NULL object as our stale identifier. v3: Needed to update the return value check after changing from using the stale error pointer to NULL. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: dri-devel@lists.freedesktop.org Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Rob Clark <robdclark@gmail.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Thierry Reding <treding@nvidia.com> [danvet: Add note about the use-case.] Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1460721308-32405-1-git-send-email-chris@chris-wilson.co.uk
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
@@ -279,7 +279,6 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
 int
 drm_gem_handle_delete(struct drm_file *filp, u32 handle)
 {
-	struct drm_device *dev;
 	struct drm_gem_object *obj;
 
 	/* This is gross. The idr system doesn't let us try a delete and
@@ -294,18 +293,19 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
 	spin_lock(&filp->table_lock);
 
 	/* Check if we currently have a reference on the object */
-	obj = idr_find(&filp->object_idr, handle);
-	if (obj == NULL) {
-		spin_unlock(&filp->table_lock);
+	obj = idr_replace(&filp->object_idr, NULL, handle);
+	spin_unlock(&filp->table_lock);
+	if (IS_ERR_OR_NULL(obj))
 		return -EINVAL;
-	}
-	dev = obj->dev;
 
-	/* Release reference and decrement refcount. */
+	/* Release driver's reference and decrement refcount. */
+	drm_gem_object_release_handle(handle, obj, filp);
+
+	/* And finally make the handle available for future allocations. */
+	spin_lock(&filp->table_lock);
 	idr_remove(&filp->object_idr, handle);
 	spin_unlock(&filp->table_lock);
 
-	drm_gem_object_release_handle(handle, obj, filp);
 	return 0;
 }
 EXPORT_SYMBOL(drm_gem_handle_delete);
