Hi Paul,
On 03/03/15 17:42, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@xxxxxxxxxxxxxxxxxx>
>
> This commit removes the open-coded CPU-offline notification with new
> common code. This change avoids calling scheduler code using RCU from
> an offline CPU that RCU is ignoring. This commit is compatible with
> the existing code in not checking for timeout during a prior offline
> for a given CPU.
>
> Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> Cc: James Hogan <james.hogan@xxxxxxxxxx>
> Cc: <linux-metag@xxxxxxxxxxxxxxx>
I gave this a try via linux-next, but unfortunately it causes the
following warning every time a CPU goes down:
META213-Thread0 DSP [LogF] CPU1: unable to kill
If I add printks, I see that the state on entry to both cpu_wait_death
and cpu_report_death is already CPU_POST_DEAD, suggesting that it hasn't
changed from its initial value.
Should arches other than x86 now be calling cpu_set_state_online()? The
patchlet below seems to resolve it for Meta (not sure if that is the
best place in the startup sequence to do it, perhaps it doesn't matter).
diff --git a/arch/metag/kernel/smp.c b/arch/metag/kernel/smp.c
index ac3a199e33e7..430e379ec71f 100644
--- a/arch/metag/kernel/smp.c
+++ b/arch/metag/kernel/smp.c
@@ -383,6 +383,7 @@ asmlinkage void secondary_start_kernel(void)
* OK, now it's safe to let the boot CPU continue
*/
set_cpu_online(cpu, true);
+ cpu_set_state_online(cpu);
complete(&cpu_running);
/*
Looking at the comment before cpu_set_state_online:
> /*
> * Mark the specified CPU online.
> *
> * Note that it is permissible to omit this call entirely, as is
> * done in architectures that do no CPU-hotplug error checking.
> */
Which suggests it wasn't wrong to omit it before your patches came
along.
Cheers
James
> ---
> arch/metag/kernel/smp.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/metag/kernel/smp.c b/arch/metag/kernel/smp.c
> index f006d2276f40..ac3a199e33e7 100644
> --- a/arch/metag/kernel/smp.c
> +++ b/arch/metag/kernel/smp.c
> @@ -261,7 +261,6 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
> }
>
> #ifdef CONFIG_HOTPLUG_CPU
> -static DECLARE_COMPLETION(cpu_killed);
>
> /*
> * __cpu_disable runs on the processor to be shutdown.
> @@ -299,7 +298,7 @@ int __cpu_disable(void)
> */
> void __cpu_die(unsigned int cpu)
> {
> - if (!wait_for_completion_timeout(&cpu_killed, msecs_to_jiffies(1)))
> + if (!cpu_wait_death(cpu, 1))
> pr_err("CPU%u: unable to kill\n", cpu);
> }
>
> @@ -314,7 +313,7 @@ void cpu_die(void)
> local_irq_disable();
> idle_task_exit();
>
> - complete(&cpu_killed);
> + (void)cpu_report_death();
>
> asm ("XOR TXENABLE, D0Re0,D0Re0\n");
> }
>
Attachment:
signature.asc
Description: OpenPGP digital signature
