From patchwork Fri Dec 11 17:46:41 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Will Deacon X-Patchwork-Id: 58302 Delivered-To: patch@linaro.org Received: by 10.112.73.68 with SMTP id j4csp28306lbv; Fri, 11 Dec 2015 09:47:35 -0800 (PST) X-Received: by 10.66.248.74 with SMTP id yk10mr26559434pac.17.1449856055125; Fri, 11 Dec 2015 09:47:35 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id u79si2417582pfa.82.2015.12.11.09.47.34; Fri, 11 Dec 2015 09:47:35 -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 S1751157AbbLKRrc (ORCPT + 28 others); Fri, 11 Dec 2015 12:47:32 -0500 Received: from cam-admin0.cambridge.arm.com ([217.140.96.50]:54701 "EHLO cam-admin0.cambridge.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750748AbbLKRrb (ORCPT ); Fri, 11 Dec 2015 12:47:31 -0500 Received: from edgewater-inn.cambridge.arm.com (edgewater-inn.cambridge.arm.com [10.1.203.29]) by cam-admin0.cambridge.arm.com (8.12.6/8.12.6) with ESMTP id tBBHkYWr019579; Fri, 11 Dec 2015 17:46:34 GMT Received: by edgewater-inn.cambridge.arm.com (Postfix, from userid 1000) id 1B6EF1AE30B0; Fri, 11 Dec 2015 17:46:42 +0000 (GMT) From: Will Deacon To: peterz@infradead.org Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, andrew.pinski@caviumnetworks.com, ddaney.cavm@gmail.com, dbueso@suse.de, Will Deacon Subject: [PATCH] locking/osq: fix ordering of node initialisation in osq_lock Date: Fri, 11 Dec 2015 17:46:41 +0000 Message-Id: <1449856001-21177-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 The Cavium guys reported a soft lockup on their arm64 machine, caused by c55a6ffa6285 ("locking/osq: Relax atomic semantics"): [ 68.909948] [] mutex_optimistic_spin+0x9c/0x1d0 [ 68.909951] [] __mutex_lock_slowpath+0x44/0x158 [ 68.909953] [] mutex_lock+0x54/0x58 [ 68.909956] [] kernfs_iop_permission+0x38/0x70 [ 68.909959] [] __inode_permission+0x88/0xd8 [ 68.909961] [] inode_permission+0x30/0x6c [ 68.909964] [] link_path_walk+0x68/0x4d4 [ 68.909966] [] path_openat+0xb4/0x2bc [ 68.909968] [] do_filp_open+0x74/0xd0 [ 68.909971] [] do_sys_open+0x14c/0x228 [ 68.909973] [] SyS_openat+0x3c/0x48 [ 68.909976] [] el0_svc_naked+0x24/0x28 This is because in osq_lock we initialise the node for the current CPU: node->locked = 0; node->next = NULL; node->cpu = curr; and then publish the current CPU in the lock tail: old = atomic_xchg_acquire(&lock->tail, curr); Once the update to lock->tail is visible to another CPU, the node is then live and can be both read and updated by concurrent lockers. Unfortunately, the ACQUIRE semantics of the xchg operation mean that there is no guarantee the contents of the node will be visible before lock tail is updated. This can lead to lock corruption when, for example, a concurrent locker races to set the next field. Fixes: c55a6ffa6285 ("locking/osq: Relax atomic semantics"): Reported-by: David Daney Reported-by: Andrew Pinski Tested-by: Andrew Pinski Acked-by: Davidlohr Bueso Signed-off-by: Will Deacon --- kernel/locking/osq_lock.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c index d092a0c9c2d4..05a37857ab55 100644 --- a/kernel/locking/osq_lock.c +++ b/kernel/locking/osq_lock.c @@ -93,10 +93,12 @@ bool osq_lock(struct optimistic_spin_queue *lock) node->cpu = curr; /* - * ACQUIRE semantics, pairs with corresponding RELEASE - * in unlock() uncontended, or fastpath. + * We need both ACQUIRE (pairs with corresponding RELEASE in + * unlock() uncontended, or fastpath) and RELEASE (to publish + * the node fields we just initialised) semantics when updating + * the lock tail. */ - old = atomic_xchg_acquire(&lock->tail, curr); + old = atomic_xchg(&lock->tail, curr); if (old == OSQ_UNLOCKED_VAL) return true;