diff mbox series

[12/12] parisc: Implement __cpu_die() and __cpu_disable() for CPU hotplugging

Message ID 20220325143833.402631-12-deller@gmx.de (mailing list archive)
State Superseded
Headers show
Series [01/12] parisc: Switch from GENERIC_CPU_DEVICES to GENERIC_ARCH_TOPOLOGY | expand

Commit Message

Helge Deller March 25, 2022, 2:38 p.m. UTC 
Add relevant code to __cpu_die() and __cpu_disable() to finally enable
the CPU hotplugging features. Reset the irq count values in smp_callin()
to zero before bringing up the CPU.

Use "chcpu -d 1" to bring CPU1 down, and "chcpu -e 1" to bring it up.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 arch/parisc/Kconfig           |  1 +
 arch/parisc/include/asm/smp.h |  9 +---
 arch/parisc/kernel/smp.c      | 80 +++++++++++++++++++++++++++++++++--
 3 files changed, 79 insertions(+), 11 deletions(-)

--
2.35.1

Comments

Rolf Eike Beer March 25, 2022, 5:10 p.m. UTC | #1 
Am Freitag, 25. März 2022, 15:38:33 CET schrieb Helge Deller:
> Add relevant code to __cpu_die() and __cpu_disable() to finally enable
> the CPU hotplugging features. Reset the irq count values in smp_callin()
> to zero before bringing up the CPU.
> 
> Use "chcpu -d 1" to bring CPU1 down, and "chcpu -e 1" to bring it up.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> ---
>  arch/parisc/Kconfig           |  1 +
>  arch/parisc/include/asm/smp.h |  9 +---
>  arch/parisc/kernel/smp.c      | 80 +++++++++++++++++++++++++++++++++--
>  3 files changed, 79 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/parisc/kernel/smp.c b/arch/parisc/kernel/smp.c
> index a32a882a2d58..60cc33fd345c 100644
> --- a/arch/parisc/kernel/smp.c
> +++ b/arch/parisc/kernel/smp.c
> @@ -430,10 +444,68 @@ void smp_cpus_done(unsigned int cpu_max)
> 
>  int __cpu_up(unsigned int cpu, struct task_struct *tidle)
>  {
> -	if (cpu != 0 && cpu < parisc_max_cpus && smp_boot_one_cpu(cpu, 
tidle))
> -		return -ENOSYS;
> +	if (cpu_online(cpu))
> +		return 0;
> +
> +	if (num_online_cpus() < parisc_max_cpus && smp_boot_one_cpu(cpu, 
tidle))
> +		return -EIO;

I had to look up parisc_max_cpus, and found this:

> static int parisc_max_cpus = 1;

Hm, signed?

> parisc_max_cpus = max_cpus;
>        if (!max_cpus)
>                printk(KERN_INFO "SMP mode deactivated.\n");

So parisc_max_cpus is now 0, which seems wrong. Shouldn't the check be before 
the assignment? This would have avoided the "cpu != 0" in the old code 
completely.

> +
> +	return cpu_online(cpu) ? 0 : -EIO;
> +}
> +
> +/*
> + * __cpu_disable runs on the processor to be shutdown.
> + */
> +int __cpu_disable(void)
> +{
> +#ifdef CONFIG_HOTPLUG_CPU
> +	unsigned int cpu = smp_processor_id();
> +
> +	remove_cpu_topology(cpu);
> +
> +	/*
> +	 * Take this CPU offline.  Once we clear this, we can't return,
> +	 * and we must not schedule until we're ready to give up the cpu.
> +	 */
> +	set_cpu_online(cpu, false);
> +
> +	/*
> +	 * disable IPI interrupt
> +	 */
> +	disable_percpu_irq(IPI_IRQ);
> +
> +	/*
> +	 * migrate IRQs away from this CPU
> +	 */
> +	irq_migrate_all_off_this_cpu();

While I really enjoy good code comments the last 2 seem a t bit wasteful, 
given that the code is basically exactly the same as the text.

> +	/*
> +	 * Flush user cache and TLB mappings, and then remove this CPU
> +	 * from the vm mask set of all processes.
> +	 *
> +	 * Caches are flushed to the Level of Unification Inner Shareable
> +	 * to write-back dirty lines to unified caches shared by all CPUs.
> +	 */
> +	flush_cache_all_local();
> +	flush_tlb_all_local(NULL);
> 
> -	return cpu_online(cpu) ? 0 : -ENOSYS;
> +	/* disable all irqs, including timer irq */
> +	local_irq_disable();
> +#endif
> +	return 0;
> +}
> +
> +/*
> + * called on the thread which is asking for a CPU to be shutdown -
> + * waits until shutdown has completed, or it is timed out.
> + */
> +void __cpu_die(unsigned int cpu)
> +{
> +	if (!cpu_wait_death(cpu, 5)) {
> +		pr_crit("CPU%u: cpu didn't die\n", cpu);
> +		return;
> +	}
> +	pr_debug("CPU%u: shutdown\n", cpu);
>  }
> 
>  #ifdef CONFIG_PROC_FS
> --
> 2.35.1
Helge Deller March 25, 2022, 6 p.m. UTC | #2 
On 3/25/22 18:10, Rolf Eike Beer wrote:
> Am Freitag, 25. März 2022, 15:38:33 CET schrieb Helge Deller:
>> Add relevant code to __cpu_die() and __cpu_disable() to finally enable
>> the CPU hotplugging features. Reset the irq count values in smp_callin()
>> to zero before bringing up the CPU.
>>
>> Use "chcpu -d 1" to bring CPU1 down, and "chcpu -e 1" to bring it up.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> ---
>>  arch/parisc/Kconfig           |  1 +
>>  arch/parisc/include/asm/smp.h |  9 +---
>>  arch/parisc/kernel/smp.c      | 80 +++++++++++++++++++++++++++++++++--
>>  3 files changed, 79 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/parisc/kernel/smp.c b/arch/parisc/kernel/smp.c
>> index a32a882a2d58..60cc33fd345c 100644
>> --- a/arch/parisc/kernel/smp.c
>> +++ b/arch/parisc/kernel/smp.c
>> @@ -430,10 +444,68 @@ void smp_cpus_done(unsigned int cpu_max)
>>
>>  int __cpu_up(unsigned int cpu, struct task_struct *tidle)
>>  {
>> -	if (cpu != 0 && cpu < parisc_max_cpus && smp_boot_one_cpu(cpu,
> tidle))
>> -		return -ENOSYS;
>> +	if (cpu_online(cpu))
>> +		return 0;
>> +
>> +	if (num_online_cpus() < parisc_max_cpus && smp_boot_one_cpu(cpu,
> tidle))
>> +		return -EIO;
>
> I had to look up parisc_max_cpus, and found this:
>
>> static int parisc_max_cpus = 1;
>
> Hm, signed?
>
>> parisc_max_cpus = max_cpus;
>>        if (!max_cpus)
>>                printk(KERN_INFO "SMP mode deactivated.\n");
>
> So parisc_max_cpus is now 0, which seems wrong. Shouldn't the check be before
> the assignment? This would have avoided the "cpu != 0" in the old code
> completely.

No, the old
	cpu != 0 && ....smp_boot_one_cpu(cpu, tidle))
