[PATCH 15/21] mips: octeon: smp: Convert to hotplug state machine

[PATCH 15/21] mips: octeon: smp: Convert to hotplug state machine

[Sebastian Andrzej Siewior]

Install the callbacks via the state machine.

Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/mips/cavium-octeon/smp.c | 24 +++---------------------
 include/linux/cpuhotplug.h    |  1 +
 2 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c
index 4d457d602d3b..228c1cab2912 100644
--- a/arch/mips/cavium-octeon/smp.c
+++ b/arch/mips/cavium-octeon/smp.c
@@ -380,29 +380,11 @@ static int octeon_update_boot_vector(unsigned int cpu)
 	return 0;
 }
 
-static int octeon_cpu_callback(struct notifier_block *nfb,
-	unsigned long action, void *hcpu)
-{
-	unsigned int cpu = (unsigned long)hcpu;
-
-	switch (action & ~CPU_TASKS_FROZEN) {
-	case CPU_UP_PREPARE:
-		octeon_update_boot_vector(cpu);
-		break;
-	case CPU_ONLINE:
-		pr_info("Cpu %d online\n", cpu);
-		break;
-	case CPU_DEAD:
-		break;
-	}
-
-	return NOTIFY_OK;
-}
-
 static int register_cavium_notifier(void)
 {
-	hotcpu_notifier(octeon_cpu_callback, 0);
-	return 0;
+	return cpuhp_setup_state_nocalls(CPUHP_MIPS_CAVIUM_PREPARE,
+					 "mips/cavium:prepare",
+					 octeon_update_boot_vector, NULL);
 }
 late_initcall(register_cavium_notifier);
 
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 4cbf88c955d6..058d312fdf6f 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -44,6 +44,7 @@ enum cpuhp_state {
 	CPUHP_SH_SH3X_PREPARE,
 	CPUHP_X86_MICRCODE_PREPARE,
 	CPUHP_NOTF_ERR_INJ_PREPARE,
+	CPUHP_MIPS_CAVIUM_PREPARE,
 	CPUHP_TIMERS_DEAD,
 	CPUHP_BRINGUP_CPU,
 	CPUHP_AP_IDLE_DEAD,
-- 
2.9.3

Re: [PATCH 15/21] mips: octeon: smp: Convert to hotplug state machine

[Matt Redfearn]

HI Sebastian,


On 06/09/16 18:04, Sebastian Andrzej Siewior wrote:
> Install the callbacks via the state machine.
>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   arch/mips/cavium-octeon/smp.c | 24 +++---------------------
>   include/linux/cpuhotplug.h    |  1 +
>   2 files changed, 4 insertions(+), 21 deletions(-)
>
> diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c
> index 4d457d602d3b..228c1cab2912 100644
> --- a/arch/mips/cavium-octeon/smp.c
> +++ b/arch/mips/cavium-octeon/smp.c
> @@ -380,29 +380,11 @@ static int octeon_update_boot_vector(unsigned int cpu)
>   	return 0;
>   }
>   
> -static int octeon_cpu_callback(struct notifier_block *nfb,
> -	unsigned long action, void *hcpu)
> -{
> -	unsigned int cpu = (unsigned long)hcpu;
> -
> -	switch (action & ~CPU_TASKS_FROZEN) {
> -	case CPU_UP_PREPARE:
> -		octeon_update_boot_vector(cpu);
> -		break;
> -	case CPU_ONLINE:
> -		pr_info("Cpu %d online\n", cpu);
> -		break;
> -	case CPU_DEAD:
> -		break;
> -	}
> -
> -	return NOTIFY_OK;
> -}
> -
>   static int register_cavium_notifier(void)
>   {
> -	hotcpu_notifier(octeon_cpu_callback, 0);
> -	return 0;
> +	return cpuhp_setup_state_nocalls(CPUHP_MIPS_CAVIUM_PREPARE,
> +					 "mips/cavium:prepare",
> +					 octeon_update_boot_vector, NULL);

This looks like a nice change which results in much cleaner code

>   }
>   late_initcall(register_cavium_notifier);
>   
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 4cbf88c955d6..058d312fdf6f 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -44,6 +44,7 @@ enum cpuhp_state {
>   	CPUHP_SH_SH3X_PREPARE,
>   	CPUHP_X86_MICRCODE_PREPARE,
>   	CPUHP_NOTF_ERR_INJ_PREPARE,
> +	CPUHP_MIPS_CAVIUM_PREPARE,

But I'm curious why we have to create a new state here - this is going 
to get very unwieldy if every variant of every architecture has to have 
it's own state values in that enumeration. Can this use, what I assume 
is (and perhaps could be documented better in 
include/linux/cpuhotplug.h), the generic prepare state CPUHP_NOTIFY_PREPARE?

Thanks,
Matt

>   	CPUHP_TIMERS_DEAD,
>   	CPUHP_BRINGUP_CPU,
>   	CPUHP_AP_IDLE_DEAD,

Re: [PATCH 15/21] mips: octeon: smp: Convert to hotplug state machine

[Matt Redfearn]

HI Sebastian,


On 06/09/16 18:04, Sebastian Andrzej Siewior wrote:
> Install the callbacks via the state machine.
>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   arch/mips/cavium-octeon/smp.c | 24 +++---------------------
>   include/linux/cpuhotplug.h    |  1 +
>   2 files changed, 4 insertions(+), 21 deletions(-)
>
> diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c
> index 4d457d602d3b..228c1cab2912 100644
> --- a/arch/mips/cavium-octeon/smp.c
> +++ b/arch/mips/cavium-octeon/smp.c
> @@ -380,29 +380,11 @@ static int octeon_update_boot_vector(unsigned int cpu)
>   	return 0;
>   }
>   
> -static int octeon_cpu_callback(struct notifier_block *nfb,
> -	unsigned long action, void *hcpu)
> -{
> -	unsigned int cpu = (unsigned long)hcpu;
> -
> -	switch (action & ~CPU_TASKS_FROZEN) {
> -	case CPU_UP_PREPARE:
> -		octeon_update_boot_vector(cpu);
> -		break;
> -	case CPU_ONLINE:
> -		pr_info("Cpu %d online\n", cpu);
> -		break;
> -	case CPU_DEAD:
> -		break;
> -	}
> -
> -	return NOTIFY_OK;
> -}
> -
>   static int register_cavium_notifier(void)
>   {
> -	hotcpu_notifier(octeon_cpu_callback, 0);
> -	return 0;
> +	return cpuhp_setup_state_nocalls(CPUHP_MIPS_CAVIUM_PREPARE,
> +					 "mips/cavium:prepare",
> +					 octeon_update_boot_vector, NULL);

This looks like a nice change which results in much cleaner code

>   }
>   late_initcall(register_cavium_notifier);
>   
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 4cbf88c955d6..058d312fdf6f 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -44,6 +44,7 @@ enum cpuhp_state {
>   	CPUHP_SH_SH3X_PREPARE,
>   	CPUHP_X86_MICRCODE_PREPARE,
>   	CPUHP_NOTF_ERR_INJ_PREPARE,
> +	CPUHP_MIPS_CAVIUM_PREPARE,

But I'm curious why we have to create a new state here - this is going 
to get very unwieldy if every variant of every architecture has to have 
it's own state values in that enumeration. Can this use, what I assume 
is (and perhaps could be documented better in 
include/linux/cpuhotplug.h), the generic prepare state CPUHP_NOTIFY_PREPARE?

Thanks,
Matt

>   	CPUHP_TIMERS_DEAD,
>   	CPUHP_BRINGUP_CPU,
>   	CPUHP_AP_IDLE_DEAD,

Re: [PATCH 15/21] mips: octeon: smp: Convert to hotplug state machine

[Sebastian Andrzej Siewior]

On 2016-09-07 09:24:57 [+0100], Matt Redfearn wrote:
> HI Sebastian,
Hi Matt,

> > --- a/include/linux/cpuhotplug.h
> > +++ b/include/linux/cpuhotplug.h
> > @@ -44,6 +44,7 @@ enum cpuhp_state {
> >   	CPUHP_SH_SH3X_PREPARE,
> >   	CPUHP_X86_MICRCODE_PREPARE,
> >   	CPUHP_NOTF_ERR_INJ_PREPARE,
> > +	CPUHP_MIPS_CAVIUM_PREPARE,
> 
> But I'm curious why we have to create a new state here - this is going to
> get very unwieldy if every variant of every architecture has to have it's
> own state values in that enumeration. Can this use, what I assume is (and
> perhaps could be documented better in include/linux/cpuhotplug.h), the
> generic prepare state CPUHP_NOTIFY_PREPARE?

We can't share the states - one state is for one callback and one
callback only. If you want CPUHP_MIPS_PREPARE and you ensure that this
used only _once_ then this can be arranged.
For online states we have dynamic allocation of ids (which is what most
drivers should use). We don't have this of STARTING + PREPARE callbacks.

> Thanks,
> Matt
> 

Sebastian

Re: [PATCH 15/21] mips: octeon: smp: Convert to hotplug state machine

[Matt Redfearn]

Hi Sebastian


On 07/09/16 15:27, Sebastian Andrzej Siewior wrote:
> On 2016-09-07 09:24:57 [+0100], Matt Redfearn wrote:
>> HI Sebastian,
> Hi Matt,
>
>>> --- a/include/linux/cpuhotplug.h
>>> +++ b/include/linux/cpuhotplug.h
>>> @@ -44,6 +44,7 @@ enum cpuhp_state {
>>>    	CPUHP_SH_SH3X_PREPARE,
>>>    	CPUHP_X86_MICRCODE_PREPARE,
>>>    	CPUHP_NOTF_ERR_INJ_PREPARE,
>>> +	CPUHP_MIPS_CAVIUM_PREPARE,
>> But I'm curious why we have to create a new state here - this is going to
>> get very unwieldy if every variant of every architecture has to have it's
>> own state values in that enumeration. Can this use, what I assume is (and
>> perhaps could be documented better in include/linux/cpuhotplug.h), the
>> generic prepare state CPUHP_NOTIFY_PREPARE?
> We can't share the states - one state is for one callback and one
> callback only. If you want CPUHP_MIPS_PREPARE and you ensure that this
> used only _once_ then this can be arranged.
> For online states we have dynamic allocation of ids (which is what most
> drivers should use). We don't have this of STARTING + PREPARE callbacks.

OK, fair enough - it just feels slightly unwieldy. That enumeration is 
going to grow to an enormous size to store every single callback which 
could be used, when no kernel is going to use all states, the majority 
of which exist for other architectures. As a result cpuhp_bp_states and 
cpuhp_ap_states are going to waste memory storing states that the kernel 
won't use.
But that's the design decision taken, so fine. I think we have to keep 
one enumeration value associated with one callback definition - anything 
else is going to end in a mess, so lets keep the values as you suggest.

Thanks,
Matt


>
>> Thanks,
>> Matt
>>
> Sebastian

Re: [PATCH 15/21] mips: octeon: smp: Convert to hotplug state machine

[Matt Redfearn]

Hi Sebastian


On 07/09/16 15:27, Sebastian Andrzej Siewior wrote:
> On 2016-09-07 09:24:57 [+0100], Matt Redfearn wrote:
>> HI Sebastian,
> Hi Matt,
>
>>> --- a/include/linux/cpuhotplug.h
>>> +++ b/include/linux/cpuhotplug.h
>>> @@ -44,6 +44,7 @@ enum cpuhp_state {
>>>    	CPUHP_SH_SH3X_PREPARE,
>>>    	CPUHP_X86_MICRCODE_PREPARE,
>>>    	CPUHP_NOTF_ERR_INJ_PREPARE,
>>> +	CPUHP_MIPS_CAVIUM_PREPARE,
>> But I'm curious why we have to create a new state here - this is going to
>> get very unwieldy if every variant of every architecture has to have it's
>> own state values in that enumeration. Can this use, what I assume is (and
>> perhaps could be documented better in include/linux/cpuhotplug.h), the
>> generic prepare state CPUHP_NOTIFY_PREPARE?
> We can't share the states - one state is for one callback and one
> callback only. If you want CPUHP_MIPS_PREPARE and you ensure that this
> used only _once_ then this can be arranged.
> For online states we have dynamic allocation of ids (which is what most
> drivers should use). We don't have this of STARTING + PREPARE callbacks.

OK, fair enough - it just feels slightly unwieldy. That enumeration is 
going to grow to an enormous size to store every single callback which 
could be used, when no kernel is going to use all states, the majority 
of which exist for other architectures. As a result cpuhp_bp_states and 
cpuhp_ap_states are going to waste memory storing states that the kernel 
won't use.
But that's the design decision taken, so fine. I think we have to keep 
one enumeration value associated with one callback definition - anything 
else is going to end in a mess, so lets keep the values as you suggest.

Thanks,
Matt


>
>> Thanks,
>> Matt
>>
> Sebastian

Re: [PATCH 15/21] mips: octeon: smp: Convert to hotplug state machine

[Ralf Baechle]

On Tue, Sep 06, 2016 at 07:04:51PM +0200, Sebastian Andrzej Siewior wrote:

I assume you want to upstream this as a single pull request, so

Acked-by: Ralf Baechle <ralf@linux-mips.org>

  Ralf

[PATCH 16/21] mips: loongson: smp: Convert to hotplug state machine

[Sebastian Andrzej Siewior]

Install the callbacks via the state machine.

Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/mips/loongson64/loongson-3/smp.c | 34 ++++++++--------------------------
 include/linux/cpuhotplug.h            |  1 +
 2 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/arch/mips/loongson64/loongson-3/smp.c b/arch/mips/loongson64/loongson-3/smp.c
index 2fec6f753a35..bd7c8cf5332d 100644
--- a/arch/mips/loongson64/loongson-3/smp.c
+++ b/arch/mips/loongson64/loongson-3/smp.c
@@ -677,7 +677,7 @@ void play_dead(void)
 	play_dead_at_ckseg1(state_addr);
 }
 
-void loongson3_disable_clock(int cpu)
+static int loongson3_disable_clock(unsigned int cpu)
 {
 	uint64_t core_id = cpu_data[cpu].core;
 	uint64_t package_id = cpu_data[cpu].package;
@@ -688,9 +688,10 @@ void loongson3_disable_clock(int cpu)
 		if (!(loongson_sysconf.workarounds & WORKAROUND_CPUHOTPLUG))
 			LOONGSON_FREQCTRL(package_id) &= ~(1 << (core_id * 4 + 3));
 	}
+	return 0;
 }
 
-void loongson3_enable_clock(int cpu)
+static int loongson3_enable_clock(unsigned int cpu)
 {
 	uint64_t core_id = cpu_data[cpu].core;
 	uint64_t package_id = cpu_data[cpu].package;
@@ -701,34 +702,15 @@ void loongson3_enable_clock(int cpu)
 		if (!(loongson_sysconf.workarounds & WORKAROUND_CPUHOTPLUG))
 			LOONGSON_FREQCTRL(package_id) |= 1 << (core_id * 4 + 3);
 	}
-}
-
-#define CPU_POST_DEAD_FROZEN	(CPU_POST_DEAD | CPU_TASKS_FROZEN)
-static int loongson3_cpu_callback(struct notifier_block *nfb,
-	unsigned long action, void *hcpu)
-{
-	unsigned int cpu = (unsigned long)hcpu;
-
-	switch (action) {
-	case CPU_POST_DEAD:
-	case CPU_POST_DEAD_FROZEN:
-		pr_info("Disable clock for CPU#%d\n", cpu);
-		loongson3_disable_clock(cpu);
-		break;
-	case CPU_UP_PREPARE:
-	case CPU_UP_PREPARE_FROZEN:
-		pr_info("Enable clock for CPU#%d\n", cpu);
-		loongson3_enable_clock(cpu);
-		break;
-	}
-
-	return NOTIFY_OK;
+	return 0;
 }
 
 static int register_loongson3_notifier(void)
 {
-	hotcpu_notifier(loongson3_cpu_callback, 0);
-	return 0;
+	return cpuhp_setup_state_nocalls(CPUHP_MIPS_LOONGSON_PREPARE,
+					 "mips/loongson:prepare",
+					 loongson3_enable_clock,
+					 loongson3_disable_clock);
 }
 early_initcall(register_loongson3_notifier);
 
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 058d312fdf6f..9e56be3eaf3f 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -45,6 +45,7 @@ enum cpuhp_state {
 	CPUHP_X86_MICRCODE_PREPARE,
 	CPUHP_NOTF_ERR_INJ_PREPARE,
 	CPUHP_MIPS_CAVIUM_PREPARE,
+	CPUHP_MIPS_LOONGSON_PREPARE,
 	CPUHP_TIMERS_DEAD,
 	CPUHP_BRINGUP_CPU,
 	CPUHP_AP_IDLE_DEAD,
-- 
2.9.3

Re: [PATCH 16/21] mips: loongson: smp: Convert to hotplug state machine

[Ralf Baechle]

On Tue, Sep 06, 2016 at 07:04:52PM +0200, Sebastian Andrzej Siewior wrote:

> Install the callbacks via the state machine.

Same here, I assume you want to upstream this as a single pull request, so

Acked-by: Ralf Baechle <ralf@linux-mips.org>

  Ralf