Hi, Denis

Any test result on pmac machine for this patch?=20

Zhuoyu

-----Original Message-----
From: Zhang Zhuoyu [mailto:zhangzhuoyu@cmss.chinamobile.com]
Sent: Wednesday, December 16, 2015 10:46 PM
To: 'Denis Kirjanov'; 'Michael Ellerman'
Cc: 'Daniel Axtens'; 'Zhang Zhuoyu'; 'benh@kernel.crashing.org';
'paulus@samba.org'; 'tglx@linutronix.de'; 'jiang.liu@linux.intel.com';
'linuxppc-dev@lists.ozlabs.org'; 'linux-kernel@vger.kernel.org'
Subject: RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
migrate_irqs()
=20
=20

quoted

-----Original Message-----
From: Denis Kirjanov [mailto:kda@linux-powerpc.org]
Sent: Wednesday, December 16, 2015 7:55 PM
To: Michael Ellerman
Cc: Daniel Axtens; Zhang Zhuoyu; benh@kernel.crashing.org;
paulus@samba.org; tglx@linutronix.de; jiang.liu@linux.intel.com;
zhangzhuoyu@cmss.chinamobile.com; linuxppc-dev@lists.ozlabs.org;
linux- kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context =

in

quoted

migrate_irqs()

On 12/16/15, Michael Ellerman [off-list ref] wrote:

quoted

On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:

quoted

Hi,

A couple of things.

Firstly, your two email addresses don't match:

Zhang Zhuoyu [off-list ref] writes:

quoted

quoted

From: Zhang Zhuoyu <redacted>

=20
Mmm, My mistake, I will correct it next time.
=20

quoted

quoted

quoted

These lines do seem odd! Are they causing a problem?

I'd be more comfortable removing them if I understood why they =

were

quoted

quoted

quoted

added. But they've been around since the beginning of git history
so that could be a bit difficult.

It's in the fullhist tree, but that doesn't tell us much (below,
named fixup_irqs()).

But I suspect those lines are actually there very deliberately.

The function is migrating interrupts off the recently offlined =

cpu,

quoted

quoted

because we don't want to take interrupts on an offline cpu.

After it's finished doing the migration, it wants to make sure =

there

quoted

quoted

are no interrupts that have already been latched by the offline =

cpu.

quoted

quoted

So it briefly enables interrupts, waits a bit for the interrupts =

to

quoted

quoted

fire, and the disables them again.

Whether that actually works I couldn't say, it is very old code, =

and

quoted

quoted

it's used on platforms where I don't ever test cpu hotplug (85xx &
powermac).

Yeah, it would be nice to test this change. I'll try it on my
quad-core pmac machine

=20
Thanks Michael for help explaining the code logic, it also resolved my =

doubts.

These snippets are suspected when I did PM benchmark on FSL MPC85xx
series(T1040, T4240), For T4240, which has 24 CPUs, it waits 1ms in
migrate_irqs()each time a CPU is plugged offline, it seems a waste of =

time. I

also did a test on T1040, after plugging offline/online CPU hundreds =

of times,

system works well. If you have any other suggestion on how to test, =

I'd like

to do more benchmark.
(1)for((i=3D0; i<1000; i++)); do echo 0 > =

/sys/devices/system/cpu/cpu1/online;

sleep 1; echo 1 > /sys/devices/system/cpu/cpu1/online; done
(2)root@t1040rdb:~# cat /proc/interrupts
           CPU0       CPU1       CPU2       CPU3
 36:        393          1        223          1   OpenPIC    36 Level =

    serial

LOC:       1946       1486       1555       1361   Local timer =

interrupts

DBL:       7371       9707       9390       7568   Doorbell interrupts
(3)root@t1040rdb:~# ps
  PID TTY          TIME CMD
 2751 ttyS0    00:00:00 sh
 2757 ttyS0    00:00:00 ps
root@t1040rdb:~# taskset -pc 1 2751
pid 2751's current affinity list: 0-3
pid 2751's new affinity list: 1
root@t1040rdb:~#
root@t1040rdb:~#
root@t1040rdb:~#
root@t1040rdb:~# echo "hello"
hello
root@t1040rdb:~#
=20

quoted

quoted

cheers


