linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH] ARM: smp: Fix the CPU hotplug race with scheduler.
Date: Mon, 20 Jun 2011 11:14:38 +0100	[thread overview]
Message-ID: <20110620101438.GD2082@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20110620095053.GA2082@n2100.arm.linux.org.uk>

On Mon, Jun 20, 2011 at 10:50:53AM +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 20, 2011 at 02:53:59PM +0530, Santosh Shilimkar wrote:
> > The current ARM CPU hotplug code suffers from couple of race conditions
> > in CPU online path with scheduler.
> > The ARM CPU hotplug code doesn't wait for hot-plugged CPU to be marked
> > active as part of cpu_notify() by the CPU which brought it up before
> > enabling interrupts.
> 
> Hmm, why not just move the set_cpu_online() call before notify_cpu_starting()
> and add the wait after the set_cpu_online() ?

Actually, the race is caused by the CPU being marked online (and therefore
available for the scheduler) but not yet active (the CPU asking this one
to boot hasn't run the online notifiers yet.)

This, I feel, is a fault of generic code.  If the CPU is not ready to have
processes scheduled on it (because migration is not initialized) then we
shouldn't be scheduling processes on the new CPU yet.

In any case, this should close the window by ensuring that we don't receive
an interrupt in the online-but-not-active case.  Can you please test?

 arch/arm/kernel/smp.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 344e52b..e34d750 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -318,9 +318,15 @@ asmlinkage void __cpuinit secondary_start_kernel(void)
 	smp_store_cpu_info(cpu);
 
 	/*
-	 * OK, now it's safe to let the boot CPU continue
+	 * OK, now it's safe to let the boot CPU continue.  Wait for
+	 * the CPU migration code to notice that the CPU is online
+	 * before we continue.
 	 */
+	local_irq_disable();
 	set_cpu_online(cpu, true);
+	while (!cpumask_test_cpu(cpu, cpu_active_mask))
+		cpu_relax();
+	local_irq_enable();
 
 	/*
 	 * OK, it's off to the idle thread for us

  reply	other threads:[~2011-06-20 10:14 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-20  9:23 [RFC PATCH] ARM: smp: Fix the CPU hotplug race with scheduler Santosh Shilimkar
2011-06-20  9:50 ` Russell King - ARM Linux
2011-06-20 10:14   ` Russell King - ARM Linux [this message]
2011-06-20 10:28     ` Santosh Shilimkar
2011-06-20 10:35       ` Russell King - ARM Linux
2011-06-20 10:45         ` Santosh Shilimkar
2011-06-20 11:42           ` Santosh Shilimkar
2011-06-20 10:44       ` Russell King - ARM Linux
2011-06-20 10:47         ` Santosh Shilimkar
2011-06-20 11:13           ` Russell King - ARM Linux
2011-06-20 11:25             ` Santosh Shilimkar
2011-06-20 11:40               ` Russell King - ARM Linux
2011-06-20 11:51                 ` Santosh Shilimkar
2011-06-20 12:19                   ` Russell King - ARM Linux
2011-06-20 12:27                     ` Santosh Shilimkar
2011-06-20 12:57                       ` Russell King - ARM Linux
2011-06-20 14:23                 ` Russell King - ARM Linux
2011-06-20 14:54                   ` Santosh Shilimkar
2011-06-20 15:01                     ` Russell King - ARM Linux
2011-06-20 15:10                       ` Santosh Shilimkar
2011-06-21  9:08                     ` Santosh Shilimkar
2011-06-21 10:00                       ` Russell King - ARM Linux
2011-06-21 10:17                         ` Santosh Shilimkar
2011-06-21 10:19                           ` Russell King - ARM Linux
2011-06-21 10:21                             ` Santosh Shilimkar
2011-06-21 10:26                               ` Russell King - ARM Linux
2011-06-21 20:16                                 ` Stephen Boyd
2011-06-21 23:10                                   ` Russell King - ARM Linux
2011-06-22  0:06                                     ` Stephen Boyd
2011-06-22 10:06                                       ` Russell King - ARM Linux
2011-06-20 10:19   ` Santosh Shilimkar

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=20110620101438.GD2082@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).