* Re: [stable] [patch 09/12] Fix SMP poweroff hangs [not found] ` <alpine.LFD.0.999.0710091625520.3838@woody.linux-foundation.org> @ 2007-10-10 0:03 ` Olof Johansson 2007-10-10 0:08 ` [PATCH] powerpc: don't enable cpu hotplug on mpic-based pseries Olof Johansson 1 sibling, 0 replies; 10+ messages in thread From: Olof Johansson @ 2007-10-10 0:03 UTC (permalink / raw) To: Linus Torvalds Cc: Mark Lord, Theodore Ts'o, Zwane Mwaikambo, linuxppc-dev, Greg KH, Greg KH, Justin Forbes, linux-kernel, Chris Wedgwood, Domenico Andreoli, Rafael J. Wysocki, stable, Randy Dunlap, Michael Krufky, Chuck Ebbert, Dave Jones, Thomas Gleixner, Chuck Wolber, akpm, alan On Tue, Oct 09, 2007 at 04:27:06PM -0700, Linus Torvalds wrote: > > > On Wed, 10 Oct 2007, Thomas Gleixner wrote: > > > > Wrapping it into a #ifdef CONFIG_X86 would be sufficient. > > Well, the ppc oops seems to be a ppc bug regardless. > > If CPU_HOTPLUG isn't defined, the thing does nothing. And if it is > defined, I don't see why/how ppc can validly oops. So I think the first > thing to do is to try to figure out why it oopses, not to disable it for > ppc. The machine Paul tried on most likely has MPIC interrupt controller, and the oops was when the pseries_cpu_disable tried calling XICS code instead. It's not surprising that it failed, I don't think IBM has (traditionally) cared about cpu hotplug on those machines. So the PPC-side fix is to not enable cpu hotplug on mpic-based systems. I'll follow up with a patch, but I have no way to test it since I only have one POWER5 machine, no other IBM hardware. I'd appreciate it if someone with hardware could verify it. -Olof ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] powerpc: don't enable cpu hotplug on mpic-based pseries [not found] ` <alpine.LFD.0.999.0710091625520.3838@woody.linux-foundation.org> 2007-10-10 0:03 ` [stable] [patch 09/12] Fix SMP poweroff hangs Olof Johansson @ 2007-10-10 0:08 ` Olof Johansson 2007-10-10 0:18 ` Stephen Rothwell 1 sibling, 1 reply; 10+ messages in thread From: Olof Johansson @ 2007-10-10 0:08 UTC (permalink / raw) To: Linus Torvalds Cc: Greg KH, Greg KH, linux-kernel, Chris Wedgwood, linuxppc-dev, paulus, tgall.foo, Thomas Gleixner, stable Don't allow cpu hotplug on systems lacking XICS interrupt controller, since current platform code is hardcoded for it. Signed-off-by: Olof Johansson <olof@lixom•net> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c index 9711eb0..e29b890 100644 --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c @@ -252,6 +252,19 @@ static struct notifier_block pseries_smp_nb = { static int __init pseries_cpu_hotplug_init(void) { + struct device_node *np; + const char *typep; + + for (np = NULL; (np = of_find_node_by_name(np, + "interrupt-controller"));) { + typep = of_get_property(np, "compatible", NULL); + if (strstr(typep, "open-pic")) { + printk(KERN_INFO "CPU Hotplug not supported on " + "systems using MPIC\n"); + return 0; + } + } + rtas_stop_self_args.token = rtas_token("stop-self"); qcss_tok = rtas_token("query-cpu-stopped-state"); ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] powerpc: don't enable cpu hotplug on mpic-based pseries 2007-10-10 0:08 ` [PATCH] powerpc: don't enable cpu hotplug on mpic-based pseries Olof Johansson @ 2007-10-10 0:18 ` Stephen Rothwell 2007-10-10 0:38 ` [PATCH v2] " Olof Johansson 0 siblings, 1 reply; 10+ messages in thread From: Stephen Rothwell @ 2007-10-10 0:18 UTC (permalink / raw) To: Olof Johansson Cc: Greg KH, Greg KH, linux-kernel, Chris Wedgwood, linuxppc-dev, paulus, tgall.foo, Thomas Gleixner, Linus Torvalds, stable [-- Attachment #1: Type: text/plain, Size: 1146 bytes --] Hi Olof, On Tue, 9 Oct 2007 19:08:15 -0500 Olof Johansson <olof@lixom•net> wrote: > > Don't allow cpu hotplug on systems lacking XICS interrupt controller, > since current platform code is hardcoded for it. > > > Signed-off-by: Olof Johansson <olof@lixom•net> > > > diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c > index 9711eb0..e29b890 100644 > --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c > +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c > @@ -252,6 +252,19 @@ static struct notifier_block pseries_smp_nb = { > > static int __init pseries_cpu_hotplug_init(void) > { > + struct device_node *np; > + const char *typep; > + > + for (np = NULL; (np = of_find_node_by_name(np, > + "interrupt-controller"));) { > + typep = of_get_property(np, "compatible", NULL); > + if (strstr(typep, "open-pic")) { > + printk(KERN_INFO "CPU Hotplug not supported on " > + "systems using MPIC\n"); You need an of_node_put(np) here. -- Cheers, Stephen Rothwell sfr@canb•auug.org.au http://www.canb.auug.org.au/~sfr/ [-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] powerpc: don't enable cpu hotplug on mpic-based pseries 2007-10-10 0:18 ` Stephen Rothwell @ 2007-10-10 0:38 ` Olof Johansson 2007-10-10 10:08 ` Milton Miller 2007-10-11 5:52 ` Paul Mackerras 0 siblings, 2 replies; 10+ messages in thread From: Olof Johansson @ 2007-10-10 0:38 UTC (permalink / raw) To: Stephen Rothwell Cc: Greg KH, Greg KH, linux-kernel, Chris Wedgwood, linuxppc-dev, paulus, tgall.foo, Thomas Gleixner, Linus Torvalds, stable Don't allow cpu hotplug on systems lacking XICS interrupt controller, since current code is hardcoded for it. Signed-off-by: Olof Johansson <olof@lixom•net> --- On Wed, Oct 10, 2007 at 10:18:26AM +1000, Stephen Rothwell wrote: > > + struct device_node *np; > > + const char *typep; > > + > > + for (np = NULL; (np = of_find_node_by_name(np, > > + "interrupt-controller"));) { > > + typep = of_get_property(np, "compatible", NULL); > > + if (strstr(typep, "open-pic")) { > > + printk(KERN_INFO "CPU Hotplug not supported on " > > + "systems using MPIC\n"); > > You need an of_node_put(np) here. Grmbl, you're right. pseries_discover_pic() doesn't have one, that's where I took the above logic from. So we're obviously already leaking device node references. Still, no reason to make it worse. -Olof diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c index 9711eb0..ae85fc0 100644 --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c @@ -252,6 +252,21 @@ static struct notifier_block pseries_smp_nb = { static int __init pseries_cpu_hotplug_init(void) { + struct device_node *np; + const char *typep; + + for (np = NULL; (np = of_find_node_by_name(np, + "interrupt-controller"));) { + typep = of_get_property(np, "compatible", NULL); + if (strstr(typep, "open-pic")) { + of_node_put(np); + + printk(KERN_INFO "CPU Hotplug not supported on " + "systems using MPIC\n"); + return 0; + } + } + rtas_stop_self_args.token = rtas_token("stop-self"); qcss_tok = rtas_token("query-cpu-stopped-state"); ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] powerpc: don't enable cpu hotplug on mpic-based pseries 2007-10-10 0:38 ` [PATCH v2] " Olof Johansson @ 2007-10-10 10:08 ` Milton Miller 2007-10-10 16:43 ` Olof Johansson 2007-10-11 5:52 ` Paul Mackerras 1 sibling, 1 reply; 10+ messages in thread From: Milton Miller @ 2007-10-10 10:08 UTC (permalink / raw) To: Olof Johansson, Paul Mackerras, Greg Kroah-Hartman Cc: linuxppc-dev, Linus Torvalds, linux-kernel, Stephen Rothwell 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 <miltonm@bga•com> -- 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; ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] powerpc: don't enable cpu hotplug on mpic-based pseries 2007-10-10 10:08 ` Milton Miller @ 2007-10-10 16:43 ` Olof Johansson 2007-10-11 15:25 ` Milton Miller 0 siblings, 1 reply; 10+ messages in thread From: Olof Johansson @ 2007-10-10 16:43 UTC (permalink / raw) To: Milton Miller Cc: Stephen Rothwell, Greg Kroah-Hartman, linux-kernel, linuxppc-dev, Paul Mackerras, Linus Torvalds 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] powerpc: don't enable cpu hotplug on mpic-based pseries 2007-10-10 16:43 ` Olof Johansson @ 2007-10-11 15:25 ` Milton Miller 0 siblings, 0 replies; 10+ messages in thread From: Milton Miller @ 2007-10-11 15:25 UTC (permalink / raw) To: Olof Johansson; +Cc: ppcdev, Paul Mackerras 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] powerpc: don't enable cpu hotplug on mpic-based pseries 2007-10-10 0:38 ` [PATCH v2] " Olof Johansson 2007-10-10 10:08 ` Milton Miller @ 2007-10-11 5:52 ` Paul Mackerras 2007-10-11 5:59 ` Olof Johansson 2007-10-11 21:42 ` Milton Miller 1 sibling, 2 replies; 10+ messages in thread From: Paul Mackerras @ 2007-10-11 5:52 UTC (permalink / raw) To: Olof Johansson Cc: Stephen Rothwell, Greg KH, Greg KH, linux-kernel, Chris Wedgwood, linuxppc-dev, tgall.foo, Thomas Gleixner, Linus Torvalds, stable 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. Paul. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] powerpc: don't enable cpu hotplug on mpic-based pseries 2007-10-11 5:52 ` Paul Mackerras @ 2007-10-11 5:59 ` Olof Johansson 2007-10-11 21:42 ` Milton Miller 1 sibling, 0 replies; 10+ messages in thread From: Olof Johansson @ 2007-10-11 5:59 UTC (permalink / raw) To: Paul Mackerras Cc: Stephen Rothwell, Greg KH, Greg KH, linux-kernel, Chris Wedgwood, linuxppc-dev, tgall.foo, Thomas Gleixner, Linus Torvalds, stable 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] powerpc: don't enable cpu hotplug on mpic-based pseries 2007-10-11 5:52 ` Paul Mackerras 2007-10-11 5:59 ` Olof Johansson @ 2007-10-11 21:42 ` Milton Miller 1 sibling, 0 replies; 10+ messages in thread From: Milton Miller @ 2007-10-11 21:42 UTC (permalink / raw) Cc: Olof Johansson, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2007-10-11 21:43 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20071008180406.052382073@mini.kroah.org>
[not found] ` <20071008180633.GJ7627@kroah.com>
[not found] ` <20071009151702.GA19209@lixom.net>
[not found] ` <20071009222003.GA21228@kroah.com>
[not found] ` <alpine.LFD.0.9999.0710100120340.7766@localhost.localdomain>
[not found] ` <alpine.LFD.0.999.0710091625520.3838@woody.linux-foundation.org>
2007-10-10 0:03 ` [stable] [patch 09/12] Fix SMP poweroff hangs Olof Johansson
2007-10-10 0:08 ` [PATCH] powerpc: don't enable cpu hotplug on mpic-based pseries Olof Johansson
2007-10-10 0:18 ` Stephen Rothwell
2007-10-10 0:38 ` [PATCH v2] " Olof Johansson
2007-10-10 10:08 ` Milton Miller
2007-10-10 16:43 ` Olof Johansson
2007-10-11 15:25 ` Milton Miller
2007-10-11 5:52 ` Paul Mackerras
2007-10-11 5:59 ` Olof Johansson
2007-10-11 21:42 ` Milton Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox