From 45e212d20fdccaf958b194e95a23ad264188c59e Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Tue, 28 Jan 2014 16:00:35 +0100 Subject: drm: group dev-lifetime related members These members are all managed by DRM-core, lets group them together so they're not split across the whole device. Signed-off-by: David Herrmann Reviewed-by: Daniel Vetter --- include/drm/drmP.h | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'include/drm/drmP.h') diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 04a7f31301f8..d6cfca9042fe 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1098,6 +1098,18 @@ struct drm_device { char *devname; /**< For /proc/interrupts */ int if_version; /**< Highest interface version set */ + /** \name Lifetime Management */ + /*@{ */ + struct device *dev; /**< Device structure of bus-device */ + struct drm_driver *driver; /**< DRM driver managing the device */ + void *dev_private; /**< DRM driver private data */ + struct address_space *dev_mapping; /**< Private addr-space just for the device */ + struct drm_minor *control; /**< Control node */ + struct drm_minor *primary; /**< Primary node */ + struct drm_minor *render; /**< Render node */ + atomic_t unplugged; /**< Flag whether dev is dead */ + /*@} */ + /** \name Locks */ /*@{ */ spinlock_t count_lock; /**< For inuse, drm_device::open_count, drm_device::buf_use */ @@ -1171,7 +1183,6 @@ struct drm_device { struct drm_agp_head *agp; /**< AGP data */ - struct device *dev; /**< Device structure */ struct pci_dev *pdev; /**< PCI device structure */ #ifdef __alpha__ struct pci_controller *hose; @@ -1182,17 +1193,11 @@ struct drm_device { struct drm_sg_mem *sg; /**< Scatter gather memory */ unsigned int num_crtcs; /**< Number of CRTCs on this device */ - void *dev_private; /**< device private data */ - struct address_space *dev_mapping; struct drm_sigdata sigdata; /**< For block_all_signals */ sigset_t sigmask; - struct drm_driver *driver; struct drm_local_map *agp_buffer_map; unsigned int agp_buffer_token; - struct drm_minor *control; /**< Control node for card */ - struct drm_minor *primary; /**< render type primary screen head */ - struct drm_minor *render; /**< render node for card */ struct drm_mode_config mode_config; /**< Current mode config */ @@ -1203,8 +1208,6 @@ struct drm_device { struct drm_vma_offset_manager *vma_offset_manager; /*@} */ int switch_power_state; - - atomic_t unplugged; /* device has been unplugged or gone away */ }; #define DRM_SWITCH_POWER_ON 0 -- cgit v1.2.3 From b9a0d15cc59e896dc6b6c07583157d78fcf72fbb Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 29 Jan 2014 12:30:15 +0100 Subject: drm: remove unused DRM_MINOR_UNASSIGNED This constant is unused, remove it. Signed-off-by: David Herrmann Reviewed-by: Daniel Vetter --- include/drm/drmP.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include/drm/drmP.h') diff --git a/include/drm/drmP.h b/include/drm/drmP.h index d6cfca9042fe..ff20b888d525 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1008,7 +1008,6 @@ struct drm_driver { struct list_head legacy_dev_list; }; -#define DRM_MINOR_UNASSIGNED 0 #define DRM_MINOR_LEGACY 1 #define DRM_MINOR_CONTROL 2 #define DRM_MINOR_RENDER 3 -- cgit v1.2.3 From cb8a239b03608079cbfb784e9ac2f522fe846c29 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 29 Jan 2014 12:31:40 +0100 Subject: drm: turn DRM_MINOR_* into enum Use enum for DRM_MINOR_* constants to avoid hard-coding the IDs. Furthermore, add a DRM_MINOR_CNT so we can perform range-checks in follow-ups. This changes the IDs of the minor-types by -1, but they're not used as indices so this is fine. Signed-off-by: David Herrmann Reviewed-by: Daniel Vetter --- include/drm/drmP.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'include/drm/drmP.h') diff --git a/include/drm/drmP.h b/include/drm/drmP.h index ff20b888d525..2c322564ca7d 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1008,9 +1008,12 @@ struct drm_driver { struct list_head legacy_dev_list; }; -#define DRM_MINOR_LEGACY 1 -#define DRM_MINOR_CONTROL 2 -#define DRM_MINOR_RENDER 3 +enum drm_minor_type { + DRM_MINOR_LEGACY, + DRM_MINOR_CONTROL, + DRM_MINOR_RENDER, + DRM_MINOR_CNT, +}; /** * Info file list entry. This structure represents a debugfs or proc file to -- cgit v1.2.3 From 099d1c290e2ebc3b798961a6c177c3aef5f0b789 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 29 Jan 2014 10:21:36 +0100 Subject: drm: provide device-refcount Lets not trick ourselves into thinking "drm_device" objects are not ref-counted. That's just utterly stupid. We manage "drm_minor" objects on each drm-device and each minor can have an unlimited number of open handles. Each of these handles has the drm_minor (and thus the drm_device) as private-data in the file-handle. Therefore, we may not destroy "drm_device" until all these handles are closed. It is *not* possible to reset all these pointers atomically and restrict access to them, and this is *not* how this is done! Instead, we use ref-counts to make sure the object is valid and not freed. Note that we currently use "dev->open_count" for that, which is *exactly* the same as a reference-count, just open coded. So this patch doesn't change any semantics on DRM devices (well, this patch just introduces the ref-count, anyway. Follow-up patches will replace open_count by it). Also note that generic VFS revoke support could allow us to drop this ref-count again. We could then just synchronously disable any fops->xy() calls. However, this is not the case, yet, and no such patches are in sight (and I seriously question the idea of dropping the ref-cnt again). Signed-off-by: David Herrmann --- drivers/gpu/drm/drm_pci.c | 2 +- drivers/gpu/drm/drm_platform.c | 2 +- drivers/gpu/drm/drm_stub.c | 56 +++++++++++++++++++++++++++++++----------- drivers/gpu/drm/drm_usb.c | 2 +- drivers/gpu/drm/tegra/bus.c | 2 +- include/drm/drmP.h | 5 +++- 6 files changed, 50 insertions(+), 19 deletions(-) (limited to 'include/drm/drmP.h') diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 5736aaa7e86c..9ded847b05b4 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -351,7 +351,7 @@ err_agp: drm_pci_agp_destroy(dev); pci_disable_device(pdev); err_free: - drm_dev_free(dev); + drm_dev_unref(dev); return ret; } EXPORT_SYMBOL(drm_get_pci_dev); diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c index 21fc82006b78..319ff5385601 100644 --- a/drivers/gpu/drm/drm_platform.c +++ b/drivers/gpu/drm/drm_platform.c @@ -64,7 +64,7 @@ static int drm_get_platform_dev(struct platform_device *platdev, return 0; err_free: - drm_dev_free(dev); + drm_dev_unref(dev); return ret; } diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 98a33c580ca1..f2f0249304b7 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -392,7 +392,7 @@ void drm_put_dev(struct drm_device *dev) } drm_dev_unregister(dev); - drm_dev_free(dev); + drm_dev_unref(dev); } EXPORT_SYMBOL(drm_put_dev); @@ -425,6 +425,9 @@ EXPORT_SYMBOL(drm_unplug_dev); * Call drm_dev_register() to advertice the device to user space and register it * with other core subsystems. * + * The initial ref-count of the object is 1. Use drm_dev_ref() and + * drm_dev_unref() to take and drop further ref-counts. + * * RETURNS: * Pointer to new DRM device, or NULL if out of memory. */ @@ -438,6 +441,7 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver, if (!dev) return NULL; + kref_init(&dev->ref); dev->dev = parent; dev->driver = driver; @@ -481,18 +485,10 @@ err_free: } EXPORT_SYMBOL(drm_dev_alloc); -/** - * drm_dev_free - Free DRM device - * @dev: DRM device to free - * - * Free a DRM device that has previously been allocated via drm_dev_alloc(). - * You must not use kfree() instead or you will leak memory. - * - * This must not be called once the device got registered. Use drm_put_dev() - * instead, which then calls drm_dev_free(). - */ -void drm_dev_free(struct drm_device *dev) +static void drm_dev_release(struct kref *ref) { + struct drm_device *dev = container_of(ref, struct drm_device, ref); + drm_put_minor(dev->control); drm_put_minor(dev->render); drm_put_minor(dev->primary); @@ -506,7 +502,39 @@ void drm_dev_free(struct drm_device *dev) kfree(dev->devname); kfree(dev); } -EXPORT_SYMBOL(drm_dev_free); + +/** + * drm_dev_ref - Take reference of a DRM device + * @dev: device to take reference of or NULL + * + * This increases the ref-count of @dev by one. You *must* already own a + * reference when calling this. Use drm_dev_unref() to drop this reference + * again. + * + * This function never fails. However, this function does not provide *any* + * guarantee whether the device is alive or running. It only provides a + * reference to the object and the memory associated with it. + */ +void drm_dev_ref(struct drm_device *dev) +{ + if (dev) + kref_get(&dev->ref); +} +EXPORT_SYMBOL(drm_dev_ref); + +/** + * drm_dev_unref - Drop reference of a DRM device + * @dev: device to drop reference of or NULL + * + * This decreases the ref-count of @dev by one. The device is destroyed if the + * ref-count drops to zero. + */ +void drm_dev_unref(struct drm_device *dev) +{ + if (dev) + kref_put(&dev->ref, drm_dev_release); +} +EXPORT_SYMBOL(drm_dev_unref); /** * drm_dev_register - Register DRM device @@ -581,7 +609,7 @@ EXPORT_SYMBOL(drm_dev_register); * * Unregister the DRM device from the system. This does the reverse of * drm_dev_register() but does not deallocate the device. The caller must call - * drm_dev_free() to free all resources. + * drm_dev_unref() to drop their final reference. */ void drm_dev_unregister(struct drm_device *dev) { diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c index 0f8cb1ae7607..c3406aad2944 100644 --- a/drivers/gpu/drm/drm_usb.c +++ b/drivers/gpu/drm/drm_usb.c @@ -30,7 +30,7 @@ int drm_get_usb_dev(struct usb_interface *interface, return 0; err_free: - drm_dev_free(dev); + drm_dev_unref(dev); return ret; } diff --git a/drivers/gpu/drm/tegra/bus.c b/drivers/gpu/drm/tegra/bus.c index e38e5967d77b..71cef5c13dc8 100644 --- a/drivers/gpu/drm/tegra/bus.c +++ b/drivers/gpu/drm/tegra/bus.c @@ -63,7 +63,7 @@ int drm_host1x_init(struct drm_driver *driver, struct host1x_device *device) return 0; err_free: - drm_dev_free(drm); + drm_dev_unref(drm); return ret; } diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 2c322564ca7d..4e53f1607355 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -43,6 +43,7 @@ #include #endif /* __alpha__ */ #include +#include #include #include #include @@ -1102,6 +1103,7 @@ struct drm_device { /** \name Lifetime Management */ /*@{ */ + struct kref ref; /**< Object ref-count */ struct device *dev; /**< Device structure of bus-device */ struct drm_driver *driver; /**< DRM driver managing the device */ void *dev_private; /**< DRM driver private data */ @@ -1666,7 +1668,8 @@ static __inline__ void drm_core_dropmap(struct drm_local_map *map) struct drm_device *drm_dev_alloc(struct drm_driver *driver, struct device *parent); -void drm_dev_free(struct drm_device *dev); +void drm_dev_ref(struct drm_device *dev); +void drm_dev_unref(struct drm_device *dev); int drm_dev_register(struct drm_device *dev, unsigned long flags); void drm_dev_unregister(struct drm_device *dev); /*@}*/ -- cgit v1.2.3 From 1616c525b98deb34b8f4b02eccf0ae3a1310fa27 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 29 Jan 2014 10:49:19 +0100 Subject: drm: add minor-lookup/release helpers Instead of accessing drm_minors_idr directly, this adds a small helper to hide the internals. This will help us later to remove the drm_global_mutex requirement for minor-lookup. Furthermore, this also makes sure that minor->dev is always valid and takes a reference-count to the device as long as the minor is used in an open-file. This way, "struct file*"->private_data->dev is guaranteed to be valid (which it has to, as we cannot reset it). Signed-off-by: David Herrmann Reviewed-by: Daniel Vetter --- drivers/gpu/drm/drm_fops.c | 50 +++++++++++++++++++++++++--------------------- drivers/gpu/drm/drm_stub.c | 39 ++++++++++++++++++++++++++++++++++++ include/drm/drmP.h | 4 ++++ 3 files changed, 70 insertions(+), 23 deletions(-) (limited to 'include/drm/drmP.h') diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 6466cb5d8b1f..79478191404a 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -79,23 +79,22 @@ static int drm_setup(struct drm_device * dev) */ int drm_open(struct inode *inode, struct file *filp) { - struct drm_device *dev = NULL; - int minor_id = iminor(inode); + struct drm_device *dev; struct drm_minor *minor; - int retcode = 0; + int retcode; int need_setup = 0; struct address_space *old_mapping; struct address_space *old_imapping; - minor = idr_find(&drm_minors_idr, minor_id); - if (!minor) - return -ENODEV; - - if (!(dev = minor->dev)) - return -ENODEV; + minor = drm_minor_acquire(iminor(inode)); + if (IS_ERR(minor)) + return PTR_ERR(minor); - if (drm_device_is_unplugged(dev)) - return -ENODEV; + dev = minor->dev; + if (drm_device_is_unplugged(dev)) { + retcode = -ENODEV; + goto err_release; + } if (!dev->open_count++) need_setup = 1; @@ -128,6 +127,8 @@ err_undo: dev->dev_mapping = old_mapping; mutex_unlock(&dev->struct_mutex); dev->open_count--; +err_release: + drm_minor_release(minor); return retcode; } EXPORT_SYMBOL(drm_open); @@ -143,33 +144,33 @@ EXPORT_SYMBOL(drm_open); */ int drm_stub_open(struct inode *inode, struct file *filp) { - struct drm_device *dev = NULL; + struct drm_device *dev; struct drm_minor *minor; - int minor_id = iminor(inode); int err = -ENODEV; const struct file_operations *new_fops; DRM_DEBUG("\n"); mutex_lock(&drm_global_mutex); - minor = idr_find(&drm_minors_idr, minor_id); - if (!minor) - goto out; - - if (!(dev = minor->dev)) - goto out; + minor = drm_minor_acquire(iminor(inode)); + if (IS_ERR(minor)) + goto out_unlock; + dev = minor->dev; if (drm_device_is_unplugged(dev)) - goto out; + goto out_release; new_fops = fops_get(dev->driver->fops); if (!new_fops) - goto out; + goto out_release; replace_fops(filp, new_fops); if (filp->f_op->open) err = filp->f_op->open(inode, filp); -out: + +out_release: + drm_minor_release(minor); +out_unlock: mutex_unlock(&drm_global_mutex); return err; } @@ -453,7 +454,8 @@ int drm_lastclose(struct drm_device * dev) int drm_release(struct inode *inode, struct file *filp) { struct drm_file *file_priv = filp->private_data; - struct drm_device *dev = file_priv->minor->dev; + struct drm_minor *minor = file_priv->minor; + struct drm_device *dev = minor->dev; int retcode = 0; mutex_lock(&drm_global_mutex); @@ -575,6 +577,8 @@ int drm_release(struct inode *inode, struct file *filp) } mutex_unlock(&drm_global_mutex); + drm_minor_release(minor); + return retcode; } EXPORT_SYMBOL(drm_release); diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index f2f0249304b7..269048215e82 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -355,6 +355,45 @@ static void drm_unplug_minor(struct drm_minor *minor) idr_remove(&drm_minors_idr, minor->index); } +/** + * drm_minor_acquire - Acquire a DRM minor + * @minor_id: Minor ID of the DRM-minor + * + * Looks up the given minor-ID and returns the respective DRM-minor object. The + * refence-count of the underlying device is increased so you must release this + * object with drm_minor_release(). + * + * As long as you hold this minor, it is guaranteed that the object and the + * minor->dev pointer will stay valid! However, the device may get unplugged and + * unregistered while you hold the minor. + * + * Returns: + * Pointer to minor-object with increased device-refcount, or PTR_ERR on + * failure. + */ +struct drm_minor *drm_minor_acquire(unsigned int minor_id) +{ + struct drm_minor *minor; + + minor = idr_find(&drm_minors_idr, minor_id); + if (!minor) + return ERR_PTR(-ENODEV); + + drm_dev_ref(minor->dev); + return minor; +} + +/** + * drm_minor_release - Release DRM minor + * @minor: Pointer to DRM minor object + * + * Release a minor that was previously acquired via drm_minor_acquire(). + */ +void drm_minor_release(struct drm_minor *minor) +{ + drm_dev_unref(minor->dev); +} + /** * drm_put_minor - Destroy DRM minor * @minor: Minor to destroy diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 4e53f1607355..82963167f161 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1672,6 +1672,10 @@ void drm_dev_ref(struct drm_device *dev); void drm_dev_unref(struct drm_device *dev); int drm_dev_register(struct drm_device *dev, unsigned long flags); void drm_dev_unregister(struct drm_device *dev); + +struct drm_minor *drm_minor_acquire(unsigned int minor_id); +void drm_minor_release(struct drm_minor *minor); + /*@}*/ /* PCI section */ -- cgit v1.2.3 From 5817878c6f4221c3ace4af63260080635063371e Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 29 Jan 2014 13:12:31 +0100 Subject: drm: remove redundant minor->device field Whenever we access minor->device, we are in a minor->kdev->...->fops callback so the minor->kdev pointer *must* be valid. Thus, simply use minor->kdev->devt instead of minor->device and remove the redundant field. Signed-off-by: David Herrmann Reviewed-by: Daniel Vetter --- drivers/gpu/drm/drm_drv.c | 4 ++-- drivers/gpu/drm/drm_fops.c | 2 +- drivers/gpu/drm/drm_stub.c | 1 - include/drm/drmP.h | 1 - 4 files changed, 3 insertions(+), 5 deletions(-) (limited to 'include/drm/drmP.h') diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 345be03c23db..ec651be2f3cb 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -344,7 +344,7 @@ long drm_ioctl(struct file *filp, DRM_DEBUG("pid=%d, dev=0x%lx, auth=%d, %s\n", task_pid_nr(current), - (long)old_encode_dev(file_priv->minor->device), + (long)old_encode_dev(file_priv->minor->kdev->devt), file_priv->authenticated, ioctl->name); /* Do not trust userspace, use our own definition */ @@ -402,7 +402,7 @@ long drm_ioctl(struct file *filp, if (!ioctl) DRM_DEBUG("invalid ioctl: pid=%d, dev=0x%lx, auth=%d, cmd=0x%02x, nr=0x%02x\n", task_pid_nr(current), - (long)old_encode_dev(file_priv->minor->device), + (long)old_encode_dev(file_priv->minor->kdev->devt), file_priv->authenticated, cmd, nr); if (kdata != stack_kdata) diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 79478191404a..4ce5318d14bc 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -471,7 +471,7 @@ int drm_release(struct inode *inode, struct file *filp) DRM_DEBUG("pid = %d, device = 0x%lx, open_count = %d\n", task_pid_nr(current), - (long)old_encode_dev(file_priv->minor->device), + (long)old_encode_dev(file_priv->minor->kdev->devt), dev->open_count); /* Release any auth tokens that might point to this file_priv, diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index fe9595b750ea..96fe5dec3822 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -318,7 +318,6 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type) if (minor_id < 0) return minor_id; - new_minor->device = MKDEV(DRM_MAJOR, minor_id); new_minor->index = minor_id; idr_replace(&drm_minors_idr, new_minor, minor_id); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 82963167f161..538079030be0 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1043,7 +1043,6 @@ struct drm_info_node { struct drm_minor { int index; /**< Minor device number */ int type; /**< Control or render */ - dev_t device; /**< Device number for mknod */ struct device *kdev; /**< Linux device */ struct drm_device *dev; -- cgit v1.2.3