All of lore.kernel.org
 help / color / mirror / Atom feed
From: tip-bot for Matt Fleming <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: hock.leong.kweh@intel.com, brgerst@gmail.com,
	dvlasenk@redhat.com, peterz@infradead.org, bp@alien8.de,
	hpa@zytor.com, luto@amacapital.net,
	torvalds@linux-foundation.org, tglx@linutronix.de,
	pure.logic@nexus-software.ie, linux-kernel@vger.kernel.org,
	jlee@suse.com, mingo@kernel.org, ard.biesheuvel@linaro.org,
	matt@codeblueprint.co.uk
Subject: [tip:efi/core] efi/capsule: Make efi_capsule_pending() lockless
Date: Fri, 6 May 2016 23:33:40 -0700	[thread overview]
Message-ID: <tip-62075e581802ea1842d5d3c490a7e46330bdb9e1@git.kernel.org> (raw)
In-Reply-To: <1462570771-13324-2-git-send-email-matt@codeblueprint.co.uk>

Commit-ID:  62075e581802ea1842d5d3c490a7e46330bdb9e1
Gitweb:     http://git.kernel.org/tip/62075e581802ea1842d5d3c490a7e46330bdb9e1
Author:     Matt Fleming <matt@codeblueprint.co.uk>
AuthorDate: Fri, 6 May 2016 22:39:27 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 7 May 2016 07:06:13 +0200

efi/capsule: Make efi_capsule_pending() lockless

Taking a mutex in the reboot path is bogus because we cannot sleep
with interrupts disabled, such as when rebooting due to panic(),

  BUG: sleeping function called from invalid context at kernel/locking/mutex.c:97
  in_atomic(): 0, irqs_disabled(): 1, pid: 7, name: rcu_sched
  Call Trace:
    dump_stack+0x63/0x89
    ___might_sleep+0xd8/0x120
    __might_sleep+0x49/0x80
    mutex_lock+0x20/0x50
    efi_capsule_pending+0x1d/0x60
    native_machine_emergency_restart+0x59/0x280
    machine_emergency_restart+0x19/0x20
    emergency_restart+0x18/0x20
    panic+0x1ba/0x217

In this case all other CPUs will have been stopped by the time we
execute the platform reboot code, so 'capsule_pending' cannot change
under our feet. We wouldn't care even if it could since we cannot wait
for it complete.

Also, instead of relying on the external 'system_state' variable just
use a reboot notifier, so we can set 'stop_capsules' while holding
'capsule_mutex', thereby avoiding a race where system_state is updated
while we're in the middle of efi_capsule_update_locked() (since CPUs
won't have been stopped at that point).

Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kweh Hock Leong <hock.leong.kweh@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: joeyli <jlee@suse.com>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1462570771-13324-2-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/capsule.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
index 0de5594..e530540 100644
--- a/drivers/firmware/efi/capsule.c
+++ b/drivers/firmware/efi/capsule.c
@@ -22,11 +22,12 @@ typedef struct {
 } efi_capsule_block_desc_t;
 
 static bool capsule_pending;
+static bool stop_capsules;
 static int efi_reset_type = -1;
 
 /*
  * capsule_mutex serialises access to both capsule_pending and
- * efi_reset_type.
+ * efi_reset_type and stop_capsules.
  */
 static DEFINE_MUTEX(capsule_mutex);
 
@@ -50,18 +51,13 @@ static DEFINE_MUTEX(capsule_mutex);
  */
 bool efi_capsule_pending(int *reset_type)
 {
-	bool rv = false;
-
-	mutex_lock(&capsule_mutex);
 	if (!capsule_pending)
-		goto out;
+		return false;
 
 	if (reset_type)
 		*reset_type = efi_reset_type;
-	rv = true;
-out:
-	mutex_unlock(&capsule_mutex);
-	return rv;
+
+	return true;
 }
 
 /*
@@ -176,7 +172,7 @@ efi_capsule_update_locked(efi_capsule_header_t *capsule,
 	 * whether to force an EFI reboot), and we're racing against
 	 * that call. Abort in that case.
 	 */
-	if (unlikely(system_state == SYSTEM_RESTART)) {
+	if (unlikely(stop_capsules)) {
 		pr_warn("Capsule update raced with reboot, aborting.\n");
 		return -EINVAL;
 	}
@@ -298,3 +294,22 @@ out:
 	return rv;
 }
 EXPORT_SYMBOL_GPL(efi_capsule_update);
+
+static int capsule_reboot_notify(struct notifier_block *nb, unsigned long event, void *cmd)
+{
+	mutex_lock(&capsule_mutex);
+	stop_capsules = true;
+	mutex_unlock(&capsule_mutex);
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block capsule_reboot_nb = {
+	.notifier_call = capsule_reboot_notify,
+};
+
+static int __init capsule_reboot_register(void)
+{
+	return register_reboot_notifier(&capsule_reboot_nb);
+}
+core_initcall(capsule_reboot_register);

  reply	other threads:[~2016-05-07  6:35 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-06 21:39 [GIT PULL 0/5] EFI changes for v4.7 Matt Fleming
2016-05-06 21:39 ` [PATCH 1/5] efi/capsule: Make efi_capsule_pending() lockless Matt Fleming
2016-05-07  6:33   ` tip-bot for Matt Fleming [this message]
2016-05-06 21:39 ` [PATCH 2/5] efibc: Fix excessive stack footprint warning Matt Fleming
2016-05-07  6:34   ` [tip:efi/core] " tip-bot for Jeremy Compostella
2016-05-09 23:41   ` [PATCH 2/5] " Elliott, Robert (Persistent Memory)
2016-05-10  8:40     ` Compostella, Jeremy
2016-05-10  8:40       ` Compostella, Jeremy
2016-05-11 12:43       ` Matt Fleming
2016-05-11 12:43         ` Matt Fleming
2016-05-11 15:16         ` Compostella, Jeremy
2016-05-11 15:16           ` Compostella, Jeremy
2016-05-14 19:20           ` Matt Fleming
2016-05-14 19:20             ` Matt Fleming
2016-05-06 21:39 ` [PATCH 3/5] efi/capsule: Move 'capsule' to the stack in efi_capsule_supported() Matt Fleming
2016-05-07  6:34   ` [tip:efi/core] " tip-bot for Matt Fleming
2016-05-06 21:39 ` [PATCH 4/5] efi: Merge boolean flag arguments Matt Fleming
2016-05-07  6:34   ` [tip:efi/core] " tip-bot for Julia Lawall
2016-05-06 21:39 ` [PATCH 5/5] efivarfs: Make efivarfs_file_ioctl static Matt Fleming
2016-05-06 21:39   ` Matt Fleming
2016-05-07  6:35   ` [tip:efi/core] efivarfs: Make efivarfs_file_ioctl() static tip-bot for Peter Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tip-62075e581802ea1842d5d3c490a7e46330bdb9e1@git.kernel.org \
    --to=tipbot@zytor.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=dvlasenk@redhat.com \
    --cc=hock.leong.kweh@intel.com \
    --cc=hpa@zytor.com \
    --cc=jlee@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=matt@codeblueprint.co.uk \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=pure.logic@nexus-software.ie \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.