[PATCH 113/123] drm/i915: Show who pinned the pages when a leak is hit

Chris Wilson chris at chris-wilson.co.uk
Thu Oct 11 20:21:35 UTC 2018


Currently, we emit a warning when freeing an object if we do so with the
pages still pinned (presumably as they are still in use somewhere). This
only tells us that there is a problem, but doesn't tell us anything
about the object or who might be pinning them and so provides no clue as
to track down the problem.

Let's try tracking everyone who pins the pages to see if we strike it
lucky and catch the culprit red handed.

v2: Suppress oom warnings, they will happen given sufficient abuse.

References: https://bugs.freedesktop.org/show_bug.cgi?id=105360
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/Kconfig.debug     |   1 +
 drivers/gpu/drm/i915/i915_drv.h        |   2 +
 drivers/gpu/drm/i915/i915_gem.c        |   6 +-
 drivers/gpu/drm/i915/i915_gem_object.c | 114 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem_object.h |  45 ++++++++++
 5 files changed, 167 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index ad4d71161dda..ed5e9435402a 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -44,6 +44,7 @@ config DRM_I915_DEBUG_GEM
         bool "Insert extra checks into the GEM internals"
         default n
         depends on DRM_I915_WERROR
+        select STACKDEPOT
         help
           Enable extra sanity checks (including BUGs) along the GEM driver
           paths that may slow the system down and if hit hang the machine.
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index adca5203ca62..db4a7bda65c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2929,6 +2929,8 @@ i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
 {
 	might_lock(&obj->mm.lock);
 
+	track_i915_gem_object_pin_pages(obj);
+
 	if (atomic_inc_not_zero(&obj->mm.pages_pin_count))
 		return 0;
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 67ece82814ea..9aca0084eb20 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2380,6 +2380,7 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
 	if (!IS_ERR(pages))
 		obj->ops->put_pages(obj, pages);
 
+	untrack_i915_gem_object_pin_pages(obj);
 unlock:
 	mutex_unlock(&obj->mm.lock);
 }
