From patchwork Mon Nov 28 12:20:09 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: 9449489 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 D3FC7600CB for ; Mon, 28 Nov 2016 12:21:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C3F6220453 for ; Mon, 28 Nov 2016 12:21:30 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B89D427BFF; Mon, 28 Nov 2016 12:21:30 +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 554C020453 for ; Mon, 28 Nov 2016 12:21:30 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7A23589D5B; Mon, 28 Nov 2016 12:21:26 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2FDFD6E2BD for ; Mon, 28 Nov 2016 12:20:31 +0000 (UTC) Received: by mail-wm0-x244.google.com with SMTP id u144so18623629wmu.0 for ; Mon, 28 Nov 2016 04:20:31 -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=bjZqgt1oSuOfqA638hOo8zFXHfUXI//HqeKk/5EDhXQ=; b=b2uKZHomnhUuKJn08zSzM0tQxeP6dsxsPGUhF93MgZHWjniIeOtLqnECSpGzHDdthX noE6pyXqJKhbVmwLH3y4T/z/feaPVPyKHDnQ28ixjFdh6ro+LnxQSrELgygiiMjVgbIT vrCJBSlqKk5ingpvRQkovkOzUuPf+K+oFY93Fm34NsXKl5osS6RlB4wd2zaf9BjRHjvK 7CezesvRaUFu8M/A6aqZN9BrjIO5d54I92TWzwdaF9YpGlNmdjaiSEHYNtTmVshhibt9 XFspJy+/51ceCkZ+4URcEYA1+5h2EqxH3cenEOIXLDq0rwPXjs/oT77Um08yixB/npcT rqow== 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=bjZqgt1oSuOfqA638hOo8zFXHfUXI//HqeKk/5EDhXQ=; b=ktVPAlHOQFWY51MXI9TO1KA/Tu5B6AVKfQq1D7WpxjkvV/2ea7K4YBrK0Cai4p1O4E 3+oTpCUJ9tF9oKgBjcCBzdh1uNCm2+XkFwcLZLKOGokV6qT73GL4OnSQTa3no/b1JaQs Qw41lvvnrkgJMZ/f+dTetI+Tv8NVitlTT+Yb5Zh0Iu16cb2/2Shs2f6sKgBl8KwYmo+p szW3vLcJX4K4JjzODsEaG7pAMA8+zjUW/PiwoMFt1OFCnBQe+tHtwNC6BKM78C2FiiXV j1a9gDQ3e/bgnmTKbfBUOedK69ZBygKJD9Gk6XyBcth4QVfyiWInoMg7s5dMyQgk4Vim 5euw== X-Gm-Message-State: AKaTC02VnQQQsYgYgcqFLQYH+/sErYbUgtnK8Yoy/HZ3S+Jg8mYPcYCYjJKtCCT99GLFbA== X-Received: by 10.28.11.208 with SMTP id 199mr19828465wml.97.1480335629603; Mon, 28 Nov 2016 04:20:29 -0800 (PST) Received: from cassiopeia.fritz.box ([2001:a61:1119:dc01:d88b:9432:f601:27ed]) by smtp.gmail.com with ESMTPSA id a13sm28830511wma.18.2016.11.28.04.20.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 28 Nov 2016 04:20:29 -0800 (PST) From: =?UTF-8?q?Nicolai=20H=C3=A4hnle?= To: linux-kernel@vger.kernel.org Subject: [PATCH 08/11] locking/ww_mutex: Yield to other waiters from optimistic spin Date: Mon, 28 Nov 2016 13:20:09 +0100 Message-Id: <1480335612-12069-9-git-send-email-nhaehnle@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1480335612-12069-1-git-send-email-nhaehnle@gmail.com> References: <1480335612-12069-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 0f6f1da..1e4da21 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; } @@ -725,7 +751,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) { @@ -830,8 +856,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);