On Fri, Apr 15, 2016 at 12:55:08PM +0100, Chris Wilson wrote:
> 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.
>
> 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 at chris-wilson.co.uk>
> Cc: dri-devel at lists.freedesktop.org
> Cc: David Airlie <airlied at linux.ie>
> Cc: Daniel Vetter <daniel.vetter at intel.com>
> Cc: Rob Clark <robdclark at gmail.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Thierry Reding <treding at nvidia.com>
I added a note about the intended use-case of this and merged it do
drm-misc.
-Daniel
> ---
> drivers/gpu/drm/drm_gem.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index da0c5320789f..e97b7a99807b 100644
> --- 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);
> --
> 2.8.0.rc3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