@@ -4194,6 +4195,7 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
 			  const struct drm_i915_gem_object_ops *ops)
 {
 	mutex_init(&obj->mm.lock);
+	debug_i915_gem_object_init(obj);
 
 	spin_lock_init(&obj->vma.lock);
 	INIT_LIST_HEAD(&obj->vma.list);
@@ -4396,8 +4398,10 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 		if (obj->ops->release)
 			obj->ops->release(obj);
 
-		if (WARN_ON(i915_gem_object_has_pinned_pages(obj)))
+		if (WARN_ON(i915_gem_object_has_pinned_pages(obj))) {
+			show_i915_gem_object_pin_pages(obj);
 			atomic_set(&obj->mm.pages_pin_count, 0);
+		}
 		__i915_gem_object_put_pages(obj, I915_MM_NORMAL);
 		GEM_BUG_ON(i915_gem_object_has_pages(obj));
 
diff --git a/drivers/gpu/drm/i915/i915_gem_object.c b/drivers/gpu/drm/i915/i915_gem_object.c
index aab8cdd80e6d..dbee9bede1cf 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/i915_gem_object.c
@@ -25,6 +25,120 @@
 #include "i915_drv.h"
 #include "i915_gem_object.h"
 
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
+
+#include <linux/sort.h>
+
+#define STACKDEPTH 12
+
+void track_i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
+{
+	unsigned long entries[STACKDEPTH];
+	struct stack_trace trace = {
+		.entries = entries,
+		.max_entries = ARRAY_SIZE(entries),
+		.skip = 1
+	};
+	unsigned long flags;
+	depot_stack_handle_t stack, *stacks;
+
+	save_stack_trace(&trace);
+	if (trace.nr_entries &&
+	    trace.entries[trace.nr_entries - 1] == ULONG_MAX)
+		trace.nr_entries--;
+
+	stack = depot_save_stack(&trace, GFP_KERNEL | __GFP_NOWARN);
+	if (!stack)
+		return;
+
+	spin_lock_irqsave(&obj->mm.debug_lock, flags);
+	stacks = krealloc(obj->mm.debug_owners,
+			  (obj->mm.debug_count + 1) * sizeof(*stacks),
+			  GFP_NOWAIT | __GFP_NOWARN);
+	if (stacks) {
+		stacks[obj->mm.debug_count++] = stack;
+		obj->mm.debug_owners = stacks;
+	}
+	spin_unlock_irqrestore(&obj->mm.debug_lock, flags);
+}
+
+void untrack_i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
+{
+	depot_stack_handle_t *stacks;
+	unsigned long flags;
+
+	spin_lock_irqsave(&obj->mm.debug_lock, flags);
+	stacks = fetch_and_zero(&obj->mm.debug_owners);
+	obj->mm.debug_count = 0;
+	spin_unlock_irqrestore(&obj->mm.debug_lock, flags);
+
+	kfree(stacks);
+}
+
+static int cmphandle(const void *_a, const void *_b)
+{
+	const depot_stack_handle_t * const a = _a, * const b = _b;
+
+	if (*a < *b)
+		return -1;
+	else if (*a > *b)
+		return 1;
+	else
+		return 0;
+}
+
+void show_i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
+{
+	unsigned long entries[STACKDEPTH];
+	depot_stack_handle_t *stacks;
+	unsigned long flags, count, i;
+	char *buf;
+
+	spin_lock_irqsave(&obj->mm.debug_lock, flags);
+	stacks = fetch_and_zero(&obj->mm.debug_owners);
+	count = fetch_and_zero(&obj->mm.debug_count);
+	spin_unlock_irqrestore(&obj->mm.debug_lock, flags);
+	if (!count)
+		return;
+
+	DRM_DEBUG_DRIVER("obj %p leaked pages, pinned %lu\n", obj, count);
+
+	buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!buf)
+		goto out_stacks;
+
+	sort(stacks, count, sizeof(*stacks), cmphandle, NULL);
+
+	for (i = 0; i < count; i++) {
+		struct stack_trace trace = {
+			.entries = entries,
+			.max_entries = ARRAY_SIZE(entries),
+		};
+		depot_stack_handle_t stack = stacks[i];
+		unsigned long rep;
+
+		rep = 1;
+		while (i + 1 < count && stacks[i + 1] == stack)
+			rep++, i++;
+
+		depot_fetch_stack(stack, &trace);
+		snprint_stack_trace(buf, PAGE_SIZE, &trace, 0);
+		DRM_DEBUG_DRIVER("obj %p pages pinned x%lu at\n%s",
+				 obj, rep, buf);
+	}
+
+	kfree(buf);
+out_stacks:
+	kfree(stacks);
+}
+
+void debug_i915_gem_object_init(struct drm_i915_gem_object *obj)
+{
+	spin_lock_init(&obj->mm.debug_lock);
+}
+
+#endif
+
 /**
  * Mark up the object's coherency levels for a given cache_level
  * @obj: #drm_i915_gem_object
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index 9a028b3e3e57..2c9ed004c77f 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -26,6 +26,9 @@
 #define __I915_GEM_OBJECT_H__
 
 #include <linux/reservation.h>
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
+#include <linux/stackdepot.h>
+#endif
 
 #include <drm/drm_vma_manager.h>
 #include <drm/drm_gem.h>
@@ -253,6 +256,12 @@ struct drm_i915_gem_object {
 		 * swizzling.
 		 */
 		bool quirked:1;
+
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
+		spinlock_t debug_lock;
+		unsigned long debug_count;
+		depot_stack_handle_t *debug_owners;
+#endif
 	} mm;
 
 	/** Breadcrumb of last rendering to the buffer.
@@ -471,6 +480,31 @@ i915_gem_object_get_tile_row_size(const struct drm_i915_gem_object *obj)
 int i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
 			       unsigned int tiling, unsigned int stride);
 
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
+
+void track_i915_gem_object_pin_pages(struct drm_i915_gem_object *obj);
+void untrack_i915_gem_object_pin_pages(struct drm_i915_gem_object *obj);
+void show_i915_gem_object_pin_pages(struct drm_i915_gem_object *obj);
+
+#else
+
+static inline void
+track_i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
+{
+}
+
+static inline void
+untrack_i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
+{
+}
+
+static inline void
+show_i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
+{
+}
+
+#endif
+
 static inline struct intel_engine_cs *
 i915_gem_object_last_write_engine(struct drm_i915_gem_object *obj)
 {
@@ -492,5 +526,16 @@ void i915_gem_object_set_cache_coherency(struct drm_i915_gem_object *obj,
 					 unsigned int cache_level);
 void i915_gem_object_flush_if_display(struct drm_i915_gem_object *obj);
 
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
+
+void debug_i915_gem_object_init(struct drm_i915_gem_object *obj);
+
+#else
+
+static inline void debug_i915_gem_object_init(struct drm_i915_gem_object *obj)
+{
+}
+
 #endif
 
+#endif
-- 
2.19.1



More information about the Intel-gfx-trybot mailing list