From patchwork Thu Sep 17 18:59:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Thomas_Hellstr=C3=B6m_=28Intel=29?= X-Patchwork-Id: 11783387 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id AB4E359D for ; Thu, 17 Sep 2020 19:00:16 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 59EE321973 for ; Thu, 17 Sep 2020 19:00:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=shipmail.org header.i=@shipmail.org header.b="PubuCQip" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 59EE321973 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=shipmail.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 994836EC7B; Thu, 17 Sep 2020 19:00:15 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from pio-pvt-msa3.bahnhof.se (pio-pvt-msa3.bahnhof.se [79.136.2.42]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7A91A6EC7B for ; Thu, 17 Sep 2020 19:00:13 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by pio-pvt-msa3.bahnhof.se (Postfix) with ESMTP id D692A3F6FB; Thu, 17 Sep 2020 21:00:11 +0200 (CEST) Authentication-Results: pio-pvt-msa3.bahnhof.se; dkim=pass (1024-bit key; unprotected) header.d=shipmail.org header.i=@shipmail.org header.b=PubuCQip; dkim-atps=neutral X-Virus-Scanned: Debian amavisd-new at bahnhof.se X-Spam-Flag: NO X-Spam-Score: -2.099 X-Spam-Level: X-Spam-Status: No, score=-2.099 tagged_above=-999 required=6.31 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no Received: from pio-pvt-msa3.bahnhof.se ([127.0.0.1]) by localhost (pio-pvt-msa3.bahnhof.se [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Hlf-mQnAZjO1; Thu, 17 Sep 2020 21:00:11 +0200 (CEST) Received: by pio-pvt-msa3.bahnhof.se (Postfix) with ESMTPA id CCBFD3F5FA; Thu, 17 Sep 2020 21:00:10 +0200 (CEST) Received: from localhost.localdomain (h-205-35.A357.priv.bahnhof.se [155.4.205.35]) by mail1.shipmail.org (Postfix) with ESMTPSA id 910EB361D84; Thu, 17 Sep 2020 21:00:10 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=shipmail.org; s=mail; t=1600369210; bh=vkHtKj0mJKgtD3QJDtDFrtLnkrxF3ZvGSjXxrbDdKEA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=PubuCQipTqVgCV05vlGp9OvnprmxBncuHZFFJUN9BBxxyYIb/LMKqOX2C9EeQxMWj Z4fateLbVo+KxoM+lxoI/FUntttcEX5qRyJyxKMZIbP7f57JC7m+0rgMaOf+ThyLXW 5T8+IMd91fnbLbGDW5JhlxRUGSniDXJI72P5VZhM= From: =?utf-8?q?Thomas_Hellstr=C3=B6m_=28Intel=29?= To: intel-gfx@lists.freedesktop.org Date: Thu, 17 Sep 2020 20:59:45 +0200 Message-Id: <20200917185945.11734-3-thomas_os@shipmail.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200917185945.11734-1-thomas_os@shipmail.org> References: <20200917185945.11734-1-thomas_os@shipmail.org> MIME-Version: 1.0 Subject: [Intel-gfx] [RFC PATCH v2 2/2] drm/i915: Introduce a i915_gem_do_ww(){} utility X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: =?utf-8?q?Thomas_Hellstr=C3=B6m?= , maarten.lankhorst@intel.com, chris@chris-wilson.co.uk Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" From: Thomas Hellström 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 --- drivers/gpu/drm/i915/i915_gem_ww.c | 74 ++++++++++++++++++++++++++++- drivers/gpu/drm/i915/i915_gem_ww.h | 56 +++++++++++++++++++++- drivers/gpu/drm/i915/i915_globals.c | 1 + drivers/gpu/drm/i915/i915_globals.h | 1 + 4 files changed, 130 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..6247af1dba87 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 #include #include #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,73 @@ 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_use - 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..b844596067c7 100644 --- a/drivers/gpu/drm/i915/i915_gem_ww.h +++ b/drivers/gpu/drm/i915/i915_gem_ww.h @@ -6,18 +6,72 @@ #define __I915_GEM_WW_H__ #include +#include #include 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) +{ + ww->loop = 0; + 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 * +__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_gem_ww_fini(_ww, _err)) + #endif diff --git a/drivers/gpu/drm/i915/i915_globals.c b/drivers/gpu/drm/i915/i915_globals.c index 3aa213684293..9087cc8c2ee3 100644 --- a/drivers/gpu/drm/i915/i915_globals.c +++ b/drivers/gpu/drm/i915/i915_globals.c @@ -94,6 +94,7 @@ static __initconst int (* const initfn[])(void) = { i915_global_request_init, i915_global_scheduler_init, i915_global_vma_init, + i915_global_ww_init, }; int __init i915_globals_init(void) diff --git a/drivers/gpu/drm/i915/i915_globals.h b/drivers/gpu/drm/i915/i915_globals.h index b2f5cd9b9b1a..5976b460ee39 100644 --- a/drivers/gpu/drm/i915/i915_globals.h +++ b/drivers/gpu/drm/i915/i915_globals.h @@ -34,5 +34,6 @@ int i915_global_objects_init(void); int i915_global_request_init(void); int i915_global_scheduler_init(void); int i915_global_vma_init(void); +int i915_global_ww_init(void); #endif /* _I915_GLOBALS_H_ */