Messages in this thread Patch in this message | | | From | Jonas Aaberg <> | Subject | RFC [PATCH] SMP: Don't schedule tasks on inactive cpu(s) | Date | Wed, 29 Feb 2012 11:42:30 +0100 |
| |
This patch removes the ability to schedule tasks on cpus that are online, but not active. The reason for this patch is that during cpu hotplug on ARM (atleast) there is a short window where cpuX (X > 0) is online, but busy-waiting on cpu0 to put it active, meanwhile cpu0 can be interrupted and try to schedule something on the cpu that is busy checking its active bit.
I am aware that this patch is not how this issue (described more in detail below) shall be corrected, it is just an attempt to hide it and highlight the issue.
On a dual core ARM system (u8500):
CPU0 wakes up CPU1 from enable_nobnboot_cpus in suspend thread.
CPU1 is executing secondary_start_kernel, waiting for CPU1 to set the active bit before enabling interrupts.
CPU0: Suspend thread is waiting for CPU1 to go online in __cpu_up, but is preempted by a timer interrupt. Kondemand thread is woken up to handle do_dbs_timer, and decides to switch frequency. This involves informing other CPU since the localtimer on each core needs to be reprogrammed, so we execute twd_cpufreq_notifier. We get stuck in generic_exec_single, waiting (with preempt_off) for CPU1 to acknowledge execution of the function. But CPU1 is waiting for CPU0:Suspend, with interrupts off.
Stacks of relevant threads: PID: 0 TASK: df05ee40 CPU: 1 COMMAND: "swapper" #0 [<c0544e6c>] (secondary_start_kernel) from [<c00a1cf8>]
PID: 570 TASK: df1b8220 CPU: 0 COMMAND: "kondemand/0" #0 [<c00afc94>] (generic_exec_single) from [<c00afefc>] #1 [<c00afefc>] (smp_call_function_single) from [<c00b01b8>] #2 [<c00b01b8>] (smp_call_function) from [<c008717c>] #3 [<c008717c>] (on_each_cpu) from [<c003eec4>] #4 [<c003eec4>] (twd_cpufreq_notifier) from [<c00a1790>] #5 [<c00a1790>] (notifier_call_chain) from [<c00a18f0>] #6 [<c00a18f0>] (__srcu_notifier_call_chain) from [<c00a191c>] #7 [<c00a191c>] (srcu_notifier_call_chain) from [<c037880c>] #8 [<c037880c>] (cpufreq_notify_transition) from [<c004d6ac>] #9 [<c004d6ac>] (ux500_cpufreq_target) from [<c0377c58>]
PID: 19 TASK: df0b2e80 CPU: 0 COMMAND: "suspend" #0 [<c05487a0>] (schedule) from [<c0548a34>] #1 [<c0548a34>] (preempt_schedule_irq) from [<c00392ec>] #2 [<c00392ec>] (svc_preempt) from [<c020a6d0>] #3 [<c0047dd0>] (mtu_timer_delay_loop) from [<c020a6d0>] #4 [<c020a6d0>] (__delay) from [<c0544fd0>] #5 [<c0544fd0>] (__cpu_up) from [<c0545bd8>] #6 [<c0545bd8>] (_cpu_up) from [<c0534824>] #7 [<c0534824>] (enable_nonboot_cpus) from [<c00b6e4c>] #8 [<c00b6e4c>] (suspend_devices_and_enter) from [<c00b6fe4>] #9 [<c00b6fe4>] (enter_state) from [<c00b82bc>]
Signed-off-by: Jonas Aaberg <jonas.aberg@stericsson.com> --- kernel/smp.c | 29 +++++++++++++++++++++-------- 1 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/kernel/smp.c b/kernel/smp.c index db197d6..3144ff5 100644 --- a/kernel/smp.c +++ b/kernel/smp.c @@ -175,7 +175,7 @@ void generic_smp_call_function_interrupt(void) /* * Shouldn't receive this interrupt on a cpu that is not yet online. */ - WARN_ON_ONCE(!cpu_online(cpu)); + WARN_ON_ONCE(!cpu_online(cpu) || !cpu_active(cpu)); /* * Ensure entry is visible on call_function_queue after we have @@ -255,7 +255,8 @@ void generic_smp_call_function_single_interrupt(void) /* * Shouldn't receive this interrupt on a cpu that is not yet online. */ - WARN_ON_ONCE(!cpu_online(smp_processor_id())); + WARN_ON_ONCE(!cpu_online(smp_processor_id()) || + !cpu_active(smp_processor_id())); raw_spin_lock(&q->lock); list_replace_init(&q->list, &list); @@ -324,7 +325,8 @@ int smp_call_function_single(int cpu, smp_call_func_t func, void *info, func(info); local_irq_restore(flags); } else { - if ((unsigned)cpu < nr_cpu_ids && cpu_online(cpu)) { + if ((unsigned)cpu < nr_cpu_ids && cpu_online(cpu) && + cpu_active(cpu)) { struct call_single_data *data = &d; if (!wait) @@ -336,7 +338,7 @@ int smp_call_function_single(int cpu, smp_call_func_t func, void *info, data->info = info; generic_exec_single(cpu, data, wait); } else { - err = -ENXIO; /* CPU not online */ + err = -ENXIO; /* CPU not online or active */ } } @@ -367,6 +369,7 @@ int smp_call_function_any(const struct cpumask *mask, { unsigned int cpu; const struct cpumask *nodemask; + struct cpumask online_and_active; int ret; /* Try for same CPU (cheapest) */ @@ -378,12 +381,18 @@ int smp_call_function_any(const struct cpumask *mask, nodemask = cpumask_of_node(cpu_to_node(cpu)); for (cpu = cpumask_first_and(nodemask, mask); cpu < nr_cpu_ids; cpu = cpumask_next_and(cpu, nodemask, mask)) { - if (cpu_online(cpu)) + if (cpu_online(cpu) && cpu_active(cpu)) goto call; } - /* Any online will do: smp_call_function_single handles nr_cpu_ids. */ - cpu = cpumask_any_and(mask, cpu_online_mask); + /* + * Any online and active will do: smp_call_function_single handles + * nr_cpu_ids. + */ + + cpumask_and(&online_and_active, cpu_online_mask, cpu_active_mask); + + cpu = cpumask_any_and(mask, &online_and_active); call: ret = smp_call_function_single(cpu, func, info, wait); put_cpu(); @@ -430,7 +439,7 @@ void __smp_call_function_single(int cpu, struct call_single_data *data, /** * smp_call_function_many(): Run a function on a set of other CPUs. - * @mask: The set of cpus to run on (only runs on online subset). + * @mask: The set of cpus to run on (only runs on online and active subset). * @func: The function to run. This must be fast and non-blocking. * @info: An arbitrary pointer to pass to the function. * @wait: If true, wait (atomically) until function has completed @@ -515,6 +524,10 @@ void smp_call_function_many(const struct cpumask *mask, /* We rely on the "and" being processed before the store */ cpumask_and(data->cpumask, mask, cpu_online_mask); + + /* Remove any non-active cpus */ + cpumask_and(data->cpumask, data->cpumask, cpu_active_mask); + cpumask_clear_cpu(this_cpu, data->cpumask); refs = cpumask_weight(data->cpumask); -- 1.7.9
| |