[PATCH 1/6] drm: call drm_unplug_minor() from drm_put_minor()

[PATCH 1/6] drm: call drm_unplug_minor() from drm_put_minor()

[David Herrmann]

This protects drm_unplug_minor() against repeated calls so we can use it
in drm_put_minor(). This allows us to further simplify it in follow-ups as
we no longer do minor-destruction in both functions but only in
drm_unplug_minor().
Also add kernel-doc comments about what these calls do.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_stub.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index c181b71..abc9d49 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -324,10 +324,30 @@ err_idr:
 EXPORT_SYMBOL(drm_get_minor);
 
 /**
- * Put a secondary minor number.
+ * drm_unplug_minor - Unplug DRM minor
+ * @minor: Minor to unplug
  *
- * \param sec_minor - structure to be released
- * \return always zero
+ * Unplugs the given DRM minor but keeps the object. So after this returns,
+ * minor->dev is still valid so existing open-files can still access it to get
+ * device information from their drm_file ojects.
+ * If the minor is already unplugged or if @minor is NULL, nothing is done.
+ * The global DRM mutex must be held by the caller.
+ */
+static void drm_unplug_minor(struct drm_minor *minor)
+{
+	if (!minor || !device_is_registered(&minor->kdev))
+		return;
+
+	drm_sysfs_device_remove(minor);
+}
+
+/**
+ * drm_put_minor - Destroy DRM minor
+ * @minor_p: Double pointer to DRM minor
+ *
+ * This calls drm_unplug_minor() on the given minor and then frees it. The minor
+ * pointer is reset to NULL before this returns.
+ * The global DRM mutex must be held by the caller.
  */
 int drm_put_minor(struct drm_minor **minor_p)
 {
@@ -339,7 +359,7 @@ int drm_put_minor(struct drm_minor **minor_p)
 	drm_debugfs_cleanup(minor);
 #endif
 
-	drm_sysfs_device_remove(minor);
+	drm_unplug_minor(minor);
 
 	idr_remove(&drm_minors_idr, minor->index);
 
@@ -349,11 +369,6 @@ int drm_put_minor(struct drm_minor **minor_p)
 }
 EXPORT_SYMBOL(drm_put_minor);
 
-static void drm_unplug_minor(struct drm_minor *minor)
-{
-	drm_sysfs_device_remove(minor);
-}
-
 /**
  * Called via drm_exit() at module unload time or when pci device is
  * unplugged.
-- 
1.8.4.1

[PATCH 2/6] drm: simplify drm_put_minor()

[David Herrmann]

Allow passing NULL as minor to simplify DRM destruction paths. Also remove
the double-pointer reset as it is no longer needed. drm_put_minor() is
only called when the underlying object is destroyed. Hence, resetting
minors to NULL is not necessary.

As drm_put_minor() is no longer used by other DRM files, we can make it
static, too.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_stub.c | 31 +++++++++++++------------------
 include/drm/drmP.h         |  1 -
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index abc9d49..4f606dc 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -343,15 +343,17 @@ static void drm_unplug_minor(struct drm_minor *minor)
 
 /**
  * drm_put_minor - Destroy DRM minor
- * @minor_p: Double pointer to DRM minor
+ * @minor: Minor to destroy
  *
- * This calls drm_unplug_minor() on the given minor and then frees it. The minor
- * pointer is reset to NULL before this returns.
+ * This calls drm_unplug_minor() on the given minor and then frees it. Nothing
+ * is done if @minor is NULL. It is fine to call this on already unplugged
+ * minors.
  * The global DRM mutex must be held by the caller.
  */
-int drm_put_minor(struct drm_minor **minor_p)
+static void drm_put_minor(struct drm_minor *minor)
 {
-	struct drm_minor *minor = *minor_p;
+	if (!minor)
+		return;
 
 	DRM_DEBUG("release secondary minor %d\n", minor->index);
 
@@ -364,10 +366,7 @@ int drm_put_minor(struct drm_minor **minor_p)
 	idr_remove(&drm_minors_idr, minor->index);
 
 	kfree(minor);
-	*minor_p = NULL;
-	return 0;
 }
-EXPORT_SYMBOL(drm_put_minor);
 
 /**
  * Called via drm_exit() at module unload time or when pci device is
@@ -556,13 +555,11 @@ err_unload:
 	if (dev->driver->unload)
 		dev->driver->unload(dev);
 err_primary_node:
-	drm_put_minor(&dev->primary);
+	drm_put_minor(dev->primary);
 err_render_node:
-	if (dev->render)
-		drm_put_minor(&dev->render);
+	drm_put_minor(dev->render);
 err_control_node:
-	if (dev->control)
-		drm_put_minor(&dev->control);
+	drm_put_minor(dev->control);
 out_unlock:
 	mutex_unlock(&drm_global_mutex);
 	return ret;
@@ -594,11 +591,9 @@ 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 (dev->control)
-		drm_put_minor(&dev->control);
-	if (dev->render)
-		drm_put_minor(&dev->render);
-	drm_put_minor(&dev->primary);
+	drm_put_minor(dev->control);
+	drm_put_minor(dev->render);
+	drm_put_minor(dev->primary);
 
 	list_del(&dev->driver_item);
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 220013d..dc02fb1 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1448,7 +1448,6 @@ extern struct drm_master *drm_master_get(struct drm_master *master);
 extern void drm_master_put(struct drm_master **master);
 
 extern void drm_put_dev(struct drm_device *dev);
-extern int drm_put_minor(struct drm_minor **minor);
 extern void drm_unplug_dev(struct drm_device *dev);
 extern unsigned int drm_debug;
 extern unsigned int drm_rnodes;
-- 
1.8.4.1

[PATCH 3/6] drm: make drm_get_minor() static

[David Herrmann]

drm_get_minor() is only used in one file. Make it static and add a
kernel-doc comment which documents the current semantics.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_stub.c | 19 +++++++++++--------
 include/drm/drmP.h         |  1 -
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 4f606dc..7feed52 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -255,16 +255,20 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
 }
 
 /**
- * Get a secondary minor number.
+ * drm_get_minor - Allocate and register new DRM minor
+ * @dev: DRM device
+ * @minor: Pointer to where new minor is stored
+ * @type: Type of minor
  *
- * \param dev device data structure
- * \param sec-minor structure to hold the assigned minor
- * \return negative number on failure.
+ * Allocate a new minor of the given type and register it. A pointer to the new
+ * minor is returned in @minor.
+ * Caller must hold the global DRM mutex.
  *
- * Search an empty entry and initialize it to the given parameters. This
- * routines assigns minor numbers to secondary heads of multi-headed cards
+ * RETURNS:
+ * 0 on success, negative error code on failure.
  */
