| Message ID | mvmbm67etfg.fsf@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: don't stop itself in smp_send_stop | expand |
On 11/29/18 8:32 AM, Andreas Schwab wrote: > Only run ipi_stop on other cpus, not itself. Don't wait for ipi_stop to > return, since it never does. Also, mark each cpu offline. > > Signed-off-by: Andreas Schwab <schwab@suse.de> > --- > arch/riscv/kernel/smp.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c > index 57b1383e5e..5c6bb5fe5a 100644 > --- a/arch/riscv/kernel/smp.c > +++ b/arch/riscv/kernel/smp.c > @@ -23,6 +23,7 @@ > #include <linux/smp.h> > #include <linux/sched.h> > #include <linux/seq_file.h> > +#include <linux/delay.h> > > #include <asm/sbi.h> > #include <asm/tlbflush.h> > @@ -148,13 +149,27 @@ void arch_send_call_function_single_ipi(int cpu) > > static void ipi_stop(void *unused) > { > + set_cpu_online (smp_processor_id(), false); > while (1) > wait_for_interrupt(); > } > > void smp_send_stop(void) > { > - on_each_cpu(ipi_stop, NULL, 1); > + unsigned long timeout; > + struct cpumask mask; > + No need to do mask copy this, if num_online_cpus() == 1. > + cpumask_copy (&mask, cpu_online_mask); > + cpumask_clear_cpu (smp_processor_id (), &mask); > + on_each_cpu_mask(&mask, ipi_stop, NULL, 0); The function header of on_each_cpu_mask says func should be non-blocking. Not sure if that's a hard requirement or not. If it is, we can introduce an extra IPI_STOP in ipi_names and just send ipi. The receiving CPU can handle the IPI_STOP. > + > + /* Wait up to one second for other CPUs to stop */ > + timeout = USEC_PER_SEC; > + while (num_online_cpus() > 1 && timeout--) > + udelay(1); > + > + if (num_online_cpus() > 1) > + pr_warn("SMP: failed to stop secondary CPUs\n"); May be print the remaining online cpus so that we know which one failed ? Regards, Atish > } > > void smp_send_reschedule(int cpu) >
On Nov 29 2018, Atish Patra <atish.patra@wdc.com> wrote: >> + cpumask_copy (&mask, cpu_online_mask); >> + cpumask_clear_cpu (smp_processor_id (), &mask); >> + on_each_cpu_mask(&mask, ipi_stop, NULL, 0); > > The function header of on_each_cpu_mask says func should be > non-blocking. Not sure if that's a hard requirement or not. What does that mean? >> + >> + /* Wait up to one second for other CPUs to stop */ >> + timeout = USEC_PER_SEC; >> + while (num_online_cpus() > 1 && timeout--) >> + udelay(1); >> + >> + if (num_online_cpus() > 1) >> + pr_warn("SMP: failed to stop secondary CPUs\n"); > > May be print the remaining online cpus so that we know which one failed ? I have copied that from arm. The only user of smp_send_stop is panic, which can cope with that, and the probability of that happening is very low anyway. Andreas.
On 12/10/18 2:58 AM, Andreas Schwab wrote: > On Nov 29 2018, Atish Patra <atish.patra@wdc.com> wrote: > >>> + cpumask_copy (&mask, cpu_online_mask); >>> + cpumask_clear_cpu (smp_processor_id (), &mask); >>> + on_each_cpu_mask(&mask, ipi_stop, NULL, 0); >> >> The function header of on_each_cpu_mask says func should be >> non-blocking. Not sure if that's a hard requirement or not. > > What does that mean? > https://elixir.bootlin.com/linux/v4.20-rc6/source/kernel/smp.c#L617 >>> + >>> + /* Wait up to one second for other CPUs to stop */ >>> + timeout = USEC_PER_SEC; >>> + while (num_online_cpus() > 1 && timeout--) >>> + udelay(1); >>> + >>> + if (num_online_cpus() > 1) >>> + pr_warn("SMP: failed to stop secondary CPUs\n"); >> >> May be print the remaining online cpus so that we know which one failed ? > > I have copied that from arm. The only user of smp_send_stop is panic, > which can cope with that, and the probability of that happening is very > low anyway. > My suggestion was based on arm64 code. IMHO, it makes more sense to follow arm64 code than arm. https://elixir.bootlin.com/linux/v4.20-rc6/source/arch/arm64/kernel/smp.c#L955 Regards, Atish > Andreas. >
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c index 57b1383e5e..5c6bb5fe5a 100644 --- a/arch/riscv/kernel/smp.c +++ b/arch/riscv/kernel/smp.c @@ -23,6 +23,7 @@ #include <linux/smp.h> #include <linux/sched.h> #include <linux/seq_file.h> +#include <linux/delay.h> #include <asm/sbi.h> #include <asm/tlbflush.h> @@ -148,13 +149,27 @@ void arch_send_call_function_single_ipi(int cpu) static void ipi_stop(void *unused) { + set_cpu_online (smp_processor_id(), false); while (1) wait_for_interrupt(); } void smp_send_stop(void) { - on_each_cpu(ipi_stop, NULL, 1); + unsigned long timeout; + struct cpumask mask; + + cpumask_copy (&mask, cpu_online_mask); + cpumask_clear_cpu (smp_processor_id (), &mask); + on_each_cpu_mask(&mask, ipi_stop, NULL, 0); + + /* Wait up to one second for other CPUs to stop */ + timeout = USEC_PER_SEC; + while (num_online_cpus() > 1 && timeout--) + udelay(1); + + if (num_online_cpus() > 1) + pr_warn("SMP: failed to stop secondary CPUs\n"); } void smp_send_reschedule(int cpu)
Only run ipi_stop on other cpus, not itself. Don't wait for ipi_stop to return, since it never does. Also, mark each cpu offline. Signed-off-by: Andreas Schwab <schwab@suse.de> --- arch/riscv/kernel/smp.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)