[PATCH] drm/gem: support BO freeing without dev->struct_mutex

Daniel Vetter daniel.vetter at ffwll.ch
Wed Apr 27 11:50:00 UTC 2016


Finally all the core gem and a lot of drivers are entirely free of
dev->struct_mutex depencies, and we can start to have an entirely
lockless unref path.

To make sure that no one who touches the core code accidentally breaks
existing drivers which still require dev->struct_mutex I've made the
might_lock check unconditional.

While at it de-inline the ref/unref functions, they've become a bit
too big.

v2: Make it not leak like a sieve.

v3: Review from Lucas:
- drop != NULL in pointer checks.
- fixup copypasted kerneldoc to actually match the functions.

Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Alex Deucher <alexdeucher at gmail.com>
Cc: Lucas Stach <l.stach at pengutronix.de>
Reviewed-by: Lucas Stach <l.stach at pengutronix.de>
Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
---
 drivers/gpu/drm/drm_gem.c | 64 ++++++++++++++++++++++++++++++++++++++---------
 include/drm/drmP.h        | 15 ++++++++---
 include/drm/drm_gem.h     | 45 ++-------------------------------
 3 files changed, 66 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 25dac31eef37..a4684a306c48 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -788,16 +788,7 @@ drm_gem_object_release(struct drm_gem_object *obj)
 }
 EXPORT_SYMBOL(drm_gem_object_release);
 
-/**
- * drm_gem_object_free - free a GEM object
- * @kref: kref of the object to free
- *
- * Called after the last reference to the object has been lost.
- * Must be called holding struct_ mutex
- *
- * Frees the object
- */
-void
+static void
 drm_gem_object_free(struct kref *kref)
 {
 	struct drm_gem_object *obj =
@@ -806,10 +797,59 @@ drm_gem_object_free(struct kref *kref)
 
 	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
 
-	if (dev->driver->gem_free_object != NULL)
+	if (dev->driver->gem_free_object_unlocked)
+		dev->driver->gem_free_object_unlocked(obj);
+	else if (dev->driver->gem_free_object)
 		dev->driver->gem_free_object(obj);
 }
-EXPORT_SYMBOL(drm_gem_object_free);
+
+/**
+ * drm_gem_object_unreference_unlocked - release a GEM BO reference
+ * @obj: GEM buffer object
+ *
+ * This releases a reference to @obj. Callers must not hold the
+ * dev->struct_mutex lock when calling this function.
+ */
+void
+drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
+{
+	struct drm_device *dev;
+
+	if (!obj)
+		return;
+
+	dev = obj->dev;
+	might_lock(&dev->struct_mutex);
+
+	if (dev->driver->gem_free_object)
+		kref_put(&obj->refcount, drm_gem_object_free);
+	else if (kref_put_mutex(&obj->refcount, drm_gem_object_free,
+				&dev->struct_mutex))
+		mutex_unlock(&dev->struct_mutex);
+}
+EXPORT_SYMBOL(drm_gem_object_unreference_unlocked);
+
+/**
+ * drm_gem_object_unreference - release a GEM BO reference
+ * @obj: GEM buffer object
+ *
+ * This releases a reference to @obj. Callers must hold the dev->struct_mutex
+ * lock when calling this function, even when the driver doesn't use
+ * dev->struct_mutex for anything.
+ *
+ * For drivers not encumbered with legacy locking use
+ * drm_gem_object_unreference_unlocked() instead.
+ */
+void
+drm_gem_object_unreference(struct drm_gem_object *obj)
+{
+	if (obj) {
+		WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
+
+		kref_put(&obj->refcount, drm_gem_object_free);
+	}
+}
+EXPORT_SYMBOL(drm_gem_object_unreference);
 
 /**
  * drm_gem_vm_open - vma->ops->open implementation for GEM
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index c81dd2250fc6..bd7b262d7af0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -580,12 +580,21 @@ struct drm_driver {
 	void (*debugfs_cleanup)(struct drm_minor *minor);
 
 	/**
-	 * Driver-specific constructor for drm_gem_objects, to set up
-	 * obj->driver_private.
+	 * @gem_free_object: deconstructor for drm_gem_objects
 	 *
-	 * Returns 0 on success.
+	 * This is deprecated and should not be used by new drivers. Use
+	 * @gem_free_object_unlocked instead.
 	 */
 	void (*gem_free_object) (struct drm_gem_object *obj);
+
+	/**
+	 * @gem_free_object_unlocked: deconstructor for drm_gem_objects
+	 *
+	 * This is for drivers which are not encumbered with dev->struct_mutex
+	 * legacy locking schemes. Use this hook instead of @gem_free_object.
+	 */
+	void (*gem_free_object_unlocked) (struct drm_gem_object *obj);
+
 	int (*gem_open_object) (struct drm_gem_object *, struct drm_file *);
 	void (*gem_close_object) (struct drm_gem_object *, struct drm_file *);
 
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 0b3e11ab8757..ae1c7f18eec0 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -175,7 +175,6 @@ struct drm_gem_object {
 };
 
 void drm_gem_object_release(struct drm_gem_object *obj);
-void drm_gem_object_free(struct kref *kref);
 int drm_gem_object_init(struct drm_device *dev,
 			struct drm_gem_object *obj, size_t size);
 void drm_gem_private_object_init(struct drm_device *dev,
@@ -199,48 +198,8 @@ drm_gem_object_reference(struct drm_gem_object *obj)
 	kref_get(&obj->refcount);
 }
 
-/**
- * drm_gem_object_unreference - release a GEM BO reference
- * @obj: GEM buffer object
- *
- * This releases a reference to @obj. Callers must hold the dev->struct_mutex
- * lock when calling this function, even when the driver doesn't use
- * dev->struct_mutex for anything.
- *
- * For drivers not encumbered with legacy locking use
- * drm_gem_object_unreference_unlocked() instead.
- */
-static inline void
-drm_gem_object_unreference(struct drm_gem_object *obj)
-{
-	if (obj != NULL) {
-		WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
-
-		kref_put(&obj->refcount, drm_gem_object_free);
-	}
-}
-
-/**
- * drm_gem_object_unreference_unlocked - release a GEM BO reference
- * @obj: GEM buffer object
- *
- * This releases a reference to @obj. Callers must not hold the
- * dev->struct_mutex lock when calling this function.
- */
-static inline void
-drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
-{
-	struct drm_device *dev;
-
-	if (!obj)
-		return;
-
-	dev = obj->dev;
-	if (kref_put_mutex(&obj->refcount, drm_gem_object_free, &dev->struct_mutex))
-		mutex_unlock(&dev->struct_mutex);
-	else
-		might_lock(&dev->struct_mutex);
-}
+void drm_gem_object_unreference_unlocked(struct drm_gem_object *obj);
+void drm_gem_object_unreference(struct drm_gem_object *obj);
 
 int drm_gem_handle_create(struct drm_file *file_priv,
 			  struct drm_gem_object *obj,
-- 
2.8.1



More information about the dri-devel mailing list