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