-int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type)
+static int drm_get_minor(struct drm_device *dev, struct drm_minor **minor,
+			 int type)
 {
 	struct drm_minor *new_minor;
 	int ret;
@@ -321,7 +325,6 @@ err_idr:
 	*minor = NULL;
 	return ret;
 }
-EXPORT_SYMBOL(drm_get_minor);
 
 /**
  * drm_unplug_minor - Unplug DRM minor
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index dc02fb1..19b8082 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1641,7 +1641,6 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver,
 void drm_dev_free(struct drm_device *dev);
 int drm_dev_register(struct drm_device *dev, unsigned long flags);
 void drm_dev_unregister(struct drm_device *dev);
-int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type);
 /*@}*/
 
 /* PCI section */
-- 
1.8.4.1

[PATCH 4/6] drm: cleanup debugfs in drm_unplug_minor()

[David Herrmann]

There is no reason to delay debugfs-cleanup to drm_put_minor(). We should
forbid any access to debugfs files once the device is dead. Chances they
oops once a card was unplugged are very high, anyway.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_stub.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 7feed52..5bf4339 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -341,6 +341,10 @@ static void drm_unplug_minor(struct drm_minor *minor)
 	if (!minor || !device_is_registered(&minor->kdev))
 		return;
 
+#if defined(CONFIG_DEBUG_FS)
+	drm_debugfs_cleanup(minor);
+#endif
+
 	drm_sysfs_device_remove(minor);
 }
 
@@ -360,10 +364,6 @@ static void drm_put_minor(struct drm_minor *minor)
 
 	DRM_DEBUG("release secondary minor %d\n", minor->index);
 
-#if defined(CONFIG_DEBUG_FS)
-	drm_debugfs_cleanup(minor);
-#endif
-
 	drm_unplug_minor(minor);
 
 	idr_remove(&drm_minors_idr, minor->index);
-- 
1.8.4.1

[PATCH 5/6] drm: remove minor-id during unplug

[David Herrmann]

Don't delay minor removal to drm_put_minor(). Otherwise, user-space can
still open the minor and cause the kernel to oops. Instead, remove the
minor during unplug so any new open() will fail to access this minor.

Note that open() and drm_unplug_minor() are both protected by the global
DRM mutex so we're fine.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_stub.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 5bf4339..4bb80cf 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -346,6 +346,7 @@ static void drm_unplug_minor(struct drm_minor *minor)
 #endif
 
 	drm_sysfs_device_remove(minor);
+	idr_remove(&drm_minors_idr, minor->index);
 }
 
 /**
@@ -365,9 +366,6 @@ static void drm_put_minor(struct drm_minor *minor)
 	DRM_DEBUG("release secondary minor %d\n", minor->index);
 
 	drm_unplug_minor(minor);
-
-	idr_remove(&drm_minors_idr, minor->index);
-
 	kfree(minor);
 }
 
-- 
1.8.4.1

[PATCH 6/6] drm: delay minor destruction to drm_dev_free()

[David Herrmann]

Instead of freeing minors in drm_dev_unregister(), we only unplug them and
delay the free to drm_dev_free(). Note that if drm_dev_register() has
never been called, minors are NULL and this has no effect.

This change is needed to allow early device unregistration. If we want to
call drm_dev_unregister() on live devices, we need to guarantee that
minors are still valid (but unplugged). This way, any open file can still
access file_priv->minor->dev to get the DRM device. However, the minor is
unplugged so no new users can occur.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_stub.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 4bb80cf..b4c51d0 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -487,6 +487,10 @@ EXPORT_SYMBOL(drm_dev_alloc);
  */
 void drm_dev_free(struct drm_device *dev)
 {
+	drm_put_minor(dev->control);
+	drm_put_minor(dev->render);
+	drm_put_minor(dev->primary);
+
 	if (dev->driver->driver_features & DRIVER_GEM)
 		drm_gem_destroy(dev);
 
@@ -592,9 +596,9 @@ 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);
 
-	drm_put_minor(dev->control);
-	drm_put_minor(dev->render);
-	drm_put_minor(dev->primary);
+	drm_unplug_minor(dev->control);
+	drm_unplug_minor(dev->render);
+	drm_unplug_minor(dev->primary);
 
 	list_del(&dev->driver_item);
 }
-- 
1.8.4.1