[PATCH 05/13] drm: provide device-refcount

From: David Herrmann <dh.herrmann@gmail.com>
To: dri-devel@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH 05/13] drm: provide device-refcount
Date: Wed, 29 Jan 2014 15:01:52 +0100	[thread overview]
Message-ID: <1391004120-687-6-git-send-email-dh.herrmann@gmail.com> (raw)
In-Reply-To: <1391004120-687-1-git-send-email-dh.herrmann@gmail.com>

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 <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_pci.c      |  2 +-
 drivers/gpu/drm/drm_platform.c |  2 +-
 drivers/gpu/drm/drm_stub.c     | 57 +++++++++++++++++++++++++++++++++++-------
 drivers/gpu/drm/drm_usb.c      |  2 +-
 drivers/gpu/drm/tegra/bus.c    |  2 +-
 include/drm/drmP.h             |  5 +++-
 6 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 5736aaa..9ded847 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 21fc820..319ff53 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 98a33c580..c51333e 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);
 
@@ -410,7 +410,7 @@ void drm_unplug_dev(struct drm_device *dev)
 	drm_device_set_unplugged(dev);
 
 	if (dev->open_count == 0) {
-		drm_put_dev(dev);
+		drm_dev_unref(dev);
 	}
 	mutex_unlock(&drm_global_mutex);
 }
@@ -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;
 
@@ -486,12 +490,10 @@ EXPORT_SYMBOL(drm_dev_alloc);
  * @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().
+ * This may not be called directly, you must use drm_dev_ref() and
+ * drm_dev_unref() to gain and drop references to the object.
  */
-void drm_dev_free(struct drm_device *dev)
+static void drm_dev_free(struct drm_device *dev)
 {
 	drm_put_minor(dev->control);
 	drm_put_minor(dev->render);
@@ -506,7 +508,44 @@ void drm_dev_free(struct drm_device *dev)
 	kfree(dev->devname);
 	kfree(dev);
 }
-EXPORT_SYMBOL(drm_dev_free);
+
+static void drm_dev_release(struct kref *ref)
+{
+	drm_dev_free(container_of(ref, struct drm_device, ref));
+}
+
+/**
+ * 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 +620,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 0f8cb1a..c3406aa 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 e38e596..71cef5c 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 c3aaf2d..2c58e94 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -43,6 +43,7 @@
 #include <asm/current.h>
 #endif				/* __alpha__ */
 #include <linux/kernel.h>
+#include <linux/kref.h>
 #include <linux/miscdevice.h>
 #include <linux/fs.h>
 #include <linux/init.h>
@@ -1099,6 +1100,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 */
@@ -1663,7 +1665,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);
 /*@}*/
-- 
1.8.5.3
