[PATCH 30/31] drm/i915: Add freed object debugging by freeing synchronously.
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Thu Aug 6 07:36:48 UTC 2020
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 118 ++++++++++++------
.../gpu/drm/i915/gem/i915_gem_object_types.h | 2 +
drivers/gpu/drm/i915/i915_gem.c | 21 ++++
drivers/gpu/drm/i915/i915_vma.c | 4 +-
4 files changed, 102 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index e53e3b1d02a7..00c02fc6613c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -34,6 +34,9 @@
#include "i915_globals.h"
#include "i915_trace.h"
+#include <linux/stacktrace.h>
+#include <linux/stackdepot.h>
+
static struct i915_global_object {
struct i915_global base;
struct kmem_cache *slab_objects;
@@ -41,12 +44,12 @@ static struct i915_global_object {
struct drm_i915_gem_object *i915_gem_object_alloc(void)
{
- return kmem_cache_zalloc(global.slab_objects, GFP_KERNEL);
+ return kzalloc(sizeof(struct drm_i915_gem_object), GFP_KERNEL);
}
void i915_gem_object_free(struct drm_i915_gem_object *obj)
{
- return kmem_cache_free(global.slab_objects, obj);
+ return kfree(obj);
}
void i915_gem_object_init(struct drm_i915_gem_object *obj,
@@ -194,56 +197,72 @@ static void __i915_gem_object_free_mmaps(struct drm_i915_gem_object *obj)
}
}
-static void __i915_gem_free_objects(struct drm_i915_private *i915,
- struct llist_node *freed)
+static void __i915_gem_object_free(struct drm_i915_gem_object *obj)
{
- struct drm_i915_gem_object *obj, *on;
-
- llist_for_each_entry_safe(obj, on, freed, freed) {
- trace_i915_gem_object_destroy(obj);
+ struct i915_vma *vma;
+ trace_i915_gem_object_destroy(obj);
+
+ if (!list_empty(&obj->vma.list)) {
+ /*
+ * Note that the vma keeps an object reference while
+ * it is active, so it *should* not sleep while we
+ * destroy it. Our debug code errs insits it *might*.
+ * For the moment, play along.
+ */
+ spin_lock(&obj->vma.lock);
+ while ((vma = list_first_entry_or_null(&obj->vma.list,
+ struct i915_vma,
+ obj_link))) {
+ GEM_BUG_ON(vma->obj != obj);
+ spin_unlock(&obj->vma.lock);
- if (!list_empty(&obj->vma.list)) {
- struct i915_vma *vma;
+ __i915_vma_put(vma);
- /*
- * Note that the vma keeps an object reference while
- * it is active, so it *should* not sleep while we
- * destroy it. Our debug code errs insits it *might*.
- * For the moment, play along.
- */
spin_lock(&obj->vma.lock);
- while ((vma = list_first_entry_or_null(&obj->vma.list,
- struct i915_vma,
- obj_link))) {
- GEM_BUG_ON(vma->obj != obj);
- spin_unlock(&obj->vma.lock);
+ }
+ spin_unlock(&obj->vma.lock);
+ }
- __i915_vma_put(vma);
+ __i915_gem_object_free_mmaps(obj);
- spin_lock(&obj->vma.lock);
- }
- spin_unlock(&obj->vma.lock);
- }
+ GEM_BUG_ON(!list_empty(&obj->lut_list));
+
+ atomic_set(&obj->mm.pages_pin_count, 0);
+ __i915_gem_object_put_pages(obj);
+ GEM_BUG_ON(i915_gem_object_has_pages(obj));
+ bitmap_free(obj->bit_17);
- __i915_gem_object_free_mmaps(obj);
+ if (obj->base.import_attach)
+ drm_prime_gem_destroy(&obj->base, NULL);
- GEM_BUG_ON(!list_empty(&obj->lut_list));
+ drm_gem_free_mmap_offset(&obj->base);
- atomic_set(&obj->mm.pages_pin_count, 0);
- __i915_gem_object_put_pages(obj);
- GEM_BUG_ON(i915_gem_object_has_pages(obj));
- bitmap_free(obj->bit_17);
+ if (obj->ops->release)
+ obj->ops->release(obj);
- if (obj->base.import_attach)
- drm_prime_gem_destroy(&obj->base, NULL);
+ /* But keep the pointer alive for RCU-protected lookups */
+ synchronize_rcu();
+ __i915_gem_free_object_rcu(&obj->rcu);
+}
+
+static void __i915_gem_free_object(struct drm_i915_gem_object *obj)
+{
+ struct drm_i915_private *i915 = to_i915(READ_ONCE(obj->base.dev));
+ intel_wakeref_t wakeref;
- drm_gem_free_mmap_offset(&obj->base);
+ with_intel_runtime_pm(&i915->runtime_pm, wakeref)
+ __i915_gem_object_free(obj);
+}
- if (obj->ops->release)
- obj->ops->release(obj);
+static void __i915_gem_free_objects(struct drm_i915_private *i915,
+ struct llist_node *freed)
+{
+ struct drm_i915_gem_object *obj, *on;
+ intel_wakeref_t wakeref;
- /* But keep the pointer alive for RCU-protected lookups */
- call_rcu(&obj->rcu, __i915_gem_free_object_rcu);
+ wakeref = intel_runtime_pm_get(&i915->runtime_pm);
+ llist_for_each_entry_safe(obj, on, freed, freed) {
+ __i915_gem_object_free(obj);
cond_resched();
}
}
@@ -264,6 +283,17 @@ static void __i915_gem_free_work(struct work_struct *work)
i915_gem_flush_free_objects(i915);
}
+static noinline void save_stack(struct drm_i915_gem_object *obj)
+{
+ unsigned long entries[32];
+ unsigned int n;
+
+ n = stack_trace_save(entries, ARRAY_SIZE(entries), 1);
+
+ /* May be called under spinlock, so avoid sleeping */
+ obj->stack = stack_depot_save(entries, n, GFP_NOWAIT);
+}
+
void i915_gem_free_object(struct drm_gem_object *gem_obj)
{
struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
@@ -298,8 +328,14 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
* worker and performing frees directly from subsequent allocations for
* crude but effective memory throttling.
*/
- if (llist_add(&obj->freed, &i915->mm.free_list))
- queue_work(i915->wq, &i915->mm.free_work);
+ save_stack(obj);
+
+ if (in_atomic() || irqs_disabled()) {
+ if (llist_add(&obj->freed, &i915->mm.free_list))
+ queue_work(i915->wq, &i915->mm.free_work);
+ } else {
+ __i915_gem_free_object(obj);
+ }
}
static bool gpu_write_needs_clflush(struct drm_i915_gem_object *obj)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index fcaba8756557..16096652b461 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -298,6 +298,8 @@ struct drm_i915_gem_object {
void *gvt_info;
};
+
+ depot_stack_handle_t stack;
};
static inline struct drm_i915_gem_object *
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 30af7e4b71ab..6ec3edd9c7f2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -37,6 +37,9 @@
#include <linux/dma-buf.h>
#include <linux/mman.h>
+#include <linux/stacktrace.h>
+#include <linux/stackdepot.h>
+
#include "display/intel_display.h"
#include "display/intel_frontbuffer.h"
@@ -1355,11 +1358,28 @@ void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ww, bool intr)
ww->contended = NULL;
}
+static void dump_freed_obj(struct drm_i915_gem_object *obj, const char *reason)
+{
+ unsigned long *entries;
+ unsigned int nr_entries;
+ char buf[512];
+
+ if (!obj->stack)
+ return;
+
+ nr_entries = stack_depot_fetch(obj->stack, &entries);
+ stack_trace_snprint(buf, sizeof(buf), entries, nr_entries, 0);
+
+ DRM_ERROR("%s, BACKTRACE: %s\n", reason, buf);
+}
+
static void i915_gem_ww_ctx_unlock_all(struct i915_gem_ww_ctx *ww)
{
struct drm_i915_gem_object *obj;
while ((obj = list_first_entry_or_null(&ww->obj_list, struct drm_i915_gem_object, obj_link))) {
+ dump_freed_obj(obj, "unlocked object freed");
+
list_del(&obj->obj_link);
i915_gem_object_unlock(obj);
}
@@ -1386,6 +1406,7 @@ int __must_check i915_gem_ww_ctx_backoff(struct i915_gem_ww_ctx *ww)
return -EINVAL;
i915_gem_ww_ctx_unlock_all(ww);
+ dump_freed_obj(ww->contended, "unlocked object slowpath");
if (ww->intr)
ret = dma_resv_lock_slow_interruptible(ww->contended->base.resv, &ww->ctx);
else
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 88bffe60ddd3..f257a26350de 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -45,12 +45,12 @@ static struct i915_global_vma {
struct i915_vma *i915_vma_alloc(void)
{
- return kmem_cache_zalloc(global.slab_vmas, GFP_KERNEL);
+ return kzalloc(sizeof(struct i915_vma), GFP_KERNEL);
}
void i915_vma_free(struct i915_vma *vma)
{
- return kmem_cache_free(global.slab_vmas, vma);
+ return kfree(vma);
}
#if IS_ENABLED(CONFIG_DRM_I915_ERRLOG_GEM) && IS_ENABLED(CONFIG_DRM_DEBUG_MM)
--
2.28.0
More information about the Intel-gfx-trybot
mailing list