commit d58830b9a740ad1c3b089196d4afdaea251dc701
Author: Zwane Mwaikambo [off-list ref]
Date:   Fri Mar 4 17:34:00 2005 -0800

    [PATCH] ppc64: generic hotplug cpu support

    Patch provides a generic hotplug cpu implementation, with the
only current
    user being pmac.  This doesn't replace real hotplug code as is =

currently

quoted

quoted

    used by LPAR systems.  Ben i can add the additional pmac
specific code to
    put the processor into a sleeping state seperately.  Thanks to
Nathan for
    testing.

    Signed-off-by: Zwane Mwaikambo [off-list ref]
    Signed-off-by: Andrew Morton [off-list ref]
    Signed-off-by: Linus Torvalds [off-list ref]

diff --git a/arch/ppc64/Kconfig b/arch/ppc64/Kconfig index

a7933ab62e98..861f4460ad02 100644

--- a/arch/ppc64/Kconfig
+++ b/arch/ppc64/Kconfig

@@ -313,7 +313,7 @@ source "drivers/pci/Kconfig"

 config HOTPLUG_CPU
 	bool "Support for hot-pluggable CPUs"
-	depends on SMP && EXPERIMENTAL && PPC_PSERIES
+	depends on SMP && EXPERIMENTAL && (PPC_PSERIES ||

PPC_PMAC)

quoted

 	select HOTPLUG
 	---help---
 	  Say Y here to be able to turn CPUs off and on.

diff --git a/arch/ppc64/kernel/idle.c b/arch/ppc64/kernel/idle.c
index 398b4682127b..51eb6af14a8f 100644
--- a/arch/ppc64/kernel/idle.c
+++ b/arch/ppc64/kernel/idle.c

@@ -293,6 +293,10 @@ static int native_idle(void)
 			power4_idle();
 		if (need_resched())
 			schedule();
+
+		if (cpu_is_offline(smp_processor_id()) &&
+		    system_state =3D=3D SYSTEM_RUNNING)
+			cpu_die();
 	}
 	return 0;
 }

diff --git a/arch/ppc64/kernel/irq.c b/arch/ppc64/kernel/irq.c =

index

quoted

quoted

0ea8016146a2..4fd7f203c1e3 100644

--- a/arch/ppc64/kernel/irq.c
+++ b/arch/ppc64/kernel/irq.c

@@ -116,6 +116,35 @@ skip:
 	return 0;
 }

