From: Thomas Hellström <thomas.hellstrom@xxxxxxxxx>
With the huge number of sites where multiple-object locking is
needed in the driver, it becomes difficult to avoid recursive
ww_acquire_ctx initialization, and the function prototypes become
bloated passing the ww_acquire_ctx around. Furthermore it's not
always easy to get the -EDEADLK handling correct and to follow it.
Introduce a i915_gem_do_ww utility that tries to remedy all these problems
by enclosing parts of a ww transaction in the following way:
my_function() {
struct i915_gem_ww_ctx *ww, template;
int err;
bool interruptible = true;
i915_do_ww(ww, &template, err, interruptible) {
err = ww_transaction_part(ww);
}
return err;
}
The utility will automatically look up an active ww_acquire_ctx if one
is initialized previously in the call chain, and if one found will
forward the -EDEADLK instead of handling it, which takes care of the
recursive initalization. Using the utility also discourages nested ww
unlocking / relocking that is both very fragile and hard to follow.
To look up and register an active ww_acquire_ctx, use a
driver-wide hash table for now. But noting that a task could only have
a single active ww_acqurie_ctx per ww_class, the active CTX is really
task state and a generic version of this utility in the ww_mutex code
could thus probably use a quick lookup from a list in the
struct task_struct.
Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem_ww.c | 73 +++++++++++++++++++++++++++++-
drivers/gpu/drm/i915/i915_gem_ww.h | 55 +++++++++++++++++++++-
2 files changed, 126 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_ww.c b/drivers/gpu/drm/i915/i915_gem_ww.c
index 3490b72cf613..9b51cb535b65 100644
--- a/drivers/gpu/drm/i915/i915_gem_ww.c
+++ b/drivers/gpu/drm/i915/i915_gem_ww.c
@@ -1,10 +1,12 @@
// SPDX-License-Identifier: MIT
/*
- * Copyright © 2020 Intel Corporation
+ * Copyright © 2019 Intel Corporation
*/
+#include <linux/rhashtable.h>
#include <linux/dma-resv.h>
#include <linux/stacktrace.h>
#include "i915_gem_ww.h"
+#include "i915_globals.h"
#include "gem/i915_gem_object.h"
void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ww, bool intr)
@@ -70,3 +72,72 @@ int __must_check i915_gem_ww_ctx_backoff(struct i915_gem_ww_ctx *ww)
return ret;
}
+
+static struct rhashtable ww_ht;
+static const struct rhashtable_params ww_params = {
+ .key_len = sizeof(struct task_struct *),
+ .key_offset = offsetof(struct i915_gem_ww_ctx, ctx.task),
+ .head_offset = offsetof(struct i915_gem_ww_ctx, head),
+ .min_size = 128,
+};
+
+static void i915_ww_item_free(void *ptr, void *arg)
+{
+ WARN_ON_ONCE(1);
+}
+
+static void i915_global_ww_exit(void)
+{
+ rhashtable_free_and_destroy(&ww_ht, i915_ww_item_free, NULL);
+}
+
+static void i915_global_ww_shrink(void)
+{
+}
+
+static struct i915_global global = {
+ .shrink = i915_global_ww_shrink,
+ .exit = i915_global_ww_exit,
+};
+
+int __init i915_global_ww_init(void)
+{
+ int ret = rhashtable_init(&ww_ht, &ww_params);
+
+ if (ret)
+ return ret;
+
+ i915_global_register(&global);
+
+ return 0;
+}
+
+void __i915_gem_ww_mark_unused(struct i915_gem_ww_ctx *ww)
+{
+ GEM_WARN_ON(rhashtable_remove_fast(&ww_ht, &ww->head, ww_params));
+}
+
+/**
+ * __i915_gem_ww_locate_or_find - return the task's i915_gem_ww_ctx context to use.
+ *
+ * @template: The context to use if there was none initialized previously
+ * in the call chain.
+ *
+ * RETURN: The task's i915_gem_ww_ctx context.
+ */
+struct i915_gem_ww_ctx *
+__i915_gem_ww_locate_or_use(struct i915_gem_ww_ctx *template)
+{
+ struct i915_gem_ww_ctx *tmp;
+
+ /* ctx.task is the hash key, so set it first. */
+ template->ctx.task = current;
+
+ /*
+ * Ideally we'd just hook the active context to the
+ * struct task_struct. But for now use a hash table.
+ */
+ tmp = rhashtable_lookup_get_insert_fast(&ww_ht, &template->head,
+ ww_params);
+ return tmp;
+}
diff --git a/drivers/gpu/drm/i915/i915_gem_ww.h b/drivers/gpu/drm/i915/i915_gem_ww.h
index 94fdf8c5f89b..1a874e2d9f13 100644
--- a/drivers/gpu/drm/i915/i915_gem_ww.h
+++ b/drivers/gpu/drm/i915/i915_gem_ww.h
@@ -6,18 +6,71 @@
#define __I915_GEM_WW_H__
#include <linux/stackdepot.h>
+#include <linux/rhashtable-types.h>
#include <drm/drm_drv.h>
struct i915_gem_ww_ctx {
struct ww_acquire_ctx ctx;
+ struct rhash_head head;
struct list_head obj_list;
struct drm_i915_gem_object *contended;
depot_stack_handle_t contended_bt;
- bool intr;
+ u32 call_depth;
+ unsigned short intr;
+ unsigned short loop;
};
void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ctx, bool intr);
void i915_gem_ww_ctx_fini(struct i915_gem_ww_ctx *ctx);
int __must_check i915_gem_ww_ctx_backoff(struct i915_gem_ww_ctx *ctx);
void i915_gem_ww_unlock_single(struct drm_i915_gem_object *obj);
+
+/* Internal functions used by the inlines! Don't use. */
+void __i915_gem_ww_mark_unused(struct i915_gem_ww_ctx *ww);
+struct i915_gem_ww_ctx *
+__i915_gem_ww_locate_or_use(struct i915_gem_ww_ctx *template);
+
+static inline int __i915_gem_ww_fini(struct i915_gem_ww_ctx *ww, int err)
+{
+ if (ww->call_depth) {
+ ww->call_depth--;
+ return err;
+ }
+
+ if (err == -EDEADLK) {
+ err = i915_gem_ww_ctx_backoff(ww);
+ if (!err)
+ ww->loop = 1;
+ }
+
+ if (!ww->loop) {
+ i915_gem_ww_ctx_fini(ww);
+ __i915_gem_ww_mark_unused(ww);
+ }
+
+ return err;
+}
+
+static inline struct i915_gem_ww_ctx *
+__ww_i915_gem_ww_init(struct i915_gem_ww_ctx *template, bool intr)
+{
+ struct i915_gem_ww_ctx *ww = __i915_gem_ww_locate_or_use(template);
+
+ if (!ww) {
+ ww = template;
+ ww->call_depth = 0;
+ i915_gem_ww_ctx_init(ww, intr);
+ } else {
+ ww->call_depth++;
+ }
+
+ ww->loop = 1;
+
+ return ww;
+}
+
+#define i915_gem_do_ww(_ww, _template, _err, _intr) \
+ for ((_ww) = __i915_gem_ww_init(&(_template), _intr); (_ww)->loop--; \
+ _err = __i915_ww_ww_fini(_ww, _err))
+
#endif
--
2.25.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
