[RFC 9/9] drm: support immediate unplugging
David Herrmann
dh.herrmann at gmail.com
Wed Jul 24 06:43:58 PDT 2013
Our current unplug-helpers remove all nodes from user-space and mark the
device as unplugged. However, any pending user-space call is still
continued. We wait for the last close() call to actually unload the
device.
This patch uses the drm_dev_get_active() infrastructure to allow immediate
unplugging. Whenever drm_put_dev() or drm_unplug_dev() is called, we now
call drm_dev_unregister(). This waits for outstanding user-space calls,
marks the device as unplugged, "fake"-closes all open files, zaps mmaps
and unregisters the device.
If there are pending open-files, we will mark them as invalid but keep
them around. The drm_release() callback will notice that and free the
device when the last open-file is closed.
So we still keep the device allocated as we did before, but we no longer
keep it registered. This is analogous to driver-core which also keeps
devices around (until last put_device() call), but allows immediate device
deregistration via unregister_device().
Signed-off-by: David Herrmann <dh.herrmann at gmail.com>
---
Documentation/DocBook/drm.tmpl | 5 ++
drivers/gpu/drm/drm_fops.c | 85 +++++++++++++++++---------
drivers/gpu/drm/drm_stub.c | 133 ++++++++++++++++++++++++++++++-----------
include/drm/drmP.h | 13 +---
4 files changed, 162 insertions(+), 74 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 7d1278e..f668f0f 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -439,6 +439,11 @@ char *date;</synopsis>
</para>
</sect3>
</sect2>
+ <sect2>
+ <title>DRM Device Management</title>
+!Pdrivers/gpu/drm/drm_stub.c device management
+!Edrivers/gpu/drm/drm_stub.c
+ </sect2>
</sect1>
<!-- Internals: memory management -->
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index b75af7d..d6af563 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -477,34 +477,30 @@ int drm_lastclose(struct drm_device * dev)
}
/**
- * Release file.
+ * drm_close() - Close file
+ * @filp: Open file to close
*
- * \param inode device inode
- * \param file_priv DRM file private.
- * \return zero on success or a negative number on failure.
+ * Close all open handles and clean up all resources associated with this open
+ * file. The open-file must not be used after this call returns.
+ *
+ * This does not actually free "file_priv" or "file_priv->minor" so following
+ * user-space entries can still test for file_priv->minor->dev and see whether
+ * it is valid.
+ *
+ * You should free "file_priv" in the real file ->release() callback.
*
- * If the hardware lock is held then free it, and take it again for the kernel
- * context since it's necessary to reclaim buffers. Unlink the file private
- * data from its list and free it. Decreases the open count and if it reaches
- * zero calls drm_lastclose().
+ * Caller must hold the global DRM mutex.
*/
-int drm_release(struct inode *inode, struct file *filp)
+void drm_close(struct file *filp)
{
struct drm_file *file_priv = filp->private_data;
struct drm_device *dev = file_priv->minor->dev;
- int retcode = 0;
-
- mutex_lock(&drm_global_mutex);
DRM_DEBUG("open_count = %d\n", dev->open_count);
if (dev->driver->preclose)
dev->driver->preclose(dev, file_priv);
- /* ========================================================
- * Begin inline drm_release
- */
-
DRM_DEBUG("pid = %d, device = 0x%lx, open_count = %d\n",
task_pid_nr(current),
(long)old_encode_dev(file_priv->minor->device),
@@ -599,26 +595,57 @@ int drm_release(struct inode *inode, struct file *filp)
drm_prime_destroy_file_private(&file_priv->prime);
put_pid(file_priv->pid);
- kfree(file_priv);
+}
+EXPORT_SYMBOL(drm_close);
- /* ========================================================
- * End inline drm_release
- */
+/**
+ * drm_release - Release file
+ * @inode: Inode of DRM device node
+ * @filp: Open file to close
+ *
+ * Release callback which should be used for DRM device file-ops. Calls
+ * drm_close() for @filp and releases the DRM device if is is unplugged and we
+ * are the last user.
+ *
+ * RETURNS:
+ * This always returns 0.
+ */
+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;
+ bool active;
+
+ mutex_lock(&drm_global_mutex);
+ /* If the device is still active, our context is valid and we need to
+ * close it properly. If it is not active, drm_dev_unregister() will
+ * have called drm_close() for us already (protected by
+ * drm_global_mutex). So skip it in this case. */
+ active = drm_dev_get_active(dev);
+
+ if (active)
+ drm_close(filp);
+ kfree(file_priv);
+
+ /* If we are the last open-file and the device is still active,
+ * call lastclose() and continue. If the device is unplugged,
+ * then drm_dev_unregister() already called lastclose() and we
+ * can finally free the device as we are the last user. */
atomic_inc(&dev->counts[_DRM_STAT_CLOSES]);
if (!--dev->open_count) {
- if (atomic_read(&dev->ioctl_count)) {
- DRM_ERROR("Device busy: %d\n",
- atomic_read(&dev->ioctl_count));
- retcode = -EBUSY;
- } else
- retcode = drm_lastclose(dev);
- if (drm_device_is_unplugged(dev))
- drm_put_dev(dev);
+ if (active)
+ drm_lastclose(dev);
+ else
+ drm_dev_free(dev);
}
+
+ if (active)
+ drm_dev_put_active(dev);
+
mutex_unlock(&drm_global_mutex);
- return retcode;
+ return 0;
}
EXPORT_SYMBOL(drm_release);
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index c0e76c0..85a0292 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -1,13 +1,4 @@
-/**
- * \file drm_stub.h
- * Stub support
- *
- * \author Rickard E. (Rik) Faith <faith at valinux.com>
- */
-
/*
- * Created: Fri Jan 19 10:48:35 2001 by faith at acm.org
- *
* Copyright 2001 VA Linux Systems, Inc., Sunnyvale, California.
* All Rights Reserved.
*
@@ -29,6 +20,45 @@
* OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
* ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ * Rickard E. (Rik) Faith <faith at valinux.com>
+ */
+
+/**
+ * DOC: device management
+ *
+ * DRM core provides basic device management and defines the lifetime. A bus
+ * driver is responsible of finding suitable devices and reacting to hotplug
+ * events. The normal device-core operations are available for DRM devices:
+ * drm_dev_alloc() allocates a new device which can be freed via drm_dev_free().
+ * This device is not advertised to user-space and is not considered active
+ * until you call drm_dev_register(). This call will start the DRM device and
+ * notify user-space about it. Once a device is registered, any device callbacks
+ * are considered active and may be entered.
+ *
+ * For platform drivers, this is all you need to do. For hotpluggable drivers, a
+ * bus needs to listen for unplug events. Once a device is unplugged, use
+ * drm_dev_unregister() to deactivate the device, interrupt all pending
+ * user-space processes waiting for the device and mark the device as gone. Once
+ * the device is unregistered, DRM core will take care of freeing it so you must
+ * not access it afterwards.
+ *
+ * DRM core tracks device usage via drm_dev_get_active() and
+ * drm_dev_put_active(). Every user-space entry point must mark a device as
+ * active as long as it is used. The DRM-core helpers do this automatically, but
+ * if a driver provides their own file-ops, they must take care of this. DRM
+ * core guarantees that drm_dev_unregister() will not be called as long as the
+ * device is active. But if you don't mark the device as active, it may get
+ * unregistered (and even freed!) at any time.
+ *
+ * Drivers must use the ->load() and ->unload() callbacks to allocate/free
+ * private device resources. DRM core does not provide device ref-counting as
+ * this isn't needed for now. Instead, drivers must assume a device is gone once
+ * ->unload() was called. Internally, a device is kept alive until
+ * drm_dev_unregister() is called. It is kept allocated until
+ * drm_dev_unregister() is called _and_ the last user-space process closed the
+ * last node of the device.
*/
#include <linux/module.h>
@@ -342,6 +372,9 @@ int drm_put_minor(struct drm_minor **minor_p)
{
struct drm_minor *minor = *minor_p;
+ if (!minor)
+ return 0;
+
DRM_DEBUG("release secondary minor %d\n", minor->index);
if (minor->type == DRM_MINOR_LEGACY)
@@ -362,7 +395,8 @@ EXPORT_SYMBOL(drm_put_minor);
static void drm_unplug_minor(struct drm_minor *minor)
{
- drm_sysfs_device_remove(minor);
+ if (minor)
+ drm_sysfs_device_remove(minor);
}
/**
@@ -374,33 +408,20 @@ static void drm_unplug_minor(struct drm_minor *minor)
*/
void drm_put_dev(struct drm_device *dev)
{
- DRM_DEBUG("\n");
-
- if (!dev) {
- DRM_ERROR("cleanup called no dev\n");
- return;
- }
-
drm_dev_unregister(dev);
- drm_dev_free(dev);
}
EXPORT_SYMBOL(drm_put_dev);
+/**
+ * drm_unplug_dev() - Unplug DRM device
+ * @dev: Device to unplug
+ *
+ * Virtually unplug DRM device, mark it as invalid and wait for any pending
+ * user-space actions. @dev might be freed after this returns.
+ */
void drm_unplug_dev(struct drm_device *dev)
{
- /* for a USB device */
- if (drm_core_check_feature(dev, DRIVER_MODESET))
- drm_unplug_minor(dev->control);
- drm_unplug_minor(dev->primary);
-
- mutex_lock(&drm_global_mutex);
-
- drm_device_set_unplugged(dev);
-
- if (dev->open_count == 0) {
- drm_put_dev(dev);
- }
- mutex_unlock(&drm_global_mutex);
+ drm_dev_unregister(dev);
}
EXPORT_SYMBOL(drm_unplug_dev);
@@ -495,6 +516,9 @@ EXPORT_SYMBOL(drm_dev_alloc);
*/
void drm_dev_free(struct drm_device *dev)
{
+ drm_put_minor(&dev->control);
+ drm_put_minor(&dev->primary);
+
if (dev->driver->driver_features & DRIVER_GEM)
drm_gem_destroy(dev);
@@ -585,10 +609,21 @@ EXPORT_SYMBOL(drm_dev_register);
* Mark DRM device as unplugged, wait for any pending user request and then
* unregister the DRM device from the system. This does the reverse of
* drm_dev_register().
+ *
+ * If there is no pending open-file, this also frees the DRM device by calling
+ * drm_dev_free(). Otherwise, it leaves the device allocated and the last real
+ * ->release() callback will free the device.
*/
void drm_dev_unregister(struct drm_device *dev)
{
struct drm_map_list *r_list, *list_temp;
+ struct drm_file *file;
+
+ /* Take fake open_count to prevent concurrent drm_release() calls from
+ * freeing the device. */
+ mutex_lock(&drm_global_mutex);
+ ++dev->open_count;
+ mutex_unlock(&drm_global_mutex);
/* Wait for pending users and then mark the device as unplugged. We
* must not hold the global-mutex while doing this. Otherwise, calls
@@ -597,6 +632,26 @@ void drm_dev_unregister(struct drm_device *dev)
dev->unplugged = true;
percpu_up_write(&dev->active);
+ mutex_lock(&drm_global_mutex);
+
+ /* By setting "unplugged" we guarantee that no new drm_open will
+ * succeed. We also know, that no user-space process can call into a DRM
+ * device, anymore. So instead of waiting for the last close() we
+ * simulate close() for all active users.
+ * This allows us to unregister the device immediately but wait for the
+ * last real release to free it actually. */
+ while (!list_empty(&dev->filelist)) {
+ file = list_entry(dev->filelist.next, struct drm_file, lhead);
+
+ /* wake up pending tasks in drm_read() */
+ wake_up_interruptible(&file->event_wait);
+ drm_close(file->filp);
+ }
+
+ /* zap all memory mappings */
+ if (dev->dev_mapping)
+ unmap_mapping_range(dev->dev_mapping, 0, ULLONG_MAX, 1);
+
drm_lastclose(dev);
if (dev->driver->unload)
@@ -610,12 +665,20 @@ void drm_dev_unregister(struct drm_device *dev)
list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head)
drm_rmmap(dev, r_list->map);
- if (drm_core_check_feature(dev, DRIVER_MODESET))
- drm_put_minor(&dev->control);
-
- drm_put_minor(&dev->primary);
+ /* Only unplug minors, do not free them. Following drm_open() calls
+ * access file_priv->minor->dev to see whether the device is still
+ * active so keep it around. drm_dev_free() frees them eventually. */
+ drm_unplug_minor(dev->control);
+ drm_unplug_minor(dev->primary);
list_del(&dev->driver_item);
+
+ /* Release our fake open_count. If there are no pending open-files,
+ * free the device directly. */
+ if (!--dev->open_count)
+ drm_dev_free(dev);
+
+ mutex_unlock(&drm_global_mutex);
}
EXPORT_SYMBOL(drm_dev_unregister);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 9689173..7e13d5d 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1212,7 +1212,7 @@ struct drm_device {
/** \name Hotplug Management */
/*@{ */
struct percpu_rw_semaphore active; /**< protect active users */
- atomic_t unplugged; /* device has been unplugged or gone away */
+ bool unplugged; /* device has been unplugged or gone away */
/*@} */
};
@@ -1250,17 +1250,9 @@ static inline int drm_core_has_MTRR(struct drm_device *dev)
#define drm_core_has_MTRR(dev) (0)
#endif
-static inline void drm_device_set_unplugged(struct drm_device *dev)
-{
- smp_wmb();
- atomic_set(&dev->unplugged, 1);
-}
-
static inline int drm_device_is_unplugged(struct drm_device *dev)
{
- int ret = atomic_read(&dev->unplugged);
- smp_rmb();
- return ret;
+ return dev->unplugged;
}
static inline bool drm_modeset_is_locked(struct drm_device *dev)
@@ -1287,6 +1279,7 @@ extern int drm_fasync(int fd, struct file *filp, int on);
extern ssize_t drm_read(struct file *filp, char __user *buffer,
size_t count, loff_t *offset);
extern int drm_release(struct inode *inode, struct file *filp);
+extern void drm_close(struct file *filp);
/* Mapping support (drm_vm.h) */
extern int drm_mmap(struct file *filp, struct vm_area_struct *vma);
--
1.8.3.3
More information about the dri-devel
mailing list