From patchwork Thu Dec 1 14:06:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Nicolai_H=C3=A4hnle?= X-Patchwork-Id: 9456241 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B30B360761 for ; Thu, 1 Dec 2016 14:07:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A64C428469 for ; Thu, 1 Dec 2016 14:07:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9661A284E3; Thu, 1 Dec 2016 14:07:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 2777728469 for ; Thu, 1 Dec 2016 14:07:41 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DA4E56E80D; Thu, 1 Dec 2016 14:07:17 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wm0-x241.google.com (mail-wm0-x241.google.com [IPv6:2a00:1450:400c:c09::241]) by gabe.freedesktop.org (Postfix) with ESMTPS id 87EE56E7F6 for ; Thu, 1 Dec 2016 14:07:11 +0000 (UTC) Received: by mail-wm0-x241.google.com with SMTP id a20so34468602wme.2 for ; Thu, 01 Dec 2016 06:07:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=xR310Sywq7XZkJhqCJy5r3XRVbXgFS7yQyOPqNu9cJM=; b=NNMqU78cecUWzmnWPL6UMN630+ggvSN8KFo3OPqW25MH1kZet+YdpnxKgfupW4L9t0 8pvt5jyjLqODFmb/+nCJrbOKrY56ZB16UlvG33gfVvrY8MT4W2IyZ53jLBRSNq9BMEXs iRMC8Yngd8kVbnFiDjxXYdd7mW4Fgvnrl1AN85lwHhFkfVhWKIEmZSoBz088aB6boanu Yw+SnW82hi3eBl3Cz24C5bF0c6uy9F3GSSmbRI3bJhT5oY2IFRuaZEuuL8dh3QLwftAe BqT0mK2OMG2ZxLzB6tXgYX1Y6iJf2xyJ969DafWSqNEC1QVxB3ItkgztnJ+rtjEOycCk e8JA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=xR310Sywq7XZkJhqCJy5r3XRVbXgFS7yQyOPqNu9cJM=; b=izneIKPGvoC2of6qxlvgQgo1QJ4cfiokg0ZzEN2ISys6gZDRecLbbB/IAaHdHx/g5h 2Mx80AphKdZNpMJTCLR4LlMMR2BsLsYSfzQJT8+Y7SeSy3p7TicEfMfRV/Fl1IwAZWll zf5RnUEVdoPJaUVQEya5KvkhwWbRgtSjqwjxtHBINNTyynEco1j/9Gg1brNREuobwn6O joQhQEFNQor433AbzjR2jUknTdw4+psOFQhXJ3ILP54S+TmwLguvau/TyfFlZ5v8P7pg BZo1T6hq3uOmdxpllnNfRko5q8UYRL63OZDza/Bwao8OOBwxVZlRGfaPyp29+i14fMkd QiVw== X-Gm-Message-State: AKaTC023mEA6xA5IlQqQck6pvlCwl+Tr7wKgiO0AkAWjfQGXQPbklRZP2+QJT0c5SzChhQ== X-Received: by 10.28.169.4 with SMTP id s4mr32177793wme.65.1480601229680; Thu, 01 Dec 2016 06:07:09 -0800 (PST) Received: from cassiopeia.fritz.box ([2001:a61:116e:7d01:dc12:8e88:ab9d:8694]) by smtp.gmail.com with ESMTPSA id vr9sm411514wjc.35.2016.12.01.06.07.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 01 Dec 2016 06:07:09 -0800 (PST) From: =?UTF-8?q?Nicolai=20H=C3=A4hnle?= To: linux-kernel@vger.kernel.org Subject: [PATCH v2 08/11] locking/ww_mutex: Yield to other waiters from optimistic spin Date: Thu, 1 Dec 2016 15:06:51 +0100 Message-Id: <1480601214-26583-9-git-send-email-nhaehnle@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1480601214-26583-1-git-send-email-nhaehnle@gmail.com> References: <1480601214-26583-1-git-send-email-nhaehnle@gmail.com> MIME-Version: 1.0 Cc: Maarten Lankhorst , =?UTF-8?q?Nicolai=20H=C3=A4hnle?= , Peter Zijlstra , dri-devel@lists.freedesktop.org, Ingo Molnar X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP From: Nicolai Hähnle Lock stealing is less beneficial for w/w mutexes since we may just end up backing off if we stole from a thread with an earlier acquire stamp that already holds another w/w mutex that we also need. So don't spin optimistically unless we are sure that there is no other waiter that might cause us to back off. Median timings taken of a contention-heavy GPU workload: Before: real 0m52.946s user 0m7.272s sys 1m55.964s After: real 0m53.086s user 0m7.360s sys 1m46.204s This particular workload still spends 20%-25% of CPU in mutex_spin_on_owner according to perf, but my attempts to further reduce this spinning based on various heuristics all lead to an increase in measured wall time despite the decrease in sys time. Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Maarten Lankhorst Cc: Daniel Vetter Cc: Chris Wilson Cc: dri-devel@lists.freedesktop.org Signed-off-by: Nicolai Hähnle --- kernel/locking/mutex.c | 48 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c index d2ca447..296605c 100644 --- a/kernel/locking/mutex.c +++ b/kernel/locking/mutex.c @@ -374,7 +374,8 @@ ww_mutex_set_context_slowpath(struct ww_mutex *lock, */ static noinline bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner, - bool use_ww_ctx, struct ww_acquire_ctx *ww_ctx) + bool use_ww_ctx, struct ww_acquire_ctx *ww_ctx, + struct mutex_waiter *waiter) { bool ret = true; @@ -397,7 +398,7 @@ bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner, break; } - if (use_ww_ctx && ww_ctx && ww_ctx->acquired > 0) { + if (use_ww_ctx && ww_ctx) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -413,7 +414,30 @@ bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner, * Check this in every inner iteration because we may * be racing against another thread's ww_mutex_lock. */ - if (READ_ONCE(ww->ctx)) { + if (ww_ctx->acquired > 0 && READ_ONCE(ww->ctx)) { + ret = false; + break; + } + + /* + * If we aren't on the wait list yet, cancel the spin + * if there are waiters. We want to avoid stealing the + * lock from a waiter with an earlier stamp, since the + * other thread may already own a lock that we also + * need. + */ + if (!waiter && + (atomic_long_read(&lock->owner) & + MUTEX_FLAG_WAITERS)) { + ret = false; + break; + } + + /* + * Similarly, stop spinning if we are no longer the + * first waiter. + */ + if (waiter && !__mutex_waiter_is_first(lock, waiter)) { ret = false; break; } @@ -479,7 +503,8 @@ static inline int mutex_can_spin_on_owner(struct mutex *lock) */ static bool mutex_optimistic_spin(struct mutex *lock, struct ww_acquire_ctx *ww_ctx, - const bool use_ww_ctx, const bool waiter) + const bool use_ww_ctx, + struct mutex_waiter *waiter) { struct task_struct *task = current; @@ -518,12 +543,12 @@ static bool mutex_optimistic_spin(struct mutex *lock, } if (!mutex_spin_on_owner(lock, owner, use_ww_ctx, - ww_ctx)) + ww_ctx, waiter)) goto fail_unlock; } /* Try to acquire the mutex if it is unlocked. */ - if (__mutex_trylock(lock, waiter)) + if (__mutex_trylock(lock, waiter != NULL)) break; /* @@ -565,7 +590,8 @@ static bool mutex_optimistic_spin(struct mutex *lock, #else static bool mutex_optimistic_spin(struct mutex *lock, struct ww_acquire_ctx *ww_ctx, - const bool use_ww_ctx, const bool waiter) + const bool use_ww_ctx, + struct mutex_waiter *waiter) { return false; } @@ -736,7 +762,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, mutex_acquire_nest(&lock->dep_map, subclass, 0, nest_lock, ip); if (__mutex_trylock(lock, false) || - mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx, false)) { + mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx, NULL)) { /* got the lock, yay! */ lock_acquired(&lock->dep_map, ip); if (use_ww_ctx && ww_ctx) { @@ -841,8 +867,10 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, * state back to RUNNING and fall through the next schedule(), * or we must see its unlock and acquire. */ - if ((first && mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx, true)) || - __mutex_trylock(lock, use_ww_ctx || first)) + if ((first && + mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx, + &waiter)) || + __mutex_trylock(lock, use_ww_ctx || first)) break; spin_lock_mutex(&lock->wait_lock, flags);