Hi Paul, On 08/30/2018 04:20 PM, Paul Burton wrote:
Hi Dengcheng, On Wed, Jul 11, 2018 at 06:27:43PM -0700, Dengcheng Zhu wrote:diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c index 03f1026..4615702 100644 --- a/arch/mips/kernel/smp-cps.c +++ b/arch/mips/kernel/smp-cps.c @@ -426,12 +406,18 @@ static enum { CPU_DEATH_POWER, } cpu_death;-void play_dead(void)+void play_dead(bool kexec) { unsigned int cpu, core, vpe_id;local_irq_disable();- idle_task_exit(); + /* + * Don't bother dealing with idle task's mm as we are executing the + * new kernel. + */ + if (!kexec) + idle_task_exit(); + cpu = smp_processor_id(); core = cpu_core(&cpu_data[cpu]); cpu_death = CPU_DEATH_POWER; @@ -454,7 +440,8 @@ void play_dead(void) }/* This CPU has chosen its way out */- (void)cpu_report_death(); + if (!kexec) + (void)cpu_report_death();Is it a problem if we just call cpu_report_death() unconditionally? At a glance it looks like we'd just change cpu_hotplug_state for the CPU, but since it's going to either power down or hang anyway that seems fine. If we could do that, then the only other thing the added kexec argument is used for is preventing us from calling idle_task_exit(). We could instead move that to arch_cpu_idle_dead() and not need to add the extra argument to each implementation of play_dead(), which should make this patch a little cleaner.
Agreed. Thanks! Dengcheng --------------------------------------------------------------------------- *From:* Paul Burton <mailto:paul.burton@xxxxxxxx> *Sent:* Thursday, August 30, 2018 4:20PM *To:* Dengcheng Zhu <mailto:dzhu@xxxxxxxxxxxx>*Cc:* Pburton <mailto:pburton@xxxxxxxxxxxx>, Ralf <mailto:ralf@xxxxxxxxxxxxxx>, Linux-mips <mailto:linux-mips@xxxxxxxxxxxxxx>
*Subject:* Re: [PATCH v2 1/6] MIPS: Make play_dead() work for kexec Hi Dengcheng, On Wed, Jul 11, 2018 at 06:27:43PM -0700, Dengcheng Zhu wrote:
diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c index 03f1026..4615702 100644 --- a/arch/mips/kernel/smp-cps.c +++ b/arch/mips/kernel/smp-cps.c @@ -426,12 +406,18 @@ static enum { CPU_DEATH_POWER, } cpu_death;-void play_dead(void)+void play_dead(bool kexec) { unsigned int cpu, core, vpe_id;local_irq_disable();- idle_task_exit(); + /* + * Don't bother dealing with idle task's mm as we are executing the + * new kernel. + */ + if (!kexec) + idle_task_exit(); + cpu = smp_processor_id(); core = cpu_core(&cpu_data[cpu]); cpu_death = CPU_DEATH_POWER; @@ -454,7 +440,8 @@ void play_dead(void) }/* This CPU has chosen its way out */- (void)cpu_report_death(); + if (!kexec) + (void)cpu_report_death();
Is it a problem if we just call cpu_report_death() unconditionally? At a
glance it looks like we'd just change cpu_hotplug_state for the CPU, but
since it's going to either power down or hang anyway that seems fine.
If we could do that, then the only other thing the added kexec argument
is used for is preventing us from calling idle_task_exit(). We could
instead move that to arch_cpu_idle_dead() and not need to add the extra
argument to each implementation of play_dead(), which should make this
patch a little cleaner.
Thanks,
Paul