was to avoid that smp_boot_one_cpu() gets called for the monarch cpu.

Anyway, you are right, the code could need cleanups.
But in my patch series I wanted to keep the changes minimal in the first place.

>
>> +
>> +	return cpu_online(cpu) ? 0 : -EIO;
>> +}
>> +
>> +/*
>> + * __cpu_disable runs on the processor to be shutdown.
>> + */
>> +int __cpu_disable(void)
>> +{
>> +#ifdef CONFIG_HOTPLUG_CPU
>> +	unsigned int cpu = smp_processor_id();
>> +
>> +	remove_cpu_topology(cpu);
>> +
>> +	/*
>> +	 * Take this CPU offline.  Once we clear this, we can't return,
>> +	 * and we must not schedule until we're ready to give up the cpu.
>> +	 */
>> +	set_cpu_online(cpu, false);
>> +
>> +	/*
>> +	 * disable IPI interrupt
>> +	 */
>> +	disable_percpu_irq(IPI_IRQ);
>> +
>> +	/*
>> +	 * migrate IRQs away from this CPU
>> +	 */
>> +	irq_migrate_all_off_this_cpu();
>
> While I really enjoy good code comments the last 2 seem a t bit wasteful,
> given that the code is basically exactly the same as the text.

Yep.

Helge
diff mbox series

Patch

diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 6bd42c82a019..ec5bb9626d06 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -56,6 +56,7 @@  config PARISC
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_REGS_AND_STACK_ACCESS_API
 	select GENERIC_SCHED_CLOCK
+	select GENERIC_IRQ_MIGRATION
 	select HAVE_UNSTABLE_SCHED_CLOCK if SMP
 	select LEGACY_TIMER_TICK
 	select CPU_NO_EFFICIENT_FFS
diff --git a/arch/parisc/include/asm/smp.h b/arch/parisc/include/asm/smp.h
index 2279ebe5e2da..94d1f21ce99a 100644
--- a/arch/parisc/include/asm/smp.h
+++ b/arch/parisc/include/asm/smp.h
@@ -44,12 +44,7 @@  static inline void smp_send_all_nop(void) { return; }

 #define NO_PROC_ID		0xFF		/* No processor magic marker */
 #define ANY_PROC_ID		0xFF		/* Any processor magic marker */
-static inline int __cpu_disable (void) {
-  return 0;
-}
-static inline void __cpu_die (unsigned int cpu) {
-  while(1)
-    ;
-}
+int __cpu_disable(void);
+void __cpu_die(unsigned int cpu);

 #endif /*  __ASM_SMP_H */
diff --git a/arch/parisc/kernel/smp.c b/arch/parisc/kernel/smp.c
index a32a882a2d58..60cc33fd345c 100644
--- a/arch/parisc/kernel/smp.c
+++ b/arch/parisc/kernel/smp.c
@@ -30,6 +30,7 @@ 
 #include <linux/ftrace.h>
 #include <linux/cpu.h>
 #include <linux/kgdb.h>
+#include <linux/sched/hotplug.h>

 #include <linux/atomic.h>
 #include <asm/current.h>
@@ -309,7 +310,7 @@  smp_cpu_init(int cpunum)
  * Slaves start using C here. Indirectly called from smp_slave_stext.
  * Do what start_kernel() and main() do for boot strap processor (aka monarch)
  */
-void __init smp_callin(unsigned long pdce_proc)
+void __cpuinit smp_callin(unsigned long pdce_proc)
 {
 	int slave_id = cpu_now_booting;

@@ -339,6 +340,19 @@  int smp_boot_one_cpu(int cpuid, struct task_struct *idle)
 	const struct cpuinfo_parisc *p = &per_cpu(cpu_data, cpuid);
 	long timeout;

+#ifdef CONFIG_HOTPLUG_CPU
+	int i;
+
+	/* reset irq statistics for this CPU */
+	memset(&per_cpu(irq_stat, cpuid), 0, sizeof(irq_cpustat_t));
+	for (i = 0; i < NR_IRQS; i++) {
+		struct irq_desc *desc = irq_to_desc(i);
+
+		if (desc && desc->kstat_irqs)
+			*per_cpu_ptr(desc->kstat_irqs, cpuid) = 0;
+	}
+#endif
+
 	/* Let _start know what logical CPU we're booting
 	** (offset into init_tasks[],cpu_data[])
 	*/
@@ -430,10 +444,68 @@  void smp_cpus_done(unsigned int cpu_max)

 int __cpu_up(unsigned int cpu, struct task_struct *tidle)
 {
-	if (cpu != 0 && cpu < parisc_max_cpus && smp_boot_one_cpu(cpu, tidle))
-		return -ENOSYS;
+	if (cpu_online(cpu))
+		return 0;
+
+	if (num_online_cpus() < parisc_max_cpus && smp_boot_one_cpu(cpu, tidle))
+		return -EIO;
+
+	return cpu_online(cpu) ? 0 : -EIO;
+}
+
+/*
+ * __cpu_disable runs on the processor to be shutdown.
+ */
+int __cpu_disable(void)
+{
+#ifdef CONFIG_HOTPLUG_CPU
+	unsigned int cpu = smp_processor_id();
+
+	remove_cpu_topology(cpu);
+
+	/*
+	 * Take this CPU offline.  Once we clear this, we can't return,
+	 * and we must not schedule until we're ready to give up the cpu.
+	 */
+	set_cpu_online(cpu, false);
+
+	/*
+	 * disable IPI interrupt
+	 */
+	disable_percpu_irq(IPI_IRQ);
+
+	/*
+	 * migrate IRQs away from this CPU
+	 */
+	irq_migrate_all_off_this_cpu();
+
+	/*
+	 * Flush user cache and TLB mappings, and then remove this CPU
+	 * from the vm mask set of all processes.
+	 *
+	 * Caches are flushed to the Level of Unification Inner Shareable
+	 * to write-back dirty lines to unified caches shared by all CPUs.
+	 */
+	flush_cache_all_local();
+	flush_tlb_all_local(NULL);

-	return cpu_online(cpu) ? 0 : -ENOSYS;
+	/* disable all irqs, including timer irq */
+	local_irq_disable();
+#endif
+	return 0;
+}
+
+/*
+ * called on the thread which is asking for a CPU to be shutdown -
+ * waits until shutdown has completed, or it is timed out.
+ */
+void __cpu_die(unsigned int cpu)
+{
+	if (!cpu_wait_death(cpu, 5)) {
+		pr_crit("CPU%u: cpu didn't die\n", cpu);
+		return;
+	}
+	pr_debug("CPU%u: shutdown\n", cpu);
 }

 #ifdef CONFIG_PROC_FS