From 39e20bcd5f4bf9fedac80188fda2e9fcab2f0360 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 2 May 2008 12:28:49 -0700 Subject: [PATCH] Add name/open ioctls, separate handle and pointer ref counts. Names are just another unique integer set (from another idr object). Names are removed when the user refernces (handles) are all destroyed -- this required that handles for objects be counted separately from internal kernel references (so that we can tell when the handles are all gone). --- linux-core/drmP.h | 43 +++++++++++ linux-core/drm_drv.c | 2 + linux-core/drm_gem.c | 193 ++++++++++++++++++++++++++++++++++++++++++-------- linux-core/i915_gem.c | 4 +- shared-core/drm.h | 31 +++++--- 5 files changed, 233 insertions(+), 40 deletions(-) diff --git a/linux-core/drmP.h b/linux-core/drmP.h index d2dc065..ebbbfa8 100644 --- a/linux-core/drmP.h +++ b/linux-core/drmP.h @@ -618,6 +618,9 @@ struct drm_gem_object { /** Reference count of this object */ struct kref refcount; + /** Handle count of this object. Each handle also holds a reference */ + struct kref handlecount; + /** Related drm device */ struct drm_device *dev; @@ -630,6 +633,12 @@ struct drm_gem_object { */ size_t size; + /** + * Global name for this object, starts at 1. 0 means unnamed. + * Access is covered by the object_name_lock in the related drm_device + */ + int name; + void *driver_private; }; @@ -923,6 +932,12 @@ struct drm_device { spinlock_t drw_lock; struct idr drw_idr; /*@} */ + + /** \name GEM information */ + /*@{ */ + spinlock_t object_name_lock; + struct idr object_name_idr; + /*@} */ }; #if __OS_HAS_AGP @@ -1307,6 +1322,9 @@ static inline struct drm_memrange *drm_get_mm(struct drm_memrange_node *block) void drm_gem_object_free (struct kref *kref); + +void +drm_gem_object_handle_free (struct kref *kref); /* Graphics Execution Manager library functions (drm_gem.c) */ static inline void drm_gem_object_reference(struct drm_gem_object *obj) @@ -1322,6 +1340,26 @@ static inline void drm_gem_object_unreference(struct drm_gem_object *obj) kref_put (&obj->refcount, drm_gem_object_free); } +static inline void drm_gem_object_handle_reference (struct drm_gem_object *obj) +{ + drm_gem_object_reference (obj); + kref_get(&obj->handlecount); +} + +static inline void drm_gem_object_handle_unreference (struct drm_gem_object *obj) +{ + if (obj == NULL) + return; + + /* + * Must bump handle count first as this may be the last + * ref, in which case the object would disappear before we + * checked for a name + */ + kref_put (&obj->handlecount, drm_gem_object_handle_free); + drm_gem_object_unreference (obj); +} + struct drm_gem_object * drm_gem_object_lookup(struct drm_device *dev, struct drm_file *filp, int handle); @@ -1335,6 +1373,11 @@ int drm_gem_pwrite_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); int drm_gem_mmap_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); +int drm_gem_name_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv); +int drm_gem_open_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv); + void drm_gem_open(struct drm_device *dev, struct drm_file *file_private); void drm_gem_release(struct drm_device *dev, struct drm_file *file_private); diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c index 11fedad..4c85cdf 100644 --- a/linux-core/drm_drv.c +++ b/linux-core/drm_drv.c @@ -156,6 +156,8 @@ static struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_GEM_PREAD, drm_gem_pread_ioctl, 0), DRM_IOCTL_DEF(DRM_IOCTL_GEM_PWRITE, drm_gem_pwrite_ioctl, 0), DRM_IOCTL_DEF(DRM_IOCTL_GEM_MMAP, drm_gem_mmap_ioctl, 0), + DRM_IOCTL_DEF(DRM_IOCTL_GEM_NAME, drm_gem_name_ioctl, DRM_AUTH), + DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, DRM_AUTH), }; #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/linux-core/drm_gem.c b/linux-core/drm_gem.c index 80e8657..2e963f2 100644 --- a/linux-core/drm_gem.c +++ b/linux-core/drm_gem.c @@ -81,6 +81,7 @@ drm_gem_object_alloc(struct drm_device *dev, size_t size) } kref_init (&obj->refcount); + kref_init (&obj->handlecount); obj->size = size; if (dev->driver->gem_init_object != NULL && @@ -96,8 +97,7 @@ drm_gem_object_alloc(struct drm_device *dev, size_t size) * Removes the mapping from handle to filp for this object. */ static int -drm_gem_handle_delete(struct drm_device *dev, struct drm_file *filp, - int handle) +drm_gem_handle_delete(struct drm_file *filp, int handle) { struct drm_gem_object *obj; @@ -121,10 +121,45 @@ drm_gem_handle_delete(struct drm_device *dev, struct drm_file *filp, /* Release reference and decrement refcount. */ idr_remove(&filp->object_idr, handle); - drm_gem_object_unreference(obj); - spin_unlock(&filp->table_lock); + drm_gem_object_handle_unreference(obj); + + return 0; +} + +/** + * Create a handle for this object. This adds a handle reference + * to the object, which includes a regular reference count. Callers + * will likely want to dereference the object afterwards. + */ +static int +drm_gem_handle_create (struct drm_file *file_priv, + struct drm_gem_object *obj, + int *handlep) +{ + int ret; + + /* + * Get the user-visible handle using idr. + */ +again: + /* ensure there is space available to allocate a handle */ + if (idr_pre_get(&file_priv->object_idr, GFP_KERNEL) == 0) { + kfree(obj); + return -ENOMEM; + } + /* do the allocation under our spinlock */ + spin_lock (&file_priv->table_lock); + ret = idr_get_new_above(&file_priv->object_idr, obj, 1, handlep); + spin_unlock (&file_priv->table_lock); + if (ret == -EAGAIN) + goto again; + + if (ret != 0) + return ret; + + drm_gem_object_handle_reference (obj); return 0; } @@ -171,27 +206,11 @@ drm_gem_alloc_ioctl(struct drm_device *dev, void *data, if (obj == NULL) return -ENOMEM; - /* Get the user-visible handle using idr. - * - * I'm not really sure why the idr api needs us to do this in two - * repeating steps. It handles internal locking of its data - * structure, yet insists that we keep its memory allocation step - * separate from its slot-finding step for locking purposes. - */ - do { - if (idr_pre_get(&file_priv->object_idr, GFP_KERNEL) == 0) { - kfree(obj); - return -EFAULT; - } + ret = drm_gem_handle_create (file_priv, obj, &handle); + drm_gem_object_handle_unreference(obj); - ret = idr_get_new_above(&file_priv->object_idr, obj, 1, - &handle); - } while (ret == -EAGAIN); - - if (ret != 0) { - drm_gem_object_unreference(obj); - return -EFAULT; - } + if (ret) + return ret; args->handle = handle; @@ -208,7 +227,7 @@ drm_gem_unreference_ioctl(struct drm_device *dev, void *data, struct drm_gem_unreference *args = data; int ret; - ret = drm_gem_handle_delete(dev, file_priv, args->handle); + ret = drm_gem_handle_delete(file_priv, args->handle); return ret; } @@ -316,6 +335,87 @@ drm_gem_pwrite_ioctl(struct drm_device *dev, void *data, } /** + * Create a global name for an object, returning the name. + * + * Note that the name does not hold a reference; when the object + * is freed, the name goes away. + */ +int +drm_gem_name_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_gem_name *args = data; + struct drm_gem_object *obj; + int ret; + + obj = drm_gem_object_lookup(dev, file_priv, args->handle); + if (obj == NULL) + return -EINVAL; + +again: + if (idr_pre_get(&dev->object_name_idr, GFP_KERNEL) == 0) { + return -ENOMEM; + } + spin_lock(&dev->object_name_lock); + if (obj->name) { + spin_unlock (&dev->object_name_lock); + return -EEXIST; + } + ret = idr_get_new_above (&dev->object_name_idr, obj, 1, + &obj->name); + spin_unlock (&dev->object_name_lock); + if (ret == -EAGAIN) + goto again; + + if (ret != 0) { + drm_gem_object_unreference(obj); + return ret; + } + + /* + * Leave the reference from the lookup around as the + * name table now holds one + */ + args->name = (uint64_t) obj->name; + + return 0; +} + +/** + * Open an object using the global name, returning a handle and the size. + * + * This handle (of course) holds a reference to the object, so the object + * will not go away until the handle is deleted. + */ +int +drm_gem_open_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_gem_open *args = data; + struct drm_gem_object *obj; + int ret; + int handle; + + spin_lock (&dev->object_name_lock); + obj = idr_find (&dev->object_name_idr, (int) args->name); + if (obj) + drm_gem_object_reference (obj); + spin_unlock (&dev->object_name_lock); + if (!obj) + return -ENOENT; + + ret = drm_gem_handle_create (file_priv, obj, &handle); + drm_gem_object_unreference (obj); + if (ret) + return ret; + + args->handle = handle; + args->size = obj->size; + + return 0; +} + +/** * Called at device open time, sets up the structure for handling refcounting * of mm objects. */ @@ -325,13 +425,13 @@ drm_gem_open(struct drm_device *dev, struct drm_file *file_private) idr_init(&file_private->object_idr); } -/** Called at device close to release the file's references on objects. */ +/** Called at device close to release the file's handle references on objects. */ static int -drm_gem_object_release(int id, void *ptr, void *data) +drm_gem_object_release_handle (int id, void *ptr, void *data) { struct drm_gem_object *obj = ptr; - drm_gem_object_unreference(obj); + drm_gem_object_handle_unreference(obj); return 0; } @@ -344,11 +444,16 @@ drm_gem_object_release(int id, void *ptr, void *data) void drm_gem_release(struct drm_device *dev, struct drm_file *file_private) { - idr_for_each(&file_private->object_idr, &drm_gem_object_release, NULL); + idr_for_each(&file_private->object_idr, &drm_gem_object_release_handle, NULL); idr_destroy(&file_private->object_idr); } +/** + * Called after the last reference to the object has been lost. + * + * Frees the object + */ void drm_gem_object_free (struct kref *kref) { @@ -359,6 +464,36 @@ drm_gem_object_free (struct kref *kref) dev->driver->gem_free_object(obj); fput(obj->filp); + kfree(obj); } EXPORT_SYMBOL(drm_gem_object_free); + +/** + * Called after the last handle to the object has been closed + * + * Removes any name for the object. Note that this must be + * called before drm_gem_object_free or we'll be touching + * freed memory + */ +void +drm_gem_object_handle_free (struct kref *kref) +{ + struct drm_gem_object *obj = container_of (kref, struct drm_gem_object, handlecount); + struct drm_device *dev = obj->dev; + + /* Remove any name for this object */ + spin_lock (&dev->object_name_lock); + if (obj->name) { + idr_remove (&dev->object_name_idr, obj->name); + spin_unlock (&dev->object_name_lock); + /* + * The object name held a reference to this object, drop + * that now. + */ + drm_gem_object_unreference (obj); + } else + spin_unlock (&dev->object_name_lock); + +} +EXPORT_SYMBOL(drm_gem_object_handle_free); diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 16bb392..2ac74b4 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -105,8 +105,10 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) if (alignment == 0) alignment = PAGE_SIZE; - if (alignment & (PAGE_SIZE - 1)) + if (alignment & (PAGE_SIZE - 1)) { + DRM_ERROR("Invalid object alignment requested %u\n", alignment); return -EINVAL; + } free_space = drm_memrange_search_free(&dev_priv->mm.gtt_space, obj->size, diff --git a/shared-core/drm.h b/shared-core/drm.h index 90c23fa..0e8da7b 100644 --- a/shared-core/drm.h +++ b/shared-core/drm.h @@ -983,16 +983,6 @@ struct drm_gem_unreference { uint32_t pad; }; -struct drm_gem_link { - /** Handle for the object being given a name. */ - uint32_t handle; - uint32_t pad; - /** Requested file name to export the object under. */ - uint64_t name_ptr; /* char *, but pointers are not 32/64 compatible */ - /** Requested file mode to export the object under. */ - mode_t mode; -}; - struct drm_gem_pread { /** Handle for the object being read. */ uint32_t handle; @@ -1033,6 +1023,25 @@ struct drm_gem_mmap { uint64_t addr_ptr; /* void *, but pointers are not 32/64 compatible */ }; +struct drm_gem_name { + /** Handle for the object being named */ + uint32_t handle; + + /** Returned global name */ + uint32_t name; +}; + +struct drm_gem_open { + /** Name of object being opened */ + uint32_t name; + + /** Returned handle for the object */ + uint32_t handle; + + /** Returned size of the object */ + uint64_t size; +}; + /** * \name Ioctls Definitions */ @@ -1058,6 +1067,8 @@ struct drm_gem_mmap { #define DRM_IOCTL_GEM_PREAD DRM_IOW(0x0b, struct drm_gem_pread) #define DRM_IOCTL_GEM_PWRITE DRM_IOW(0x0c, struct drm_gem_pwrite) #define DRM_IOCTL_GEM_MMAP DRM_IOWR(0x0d, struct drm_gem_mmap) +#define DRM_IOCTL_GEM_NAME DRM_IOWR(0x0e, struct drm_gem_name) +#define DRM_IOCTL_GEM_OPEN DRM_IOWR(0x0f, struct drm_gem_open) #define DRM_IOCTL_SET_UNIQUE DRM_IOW( 0x10, struct drm_unique) #define DRM_IOCTL_AUTH_MAGIC DRM_IOW( 0x11, struct drm_auth) -- 2.7.4