+#ifdef CONFIG_HOTPLUG_CPU
+void fixup_irqs(cpumask_t map)
+{
+	unsigned int irq;
+	static int warned;
+
+	for_each_irq(irq) {
+		cpumask_t mask;
+
+		if (irq_desc[irq].status & IRQ_PER_CPU)
+			continue;
+
+		cpus_and(mask, irq_affinity[irq], map);
+		if (any_online_cpu(mask) =3D=3D NR_CPUS) {
+			printk("Breaking affinity for irq %i\n", irq);
+			mask =3D map;
+		}
+		if (irq_desc[irq].handler->set_affinity)
+			irq_desc[irq].handler->set_affinity(irq, mask);
+		else if (irq_desc[irq].action && !(warned++))
+			printk("Cannot set affinity for irq %i\n", irq);
+	}
+
+	local_irq_enable();
+	mdelay(1);
+	local_irq_disable();
+}
+#endif
+
 extern int noirqdebug;

 /*

diff --git a/arch/ppc64/kernel/pSeries_setup.c

b/arch/ppc64/kernel/pSeries_setup.c
index f603397b7b04..0426892749c6 100644

--- a/arch/ppc64/kernel/pSeries_setup.c
+++ b/arch/ppc64/kernel/pSeries_setup.c

@@ -320,8 +320,9 @@ static  void __init pSeries_discover_pic(void)
 	}
 }

-static void pSeries_cpu_die(void)
+static void pSeries_mach_cpu_die(void)
 {
+	idle_task_exit();
 	local_irq_disable();
 	/* Some hardware requires clearing the CPPR, while other =

hardware

quoted

does not

quoted

 	 * it is safe either way

@@ -599,7 +600,7 @@ struct machdep_calls __initdata pSeries_md =3D =

{

quoted

quoted

 	.power_off		=3D rtas_power_off,
 	.halt			=3D rtas_halt,
 	.panic			=3D rtas_os_term,
-	.cpu_die		=3D pSeries_cpu_die,
+	.cpu_die		=3D pSeries_mach_cpu_die,
 	.get_boot_time		=3D pSeries_get_boot_time,
 	.get_rtc_time		=3D pSeries_get_rtc_time,
 	.set_rtc_time		=3D pSeries_set_rtc_time,

diff --git a/arch/ppc64/kernel/pmac_setup.c

b/arch/ppc64/kernel/pmac_setup.c index 41fa6e95a06f..5c56fc956245
100644

--- a/arch/ppc64/kernel/pmac_setup.c
+++ b/arch/ppc64/kernel/pmac_setup.c

@@ -439,6 +439,9 @@ static int __init pmac_probe(int platform)  }

 struct machdep_calls __initdata pmac_md =3D {
+#ifdef CONFIG_HOTPLUG_CPU
+	.cpu_die		=3D generic_mach_cpu_die,
+#endif
 	.probe			=3D pmac_probe,
 	.setup_arch		=3D pmac_setup_arch,
 	.init_early		=3D pmac_init_early,

diff --git a/arch/ppc64/kernel/pmac_smp.c

b/arch/ppc64/kernel/pmac_smp.c index e0b37079943c..c27588ede2fe

100644

quoted

--- a/arch/ppc64/kernel/pmac_smp.c
+++ b/arch/ppc64/kernel/pmac_smp.c

@@ -308,4 +308,9 @@ struct smp_ops_t core99_smp_ops __pmacdata =3D

{

quoted

quoted

void __init pmac_setup_smp(void)  {
 	smp_ops =3D &core99_smp_ops;
+#ifdef CONFIG_HOTPLUG_CPU
+	smp_ops->cpu_enable =3D generic_cpu_enable;
+	smp_ops->cpu_disable =3D generic_cpu_disable;
+	smp_ops->cpu_die =3D generic_cpu_die; #endif
 }

diff --git a/arch/ppc64/kernel/setup.c b/arch/ppc64/kernel/setup.c
index d98c320828e5..078c3551ce8a 100644
--- a/arch/ppc64/kernel/setup.c
+++ b/arch/ppc64/kernel/setup.c

@@ -1377,9 +1377,6 @@ early_param("xmon", early_xmon);

 void cpu_die(void)
 {
-	idle_task_exit();
 	if (ppc_md.cpu_die)
 		ppc_md.cpu_die();
-	local_irq_disable();
-	for (;;);
 }

diff --git a/arch/ppc64/kernel/smp.c b/arch/ppc64/kernel/smp.c =

index

quoted

quoted

a9e43792f8fe..cde1947432a1 100644

--- a/arch/ppc64/kernel/smp.c
+++ b/arch/ppc64/kernel/smp.c

@@ -30,6 +30,7 @@
 #include <linux/err.h>
 #include <linux/sysdev.h>
 #include <linux/cpu.h>
+#include <linux/notifier.h>

 #include <asm/ptrace.h>
 #include <asm/atomic.h>

@@ -406,10 +407,89 @@ void __devinit smp_prepare_boot_cpu(void)
 	current_set[boot_cpuid] =3D current->thread_info;  }

+#ifdef CONFIG_HOTPLUG_CPU
+/* State of each CPU during hotplug phases */ DEFINE_PER_CPU(int,
+cpu_state) =3D { 0 };
+
+int generic_cpu_disable(void)
+{
+	unsigned int cpu =3D smp_processor_id();
+
+	if (cpu =3D=3D boot_cpuid)
+		return -EBUSY;
+
+	systemcfg->processorCount--;
+	cpu_clear(cpu, cpu_online_map);
+	fixup_irqs(cpu_online_map);
+	return 0;
+}
+
+int generic_cpu_enable(unsigned int cpu) {
+	/* Do the normal bootup if we haven't
+	 * already bootstrapped. */
+	if (system_state !=3D SYSTEM_RUNNING)
+		return -ENOSYS;
+
+	/* get the target out of it's holding state */
+	per_cpu(cpu_state, cpu) =3D CPU_UP_PREPARE;
+	wmb();
+
+	while (!cpu_online(cpu))
+		cpu_relax();
+
+	fixup_irqs(cpu_online_map);
+	/* counter the irq disable in fixup_irqs */
+	local_irq_enable();
+	return 0;
+}
+
+void generic_cpu_die(unsigned int cpu) {
+	int i;
+
+	for (i =3D 0; i < 100; i++) {
+		rmb();
+		if (per_cpu(cpu_state, cpu) =3D=3D CPU_DEAD)
+			return;
+		msleep(100);
+	}
+	printk(KERN_ERR "CPU%d didn't die...\n", cpu); }
+
+void generic_mach_cpu_die(void)
+{
+	unsigned int cpu;
+
+	local_irq_disable();
+	cpu =3D smp_processor_id();
+	printk(KERN_DEBUG "CPU%d offline\n", cpu);
+	__get_cpu_var(cpu_state) =3D CPU_DEAD;
+	wmb();
+	while (__get_cpu_var(cpu_state) !=3D CPU_UP_PREPARE)
+		cpu_relax();
+
+	flush_tlb_pending();
+	cpu_set(cpu, cpu_online_map);
+	local_irq_enable();
+}
+#endif
+
+static int __devinit cpu_enable(unsigned int cpu) {
+	if (smp_ops->cpu_enable)
+		return smp_ops->cpu_enable(cpu);
+
+	return -ENOSYS;
+}
+
 int __devinit __cpu_up(unsigned int cpu)  {
 	int c;

+	if (!cpu_enable(cpu))
+		return 0;
+
 	/* At boot, don't bother with non-present cpus -JSCHOPP */
 	if (system_state < SYSTEM_RUNNING && !cpu_present(cpu))
 		return -ENOENT;

diff --git a/arch/ppc64/kernel/sysfs.c b/arch/ppc64/kernel/sysfs.c
index bbc9dcda17f7..0925694c3ce5 100644
--- a/arch/ppc64/kernel/sysfs.c
+++ b/arch/ppc64/kernel/sysfs.c

@@ -18,7 +18,7 @@
 #include <asm/systemcfg.h>
 #include <asm/paca.h>
 #include <asm/lppaca.h>
-
+#include <asm/machdep.h>

 static DEFINE_PER_CPU(struct cpu, cpu_devices);

@@ -413,9 +413,7 @@ static int __init topology_init(void)
 		 * CPU.  For instance, the boot cpu might never be valid
 		 * for hotplugging.
 		 */
-#ifdef CONFIG_HOTPLUG_CPU
-		if (systemcfg->platform !=3D PLATFORM_PSERIES_LPAR)
-#endif
+		if (!ppc_md.cpu_die)
 			c->no_control =3D 1;

 		if (cpu_online(cpu) || (c->no_control =3D=3D 0)) { diff --git

a/include/asm-ppc64/machdep.h b/include/asm-ppc64/machdep.h

index

quoted

quoted

476d2185ffd1..03fe499c7604 100644

--- a/include/asm-ppc64/machdep.h
+++ b/include/asm-ppc64/machdep.h

@@ -30,6 +30,7 @@ struct smp_ops_t {
 	void  (*setup_cpu)(int nr);
 	void  (*take_timebase)(void);
 	void  (*give_timebase)(void);
+	int   (*cpu_enable)(unsigned int nr);
 	int   (*cpu_disable)(void);
 	void  (*cpu_die)(unsigned int nr);  }; diff --git

a/include/asm-ppc64/smp.h b/include/asm-ppc64/smp.h index
965980bbbb57..c8646fa999c2 100644

--- a/include/asm-ppc64/smp.h
+++ b/include/asm-ppc64/smp.h

@@ -29,7 +29,7 @@
 extern int boot_cpuid;
 extern int boot_cpuid_phys;

-extern void cpu_die(void) __attribute__((noreturn));
+extern void cpu_die(void);

 #ifdef CONFIG_SMP

@@ -37,6 +37,13 @@ extern void smp_send_debugger_break(int cpu);

struct pt_regs;  extern void smp_message_recv(int, struct pt_regs
*);

+#ifdef CONFIG_HOTPLUG_CPU
+extern void fixup_irqs(cpumask_t map); int
+generic_cpu_disable(void); int generic_cpu_enable(unsigned int
+cpu); void generic_cpu_die(unsigned int cpu); void
+generic_mach_cpu_die(void); #endif

 #define __smp_processor_id() (get_paca()->paca_index)  #define
hard_smp_processor_id() (get_paca()->hw_cpu_id)

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev