From patchwork Tue Nov 28 18:42:18 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Will Deacon X-Patchwork-Id: 119897 Delivered-To: patch@linaro.org Received: by 10.140.22.227 with SMTP id 90csp2072623qgn; Tue, 28 Nov 2017 10:42:24 -0800 (PST) X-Google-Smtp-Source: AGs4zMYJgHmD5ZZtov8uJ/QfV7w7T5YXmnYYymys5eFPjwZ5uwPOf1f/xgyadWgxbHr1lkfef0Y8 X-Received: by 10.99.171.77 with SMTP id k13mr80115pgp.229.1511894543971; Tue, 28 Nov 2017 10:42:23 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1511894543; cv=none; d=google.com; s=arc-20160816; b=vWP1ImW5LQg4Z3KMtsU2gBcOln0RqWVGt2LJB/bE47F6Phoi4E/tocS4HLPQdW0g8r e6b/g0SmlL08XYuExPoDbF30FKPPcJYsudrOke3Rx2XiVehciZWNJyGsQPvcnWFAs6R4 zDxuRxQhOplJcSWf5VC/kbsZwY4fch1If11LzdrVON8vN6bMEbOC7niXe2/L43o0gXS+ XLgRXc+qZkn0zeqZM5xGZlkMzAdbvW/MX8NFBiD8qcGd32/t6K7HXkFhR0Mr0lNawuam bJXa8GtjTxhZejoNxZoaWYeo+DvPVQWk9nSsExQcZH62ussLbAc/5D9qyFAD0Rv8wl4a /wYQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:references:in-reply-to:message-id:date :subject:cc:to:from:arc-authentication-results; bh=J3uYXoDRxuYCW15DWX4jnc7QNUmbyayhGaX7REUVDxc=; b=pu0410J4K+Z/te9Kq+5top1FT/TsU7ZOjmK7tqqk101mzH++ccxIJRIaghbK4ot7yL 36POofVOdLhbnVl8RdxLvwdP2R4PCo2TnHZ5gTbzYPqPR5um0VmSLbV9fTt6KCuyhtvC dz0+y9ZIwzY2Q16ZZnYNQ1J7jtVDfB2od5nzUUIpLSiH/DvEahrrITkZgWsyDF/7m7G8 wF18uOWLpBpdNTm44VCuIeJm/55Q5BXftmVxgKXTc7c6qWBbrcCA9SRDSU+Uw8fQAfas vCI9T3VAnzDwoOohHB87qAkLAaQX++zo/hV1B63wUPMm7o8JQXmCNZkfoolRHx015DgG s8Pg== 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 m8si27923817pfi.343.2017.11.28.10.42.23; Tue, 28 Nov 2017 10:42:23 -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 S1754073AbdK1SmV (ORCPT + 28 others); Tue, 28 Nov 2017 13:42:21 -0500 Received: from foss.arm.com ([217.140.101.70]:58318 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751676AbdK1SmS (ORCPT ); Tue, 28 Nov 2017 13:42:18 -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 D95EB1596; Tue, 28 Nov 2017 10:42:17 -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 AB19E3F578; Tue, 28 Nov 2017 10:42:17 -0800 (PST) Received: by edgewater-inn.cambridge.arm.com (Postfix, from userid 1000) id 293D71AE50A0; Tue, 28 Nov 2017 18:42:20 +0000 (GMT) From: Will Deacon To: linux-kernel@vger.kernel.org Cc: sebott@linux.vnet.ibm.com, schwidefsky@de.ibm.com, heiko.carstens@de.ibm.com, peterz@infradead.org, mingo@kernel.org, Will Deacon Subject: [PATCH 1/2] locking/core: Fix deadlock during boot on systems with GENERIC_LOCKBREAK Date: Tue, 28 Nov 2017 18:42:18 +0000 Message-Id: <1511894539-7988-2-git-send-email-will.deacon@arm.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1511894539-7988-1-git-send-email-will.deacon@arm.com> References: <1511894539-7988-1-git-send-email-will.deacon@arm.com> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Commit a8a217c22116 ("locking/core: Remove {read,spin,write}_can_lock()") removed the definition of raw_spin_can_lock, causing the GENERIC_LOCKBREAK spin_lock routines to poll the break_lock field when waiting on a lock. This has been reported to cause a deadlock during boot on s390, because the break_lock field is also set by the waiters, and can potentially remain set indefinitely if no other CPUs come in to take the lock after it has been released. This patch removes the explicit spinning on break_lock from the waiters, instead relying on the outer trylock operation to determine when the lock is available. Cc: Martin Schwidefsky Cc: Heiko Carstens Cc: Peter Zijlstra Cc: Ingo Molnar Fixes: a8a217c22116 ("locking/core: Remove {read,spin,write}_can_lock()") Reported-by: Sebastian Ott Tested-by: Sebastian Ott Signed-off-by: Will Deacon --- kernel/locking/spinlock.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) -- 2.1.4 diff --git a/kernel/locking/spinlock.c b/kernel/locking/spinlock.c index 1fd1a7543cdd..0ebb253e2199 100644 --- a/kernel/locking/spinlock.c +++ b/kernel/locking/spinlock.c @@ -68,8 +68,8 @@ void __lockfunc __raw_##op##_lock(locktype##_t *lock) \ \ if (!(lock)->break_lock) \ (lock)->break_lock = 1; \ - while ((lock)->break_lock) \ - arch_##op##_relax(&lock->raw_lock); \ + \ + arch_##op##_relax(&lock->raw_lock); \ } \ (lock)->break_lock = 0; \ } \ @@ -88,8 +88,8 @@ unsigned long __lockfunc __raw_##op##_lock_irqsave(locktype##_t *lock) \ \ if (!(lock)->break_lock) \ (lock)->break_lock = 1; \ - while ((lock)->break_lock) \ - arch_##op##_relax(&lock->raw_lock); \ + \ + arch_##op##_relax(&lock->raw_lock); \ } \ (lock)->break_lock = 0; \ return flags; \ From patchwork Tue Nov 28 18:42:19 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Will Deacon X-Patchwork-Id: 119899 Delivered-To: patch@linaro.org Received: by 10.140.22.227 with SMTP id 90csp2073023qgn; Tue, 28 Nov 2017 10:42:47 -0800 (PST) X-Google-Smtp-Source: AGs4zMbWHIvs98z1NQBpXyJwVJBiFtBKOpgxdglLbiWZdUcs4hs3xeL3nBIQr8M51EfQQwQds4V/ X-Received: by 10.84.229.5 with SMTP id b5mr77996plk.405.1511894567653; Tue, 28 Nov 2017 10:42:47 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1511894567; cv=none; d=google.com; s=arc-20160816; b=ZB44N6XiYS3NJ4Ue8JNPflKh9KvGgWt2+10Jb5wA3rar3MPl1T+DbrldHcsIJsGiws 5mOENFSuoAXVcRq3JD/tY35oqW664l34zIAkreWZbx3D6J9wg6Ke+9Ox1sUg0GacPAps 5WkOkwssjxgFp9YoH4ChCRvucYFVCVLhf8BxlPdALrPrMdxXclWEHLq11zx9Cb0L0Kmo BDmn5BUAbNaCgqRoB99pLTNDB1+bLpGq7Gx0In+j4b2rQZpeh7DY+lfjaMRcJehgWC3U g52QyBL7Oqy2XmHed+Ohp1mI70E2w/XlOQvJcMk8JtKZzNZezX5DILYH1MH6w/dt32HD zYLw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:references:in-reply-to:message-id:date :subject:cc:to:from:arc-authentication-results; bh=JaXjhR7JMHOs16yYERfAWTOG0Hr0rGz/NCxRj2Kaq6A=; b=PriIAAD35PEEcLzKkuM/lX7Joxn8pGnkpDufkmphhnWhkyagTa2DKfx7ubUt8klrw5 8VX0t35l+A1z+9UobrlGRLKXnZn8Yxlu5rftf460iOFWyb2efCYB+f7b9YeqihVmLyeQ 4keDzFDVRRQehzji4d/o/cw9jSDjLL7w/7JQO8TXL0XjVdfiUWmaJN2Mabxv9RY5OyNu R7ivYWsPTrnFgYc46p7EBlN8uaYjWougKyAp+WZGWQ7d8U31bw49k6kKMbySK45NhOg3 9dFFQUjRM7H+lbWfrl39AstnJADCxYq84gSoQUQ2US6mvNNJ6vG2diqJfLYcQ50xHVeG NBsA== 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 s64si27578143pfj.279.2017.11.28.10.42.47; Tue, 28 Nov 2017 10:42:47 -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 S932070AbdK1Smp (ORCPT + 28 others); Tue, 28 Nov 2017 13:42:45 -0500 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:58334 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752236AbdK1SmS (ORCPT ); Tue, 28 Nov 2017 13:42:18 -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 E44F515BF; Tue, 28 Nov 2017 10:42:17 -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 B5FF93F706; Tue, 28 Nov 2017 10:42:17 -0800 (PST) Received: by edgewater-inn.cambridge.arm.com (Postfix, from userid 1000) id 3BAF11AE50A4; Tue, 28 Nov 2017 18:42:20 +0000 (GMT) From: Will Deacon To: linux-kernel@vger.kernel.org Cc: sebott@linux.vnet.ibm.com, schwidefsky@de.ibm.com, heiko.carstens@de.ibm.com, peterz@infradead.org, mingo@kernel.org, Will Deacon Subject: [PATCH 2/2] locking/core: Remove break_lock field when CONFIG_GENERIC_LOCKBREAK=y Date: Tue, 28 Nov 2017 18:42:19 +0000 Message-Id: <1511894539-7988-3-git-send-email-will.deacon@arm.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1511894539-7988-1-git-send-email-will.deacon@arm.com> References: <1511894539-7988-1-git-send-email-will.deacon@arm.com> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org When CONFIG_GENERIC_LOCKBEAK=y, locking structures grow an extra int break_lock field which is used to implement raw_spin_is_contended() by setting the field to 1 when waiting on a lock and clearing it to zero when holding a lock. However, there are a few problems with this approach: - There is a write-write race between a CPU successfully taking the lock (and subsequently writing break_lock = 0) and a waiter waiting on the lock (and subsequently writing break_lock = 1). This could result in a contended lock being reported as uncontended and vice-versa. - On machines with store buffers, nothing guarantees that the writes to break_lock are visible to other CPUs at any particular time. - READ_ONCE/WRITE_ONCE are not used, so the field is potentially susceptible to harmful compiler optimisations, Consequently, the usefulness of this field is unclear and we'd be better off removing it and allowing architectures to implement raw_spin_is_contended() by providing a definition of arch_spin_is_contended(), as they can when CONFIG_GENERIC_LOCKBREAK=n. Cc: Martin Schwidefsky Cc: Heiko Carstens Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Sebastian Ott Signed-off-by: Will Deacon --- include/linux/rwlock_types.h | 3 --- include/linux/spinlock.h | 5 ----- include/linux/spinlock_types.h | 3 --- kernel/locking/spinlock.c | 9 +-------- 4 files changed, 1 insertion(+), 19 deletions(-) -- 2.1.4 diff --git a/include/linux/rwlock_types.h b/include/linux/rwlock_types.h index cc0072e93e36..857a72ceb794 100644 --- a/include/linux/rwlock_types.h +++ b/include/linux/rwlock_types.h @@ -10,9 +10,6 @@ */ typedef struct { arch_rwlock_t raw_lock; -#ifdef CONFIG_GENERIC_LOCKBREAK - unsigned int break_lock; -#endif #ifdef CONFIG_DEBUG_SPINLOCK unsigned int magic, owner_cpu; void *owner; diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h index a39186194cd6..3bf273538840 100644 --- a/include/linux/spinlock.h +++ b/include/linux/spinlock.h @@ -107,16 +107,11 @@ do { \ #define raw_spin_is_locked(lock) arch_spin_is_locked(&(lock)->raw_lock) -#ifdef CONFIG_GENERIC_LOCKBREAK -#define raw_spin_is_contended(lock) ((lock)->break_lock) -#else - #ifdef arch_spin_is_contended #define raw_spin_is_contended(lock) arch_spin_is_contended(&(lock)->raw_lock) #else #define raw_spin_is_contended(lock) (((void)(lock), 0)) #endif /*arch_spin_is_contended*/ -#endif /* * This barrier must provide two things: diff --git a/include/linux/spinlock_types.h b/include/linux/spinlock_types.h index 73548eb13a5d..24b4e6f2c1a2 100644 --- a/include/linux/spinlock_types.h +++ b/include/linux/spinlock_types.h @@ -19,9 +19,6 @@ typedef struct raw_spinlock { arch_spinlock_t raw_lock; -#ifdef CONFIG_GENERIC_LOCKBREAK - unsigned int break_lock; -#endif #ifdef CONFIG_DEBUG_SPINLOCK unsigned int magic, owner_cpu; void *owner; diff --git a/kernel/locking/spinlock.c b/kernel/locking/spinlock.c index 0ebb253e2199..936f3d14dd6b 100644 --- a/kernel/locking/spinlock.c +++ b/kernel/locking/spinlock.c @@ -66,12 +66,8 @@ void __lockfunc __raw_##op##_lock(locktype##_t *lock) \ break; \ preempt_enable(); \ \ - if (!(lock)->break_lock) \ - (lock)->break_lock = 1; \ - \ arch_##op##_relax(&lock->raw_lock); \ } \ - (lock)->break_lock = 0; \ } \ \ unsigned long __lockfunc __raw_##op##_lock_irqsave(locktype##_t *lock) \ @@ -86,12 +82,9 @@ unsigned long __lockfunc __raw_##op##_lock_irqsave(locktype##_t *lock) \ local_irq_restore(flags); \ preempt_enable(); \ \ - if (!(lock)->break_lock) \ - (lock)->break_lock = 1; \ - \ arch_##op##_relax(&lock->raw_lock); \ } \ - (lock)->break_lock = 0; \ + \ return flags; \ } \ \