From patchwork Wed Jan 31 12:20:46 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Will Deacon X-Patchwork-Id: 126325 Delivered-To: patch@linaro.org Received: by 10.46.124.24 with SMTP id x24csp672920ljc; Wed, 31 Jan 2018 04:20:59 -0800 (PST) X-Google-Smtp-Source: AH8x224n2+zO7CXpvp7jquSVDh3DQaM2ZobNHF7U/cqz3x+E5zi5RoJBgqBDlN2ErHy/IH8hbzFc X-Received: by 2002:a17:902:424:: with SMTP id 33-v6mr27931480ple.57.1517401259330; Wed, 31 Jan 2018 04:20:59 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1517401259; cv=none; d=google.com; s=arc-20160816; b=X4UNgePJZkFJz13KziKG7H1z8hgKSdJSxSrtTDXjxah6FlqemQMZI5PGORbshdYIsn wc4KXjuD0+RpGn4O52q+F4gmY8Xg6tYlBT4WRFgaqnJkNDU1w/m/rgRAUNTn1vN/ukFm zT3rtnkodIW1cmq/EFBdwqwr4/c3mOBo/bod/xA97mMLOiI2XXBF0JzHbUkci01WiYHk s2nNGF0h4qpmJBQYc3XSNJVG/gk1LAble3ZXU+J9G9stVZCwbHJcs+OJJ4aN6Bc/7T4F SszgYZHNyjVEQ3O1PhyDD8wbqVV8F93pBoKwt0HI+Aq7WSaVnt99XxpdKUm/21fqha4P +JPA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :arc-authentication-results; bh=vB4c5Ll6jtZXdTjA371wVzq2w1B2AUInu2d/XstFjs0=; b=NL/LCUa6KW49kmKXgoCdWnHy7iMjMl0bcMCqY0UqHETwVovR3VUeAtEiZOymK2H6wm /HuMAyxRXn5inZRegfXS9kJ7nMeNoR5aXOrUHUtC+dUAikh6IhTNfxX8MJlh/7IcED0R XagapARnS8krje6OulsUtMGeM6CJaZK3lKGSgZEzirqcLji8DnRyOcNENMlWlMYlfGH4 rUiEtSvxFoWJXlSm6WLZ50afji3NilPF3tI+wbVx3EHbwJUZwoiwpgnMh+u4HrKMD2XQ bKlQXbqRVYMQ449SHtdSTRRf6rgrbF66YkFnkH/aOZXt5z2UTaRQBRqT6lhC/qzXpYom L8pQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id l8si10937346pgf.140.2018.01.31.04.20.59; Wed, 31 Jan 2018 04:20:59 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752887AbeAaMU5 (ORCPT + 27 others); Wed, 31 Jan 2018 07:20:57 -0500 Received: from foss.arm.com ([217.140.101.70]:37426 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751662AbeAaMU4 (ORCPT ); Wed, 31 Jan 2018 07:20:56 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8478E1435; Wed, 31 Jan 2018 04:20:46 -0800 (PST) Received: from edgewater-inn.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 559713F487; Wed, 31 Jan 2018 04:20:46 -0800 (PST) Received: by edgewater-inn.cambridge.arm.com (Postfix, from userid 1000) id E4B651AE10C2; Wed, 31 Jan 2018 12:20:47 +0000 (GMT) From: Will Deacon To: linux-kernel@vger.kernel.org Cc: Will Deacon , Peter Zijlstra , Ingo Molnar Subject: [PATCH] locking/qspinlock: Ensure node is initialised before updating prev->next Date: Wed, 31 Jan 2018 12:20:46 +0000 Message-Id: <1517401246-2750-1-git-send-email-will.deacon@arm.com> X-Mailer: git-send-email 2.1.4 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org When a locker ends up queuing on the qspinlock locking slowpath, we initialise the relevant mcs node and publish it indirectly by updating the tail portion of the lock word using xchg_tail. If we find that there was a pre-existing locker in the queue, we subsequently update their ->next field to point at our node so that we are notified when it's our turn to take the lock. This can be roughly illustrated as follows: /* Initialise the fields in node and encode a pointer to node in tail */ tail = initialise_node(node); /* * Exchange tail into the lockword using an atomic read-modify-write * operation with release semantics */ old = xchg_tail(lock, tail); /* If there was a pre-existing waiter ... */ if (old & _Q_TAIL_MASK) { prev = decode_tail(old); smp_read_barrier_depends(); /* ... then update their ->next field to point to node. WRITE_ONCE(prev->next, node); } The conditional update of prev->next therefore relies on the address dependency from the result of xchg_tail ensuring order against the prior initialisation of node. However, since the release semantics of the xchg_tail operation apply only to the write portion of the RmW, then this ordering is not guaranteed and it is possible for the CPU to return old before the writes to node have been published, consequently allowing us to point prev->next to an uninitialised node. This patch fixes the problem by making the update of prev->next a RELEASE operation, which also removes the reliance on dependency ordering. Cc: Peter Zijlstra Cc: Ingo Molnar Signed-off-by: Will Deacon --- kernel/locking/qspinlock.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) -- 2.1.4 diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c index 294294c71ba4..1ebbc366a31d 100644 --- a/kernel/locking/qspinlock.c +++ b/kernel/locking/qspinlock.c @@ -408,16 +408,15 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val) */ if (old & _Q_TAIL_MASK) { prev = decode_tail(old); + /* - * The above xchg_tail() is also a load of @lock which generates, - * through decode_tail(), a pointer. - * - * The address dependency matches the RELEASE of xchg_tail() - * such that the access to @prev must happen after. + * We must ensure that the stores to @node are observed before + * the write to prev->next. The address dependency on xchg_tail + * is not sufficient to ensure this because the read component + * of xchg_tail is unordered with respect to the initialisation + * of node. */ - smp_read_barrier_depends(); - - WRITE_ONCE(prev->next, node); + smp_store_release(prev->next, node); pv_wait_node(node, prev); arch_mcs_spin_lock_contended(&node->locked);