Re: [PATCH v2] powerpc: don't enable cpu hotplug on mpic-based pseries

2007-10-11 Thread Milton Miller
On Oct 10, 2007, at 11:43 AM, Olof Johansson wrote:
 On Wed, Oct 10, 2007 at 05:08:44AM -0500, Milton Miller wrote:
 Olof's patch searched the device-tree again, looking for an mpic.   
 This
 code instead checks that we found an xics the first time by checking 
 the
 init function.

 I'm glad you find the kernel so perfect that your best use of time is
 to tweak the code added in a non-critical path (performance-wise) with
 a new way of checking for hardware features (no other code in the 
 kernel
 checks ppc_md function pointers for that purpose).

I never argued performance, I posted because !mpic != !!xics.  The 
tweak of not searching again was about avoiding code duplication.  I 
considered moving the call from an init function to an explicit call 
when we find it like the smp and kexec hooks are handled.   It would 
reorder the notifier registration (from arch_initcall to setup_arch) 
which would require additional testing.

 Pseries can only have mpic or xics, so it's not like it matters if we
 check for mpic or !xics, either.

Actually, in my work with simulators, sometimes I use neither, so the 
check is not equivalent.  We recently changed the call to 
ppc_md.init_IRQ to allow this.

 But hey, any color bike shed will do, and I don't care which one Paul
 chooses to merge. I will not waste more time debating this simple patch
 though. :)

 -Olof

milton

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH v2] powerpc: don't enable cpu hotplug on mpic-based pseries

2007-10-11 Thread Milton Miller
Paul Mackerras writes:
 Olof Johansson writes:
 Don't allow cpu hotplug on systems lacking XICS interrupt controller,
 since current code is hardcoded for it.
 ...
 +for (np = NULL; (np = of_find_node_by_name(np,
 +   interrupt-controller));) {
 
 Looks like for_each_node_by_name would be nicer here.
 
 If you agree, I'll hand-edit your patch to do that and apply it.  Of
 course, ultimately we should implement the necessary mpic bits to
 support cpu hotplug.
 

While you are editing, can you please change the condition to finding
ppc-xicp instead of not finding open-pic?

thanks.
milton

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH v2] powerpc: don't enable cpu hotplug on mpic-based pseries

2007-10-10 Thread Milton Miller

Don't allow cpu hotplug on pSeries systems lacking XICS interrupt controller,
since current code is hardcoded to call xics routines.

Signed-off-by: Milton Miller [EMAIL PROTECTED]
--

Olof's patch searched the device-tree again, looking for an mpic.   This
code instead checks that we found an xics the first time by checking the
init function.

diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c 
b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 9711eb0..20f010a 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -262,6 +262,12 @@ static int __init pseries_cpu_hotplug_init(void)
return 0;
}
 
+   if (ppc_md.init_IRQ != xics_init_IRQ) {
+   printk(KERN_INFO pSeries CPU Hotplug only supported on xics 
+   interrupt controllers - disabling);
+   return 0;
+   }
+
ppc_md.cpu_die = pseries_mach_cpu_die;
smp_ops-cpu_disable = pseries_cpu_disable;
smp_ops-cpu_die = pseries_cpu_die;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH v2] powerpc: don't enable cpu hotplug on mpic-based pseries

2007-10-10 Thread Olof Johansson
On Wed, Oct 10, 2007 at 05:08:44AM -0500, Milton Miller wrote:
 Olof's patch searched the device-tree again, looking for an mpic.   This
 code instead checks that we found an xics the first time by checking the
 init function.

I'm glad you find the kernel so perfect that your best use of time is
to tweak the code added in a non-critical path (performance-wise) with
a new way of checking for hardware features (no other code in the kernel
checks ppc_md function pointers for that purpose).

Pseries can only have mpic or xics, so it's not like it matters if we
check for mpic or !xics, either.

But hey, any color bike shed will do, and I don't care which one Paul
chooses to merge. I will not waste more time debating this simple patch
though. :)


-Olof
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH v2] powerpc: don't enable cpu hotplug on mpic-based pseries

2007-10-10 Thread Olof Johansson
On Thu, Oct 11, 2007 at 03:52:04PM +1000, Paul Mackerras wrote:
 Olof Johansson writes:
 
  Don't allow cpu hotplug on systems lacking XICS interrupt controller,
  since current code is hardcoded for it.
 ...
  +   for (np = NULL; (np = of_find_node_by_name(np,
  +  interrupt-controller));) {
 
 Looks like for_each_node_by_name would be nicer here.
 
 If you agree, I'll hand-edit your patch to do that and apply it.

Go for it, I just stole that out of the detection code. That
could be changed accordingly as well at some point (I'm guessing
for_each_node_by_name didn't exist when that was written).

 Of course, ultimately we should implement the necessary mpic bits to
 support cpu hotplug.

Yep. I'll leave that to people who have hardware access. :-)


Thanks,

-Olof
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev