From patchwork Fri Nov 13 04:47:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kai-Heng Feng X-Patchwork-Id: 1399542 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CXQv721kNz9sVL; Fri, 13 Nov 2020 15:47:45 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1kdQzu-0004p1-63; Fri, 13 Nov 2020 04:47:42 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1kdQzs-0004oj-7q for kernel-team@lists.ubuntu.com; Fri, 13 Nov 2020 04:47:40 +0000 Received: from 61-220-137-34.hinet-ip.hinet.net ([61.220.137.34] helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1kdQzr-0007hL-C1 for kernel-team@lists.ubuntu.com; Fri, 13 Nov 2020 04:47:40 +0000 From: Kai-Heng Feng To: kernel-team@lists.ubuntu.com Subject: [PATCH 1/1] Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire callbacks" Date: Fri, 13 Nov 2020 12:47:31 +0800 Message-Id: <20201113044731.570886-2-kai.heng.feng@canonical.com> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20201113044731.570886-1-kai.heng.feng@canonical.com> References: <20201113044731.570886-1-kai.heng.feng@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" BugLink: https://bugs.launchpad.net/bugs/1896091 This reverts commit c1484c993957fe6b366ab453fbb7b50c0d2916de. Signed-off-by: Kai-Heng Feng Acked-by: Stefan Bader Acked-by: Kleber Sacilotto de Souza --- drivers/gpu/drm/i915/i915_active.c | 52 ++++-------------------- drivers/gpu/drm/i915/i915_active.h | 10 +++-- drivers/gpu/drm/i915/i915_active_types.h | 2 - 3 files changed, 14 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c index 768713657e85..d960d0be5bd2 100644 --- a/drivers/gpu/drm/i915/i915_active.c +++ b/drivers/gpu/drm/i915/i915_active.c @@ -148,22 +148,8 @@ __active_retire(struct i915_active *ref) spin_unlock_irqrestore(&ref->tree_lock, flags); /* After the final retire, the entire struct may be freed */ - if (ref->retire) { - if (ref->active) { - bool freed = false; - - /* Don't race with the active callback, and avoid UaF */ - down_write(&ref->rwsem); - ref->freed = &freed; - ref->retire(ref); - if (!freed) { - ref->freed = NULL; - up_write(&ref->rwsem); - } - } else { - ref->retire(ref); - } - } + if (ref->retire) + ref->retire(ref); /* ... except if you wait on it, you must manage your own references! */ wake_up_var(ref); @@ -293,8 +279,7 @@ void __i915_active_init(struct i915_active *ref, int (*active)(struct i915_active *ref), void (*retire)(struct i915_active *ref), struct lock_class_key *mkey, - struct lock_class_key *wkey, - struct lock_class_key *rkey) + struct lock_class_key *wkey) { unsigned long bits; @@ -303,13 +288,8 @@ void __i915_active_init(struct i915_active *ref, ref->flags = 0; ref->active = active; ref->retire = ptr_unpack_bits(retire, &bits, 2); - ref->freed = NULL; - if (ref->active && ref->retire) { - __init_rwsem(&ref->rwsem, "i915_active.rwsem", rkey); + if (bits & I915_ACTIVE_MAY_SLEEP) ref->flags |= I915_ACTIVE_RETIRE_SLEEPS; - } else if (bits & I915_ACTIVE_MAY_SLEEP) { - ref->flags |= I915_ACTIVE_RETIRE_SLEEPS; - } spin_lock_init(&ref->tree_lock); ref->tree = RB_ROOT; @@ -448,20 +428,8 @@ int i915_active_acquire(struct i915_active *ref) return err; if (likely(!i915_active_acquire_if_busy(ref))) { - if (ref->active) { - if (ref->retire) { - /* - * This can be a recursive call, and the mutex - * above already protects from concurrent active - * callbacks, so a read lock fits best. - */ - down_read(&ref->rwsem); - err = ref->active(ref); - up_read(&ref->rwsem); - } else { - err = ref->active(ref); - } - } + if (ref->active) + err = ref->active(ref); if (!err) { spin_lock_irq(&ref->tree_lock); /* __active_retire() */ debug_active_activate(ref); @@ -683,20 +651,16 @@ int i915_sw_fence_await_active(struct i915_sw_fence *fence, return await_active(ref, flags, sw_await_fence, fence, fence); } +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) void i915_active_fini(struct i915_active *ref) { - if (ref->freed) { - *ref->freed = true; - up_write(&ref->rwsem); - } -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) debug_active_fini(ref); GEM_BUG_ON(atomic_read(&ref->count)); GEM_BUG_ON(work_pending(&ref->work)); GEM_BUG_ON(!RB_EMPTY_ROOT(&ref->tree)); mutex_destroy(&ref->mutex); -#endif } +#endif static inline bool is_idle_barrier(struct active_node *node, u64 idx) { diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h index a2e3742b1090..cf4058150966 100644 --- a/drivers/gpu/drm/i915/i915_active.h +++ b/drivers/gpu/drm/i915/i915_active.h @@ -153,16 +153,14 @@ void __i915_active_init(struct i915_active *ref, int (*active)(struct i915_active *ref), void (*retire)(struct i915_active *ref), struct lock_class_key *mkey, - struct lock_class_key *wkey, - struct lock_class_key *rkey); + struct lock_class_key *wkey); /* Specialise each class of i915_active to avoid impossible lockdep cycles. */ #define i915_active_init(ref, active, retire) do { \ static struct lock_class_key __mkey; \ static struct lock_class_key __wkey; \ - static struct lock_class_key __rkey; \ \ - __i915_active_init(ref, active, retire, &__mkey, &__wkey, &__rkey); \ + __i915_active_init(ref, active, retire, &__mkey, &__wkey); \ } while (0) int i915_active_ref(struct i915_active *ref, @@ -215,7 +213,11 @@ i915_active_is_idle(const struct i915_active *ref) return !atomic_read(&ref->count); } +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) void i915_active_fini(struct i915_active *ref); +#else +static inline void i915_active_fini(struct i915_active *ref) { } +#endif int i915_active_acquire_preallocate_barrier(struct i915_active *ref, struct intel_engine_cs *engine); diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h index aaee2548cb19..6360c3e4b765 100644 --- a/drivers/gpu/drm/i915/i915_active_types.h +++ b/drivers/gpu/drm/i915/i915_active_types.h @@ -32,8 +32,6 @@ struct active_node; struct i915_active { atomic_t count; struct mutex mutex; - struct rw_semaphore rwsem; - bool *freed; spinlock_t tree_lock; struct active_node *cache;