Hi Sebastian


On 07/09/16 15:27, Sebastian Andrzej Siewior wrote:

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

quoted

HI Sebastian,

Hi Matt,

quoted

quoted

--- 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

quoted

Thanks,
Matt

Sebastian