From patchwork Fri Nov 27 12:04:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Matthew Auld X-Patchwork-Id: 11935613 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04A66C2D0E4 for ; Fri, 27 Nov 2020 12:08:37 +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 A0DBB21D81 for ; Fri, 27 Nov 2020 12:08:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A0DBB21D81 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BDADD6EC19; Fri, 27 Nov 2020 12:08:01 +0000 (UTC) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2145B6EBFF; Fri, 27 Nov 2020 12:07:54 +0000 (UTC) IronPort-SDR: 7ojyYevpsXl7Jaq3VZYo3+JUtVxappuBeE04i/rCVoSo/6zd3qo7IKeFMD8MHlq+9LCbNm5nHw YCQgart7WkUw== X-IronPort-AV: E=McAfee;i="6000,8403,9817"; a="168883380" X-IronPort-AV: E=Sophos;i="5.78,374,1599548400"; d="scan'208";a="168883380" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Nov 2020 04:07:52 -0800 IronPort-SDR: xnFWp5S+ice37t3AlO2ruyE7HWJfsEDUfALa9+bMCX3BMEzdI+IyQJGUtq7jKXpXi/CE3/1ePm 4XktJXz0KY7Q== X-IronPort-AV: E=Sophos;i="5.78,374,1599548400"; d="scan'208";a="548028588" Received: from mjgleeso-mobl.ger.corp.intel.com (HELO mwauld-desk1.ger.corp.intel.com) ([10.251.85.2]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Nov 2020 04:07:50 -0800 From: Matthew Auld To: intel-gfx@lists.freedesktop.org Subject: [RFC PATCH 014/162] drm/i915: Ensure we hold the object mutex in pin correctly v2 Date: Fri, 27 Nov 2020 12:04:50 +0000 Message-Id: <20201127120718.454037-15-matthew.auld@intel.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20201127120718.454037-1-matthew.auld@intel.com> References: <20201127120718.454037-1-matthew.auld@intel.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dri-devel@lists.freedesktop.org, =?utf-8?q?Thomas_Hellstr=C3=B6m?= Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Maarten Lankhorst Currently we have a lot of places where we hold the gem object lock, but haven't yet been converted to the ww dance. Complain loudly about those places. i915_vma_pin shouldn't have the obj lock held, so we can do a ww dance, while i915_vma_pin_ww should. Signed-off-by: Maarten Lankhorst Cc: Thomas Hellström --- drivers/gpu/drm/i915/gt/intel_renderstate.c | 2 +- drivers/gpu/drm/i915/gt/intel_timeline.c | 4 +- drivers/gpu/drm/i915/i915_vma.c | 46 +++++++++++++++++++-- drivers/gpu/drm/i915/i915_vma.h | 5 +++ 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_renderstate.c b/drivers/gpu/drm/i915/gt/intel_renderstate.c index ea2a77c7b469..a68e5c23a67c 100644 --- a/drivers/gpu/drm/i915/gt/intel_renderstate.c +++ b/drivers/gpu/drm/i915/gt/intel_renderstate.c @@ -196,7 +196,7 @@ int intel_renderstate_init(struct intel_renderstate *so, if (err) goto err_context; - err = i915_vma_pin(so->vma, 0, 0, PIN_GLOBAL | PIN_HIGH); + err = i915_vma_pin_ww(so->vma, &so->ww, 0, 0, PIN_GLOBAL | PIN_HIGH); if (err) goto err_context; diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c index 479eb5440bc6..b2d04717db20 100644 --- a/drivers/gpu/drm/i915/gt/intel_timeline.c +++ b/drivers/gpu/drm/i915/gt/intel_timeline.c @@ -46,7 +46,7 @@ static int __hwsp_alloc(struct intel_gt *gt, struct intel_timeline_hwsp *hwsp) goto out_unlock; } - /* Pin early so we can call i915_ggtt_pin unlocked. */ + /* Pin early so we can call i915_ggtt_pin_unlocked(). */ hwsp->vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB); if (IS_ERR(hwsp->vaddr)) { ret = PTR_ERR(hwsp->vaddr); @@ -514,7 +514,7 @@ __intel_timeline_get_seqno(struct intel_timeline *tl, goto err_rollback; } - err = i915_ggtt_pin(vma, NULL, 0, PIN_HIGH); + err = i915_ggtt_pin_unlocked(vma, 0, PIN_HIGH); if (err) { __idle_hwsp_free(vma->private, cacheline); goto err_rollback; diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 8e8c80ccbe32..e07621825da9 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -862,7 +862,8 @@ int i915_vma_pin_ww(struct i915_vma *vma, struct i915_gem_ww_ctx *ww, unsigned int bound; int err; - if (IS_ENABLED(CONFIG_PROVE_LOCKING) && debug_locks) { +#ifdef CONFIG_PROVE_LOCKING + if (debug_locks) { bool pinned_bind_wo_alloc = vma->obj && i915_gem_object_has_pinned_pages(vma->obj) && !vma->vm->allocate_va_range; @@ -870,7 +871,10 @@ int i915_vma_pin_ww(struct i915_vma *vma, struct i915_gem_ww_ctx *ww, if (lockdep_is_held(&vma->vm->i915->drm.struct_mutex) && !pinned_bind_wo_alloc) WARN_ON(!ww); + if (ww && vma->resv) + assert_vma_held(vma); } +#endif BUILD_BUG_ON(PIN_GLOBAL != I915_VMA_GLOBAL_BIND); BUILD_BUG_ON(PIN_USER != I915_VMA_LOCAL_BIND); @@ -1017,8 +1021,8 @@ static void flush_idle_contexts(struct intel_gt *gt) intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT); } -int i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww, - u32 align, unsigned int flags) +static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww, + u32 align, unsigned int flags, bool unlocked) { struct i915_address_space *vm = vma->vm; int err; @@ -1026,7 +1030,10 @@ int i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww, GEM_BUG_ON(!i915_vma_is_ggtt(vma)); do { - err = i915_vma_pin_ww(vma, ww, 0, align, flags | PIN_GLOBAL); + if (ww || unlocked) + err = i915_vma_pin_ww(vma, ww, 0, align, flags | PIN_GLOBAL); + else + err = i915_vma_pin(vma, 0, align, flags | PIN_GLOBAL); if (err != -ENOSPC) { if (!err) { err = i915_vma_wait_for_bind(vma); @@ -1045,6 +1052,37 @@ int i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww, } while (1); } +int i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww, + u32 align, unsigned int flags) +{ +#ifdef CONFIG_LOCKDEP + WARN_ON(!ww && vma->resv && dma_resv_held(vma->resv)); +#endif + + return __i915_ggtt_pin(vma, ww, align, flags, false); +} + +/** + * i915_ggtt_pin_unlocked - Pin a vma to ggtt without the underlying + * object's dma-resv held, but with object pages pinned. + * + * @vma: The vma to pin. + * @align: ggtt alignment. + * @flags: Pinning flags + * + * RETURN: Zero on success, negative error code on error. + * + * This function relies on the fact that object pages are already pinned, + * and that ggtt pinning doesn't require any page table page allocations + * to pin a vma without dma_resv lock and ww acquire context. + */ +int i915_ggtt_pin_unlocked(struct i915_vma *vma, u32 align, unsigned int flags) +{ + if (IS_ENABLED(CONFIG_LOCKDEP)) + WARN_ON(vma->obj && !i915_gem_object_has_pinned_pages(vma->obj)); + return __i915_ggtt_pin(vma, NULL, align, flags, true); +} + static void __vma_close(struct i915_vma *vma, struct intel_gt *gt) { /* diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h index 5b3a3c653454..22387a361999 100644 --- a/drivers/gpu/drm/i915/i915_vma.h +++ b/drivers/gpu/drm/i915/i915_vma.h @@ -243,12 +243,17 @@ i915_vma_pin_ww(struct i915_vma *vma, struct i915_gem_ww_ctx *ww, static inline int __must_check i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags) { +#ifdef CONFIG_LOCKDEP + WARN_ON_ONCE(vma->resv && dma_resv_held(vma->resv)); +#endif return i915_vma_pin_ww(vma, NULL, size, alignment, flags); } int i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww, u32 align, unsigned int flags); +int i915_ggtt_pin_unlocked(struct i915_vma *vma, u32 align, unsigned int flags); + static inline int i915_vma_pin_count(const struct i915_vma *vma) { return atomic_read(&vma->flags) & I915_VMA_PIN_MASK;