diff mbox

[RFC,9/9] drm: support immediate unplugging

Message ID 1374673438-26251-10-git-send-email-dh.herrmann@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Herrmann July 24, 2013, 1:43 p.m. UTC
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@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 mbox

Patch

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@valinux.com>
- */
-
 /*
- * Created: Fri Jan 19 10:48:35 2001 by faith@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@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);