2006-05-15 14:08:29

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH] x86 NUMA panic compile error

Is this check now needed given we have the zone alignment patches in
this -mm also? I think we want to make it at least possible to
boot such a kernel on a 'flat' machine.

-apw

=== 8< ===
x86 NUMA panic compile error

Seem we have a syntax error rising from the the new panic added
to let people know NUMA didn't work on physically non-numa hardware.

Signed-off-by: Andy Whitcroft <[email protected]>
---
srat.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -upN reference/arch/i386/kernel/srat.c current/arch/i386/kernel/srat.c
--- reference/arch/i386/kernel/srat.c
+++ current/arch/i386/kernel/srat.c
@@ -269,7 +269,7 @@ int __init get_memcfg_from_srat(void)
extern int use_cyclone;
if (use_cyclone == 0) {
/* Make sure user sees something */
- static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else."
+ static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else.";
early_printk(s);
panic(s);
}


2006-05-15 17:53:21

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


* Andy Whitcroft <[email protected]> wrote:

> if (use_cyclone == 0) {
> /* Make sure user sees something */
> - static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else."
> + static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else.";
> early_printk(s);
> panic(s);
> }

i still strongly oppose the original Andi hack... numerous reasons were
given not to apply it (it's nice to simulate/trigger rarer features on
mainstream hardware too, and this ability to boot NUMA on my flat x86
testbox found at least one other NUMA bug already). Furthermore, the
crash i reported was fixed by the NUMA patchset. Andrew, please drop:

x86_64-mm-i386-numa-summit-check.patch

(which has nothing to do with x86_64 anyway)

Ingo

2006-05-15 18:01:44

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

On Monday 15 May 2006 19:53, Ingo Molnar wrote:
>
> * Andy Whitcroft <[email protected]> wrote:
>
> > if (use_cyclone == 0) {
> > /* Make sure user sees something */
> > - static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else."
> > + static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else.";
> > early_printk(s);
> > panic(s);
> > }
>
> i still strongly oppose the original Andi hack... numerous reasons were
> given not to apply it (it's nice to simulate/trigger rarer features on
> mainstream hardware too, and this ability to boot NUMA on my flat x86
> testbox found at least one other NUMA bug already). Furthermore, the
> crash i reported was fixed by the NUMA patchset. Andrew, please drop:

The problem is that it's not regularly used on a wide range
of boxes so it will eventually break again. We had this cycle several
times already.

It's also missing a lot of the workarounds for broken SRATs that
are needed for many of the existing NUMA systems.

If there's consensus i386 NUMA is useful I can drop it, but I predict
it will just eventually break again.

> x86_64-mm-i386-numa-summit-check.patch
>
> (which has nothing to do with x86_64 anyway)

I have a lot of i386 or combined i386/x86-64 patches in my tree - just Andrew's
merge script doesn't pick that up.

-Andi

2006-05-15 18:05:48

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

Ingo Molnar <[email protected]> wrote:
>
>
> * Andy Whitcroft <[email protected]> wrote:
>
> > if (use_cyclone == 0) {
> > /* Make sure user sees something */
> > - static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else."
> > + static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else.";
> > early_printk(s);
> > panic(s);
> > }
>
> i still strongly oppose the original Andi hack... numerous reasons were
> given not to apply it (it's nice to simulate/trigger rarer features on
> mainstream hardware too, and this ability to boot NUMA on my flat x86
> testbox found at least one other NUMA bug already). Furthermore, the
> crash i reported was fixed by the NUMA patchset.

I'll be darned. I never knew it was even possible to run x86 numa kernels
on non-numa boxen. I'd have tested about 1000000 of Christoph Lameter's
patches if someone had told me. Yes, it's useful.

> Andrew, please drop:
>
> x86_64-mm-i386-numa-summit-check.patch

bang.

> (which has nothing to do with x86_64 anyway)

True.

I guess the concern here is that we don't want people building these
frankenkernels and then sending us bug reports against them.

So it is perhaps reasonable to do this panic, but only if !CONFIG_EMBEDDED?
(It really is time to start renaming CONFIG_EMBEDDED to CONFIG_DONT_DO_THIS
or something).

2006-05-15 18:09:30

by Andy Whitcroft

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

Andrew Morton wrote:
> Ingo Molnar <[email protected]> wrote:
>
>>
>>* Andy Whitcroft <[email protected]> wrote:
>>
>>
>>> if (use_cyclone == 0) {
>>> /* Make sure user sees something */
>>>- static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else."
>>>+ static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else.";
>>> early_printk(s);
>>> panic(s);
>>> }
>>
>>i still strongly oppose the original Andi hack... numerous reasons were
>>given not to apply it (it's nice to simulate/trigger rarer features on
>>mainstream hardware too, and this ability to boot NUMA on my flat x86
>>testbox found at least one other NUMA bug already). Furthermore, the
>>crash i reported was fixed by the NUMA patchset.
>
>
> I'll be darned. I never knew it was even possible to run x86 numa kernels
> on non-numa boxen. I'd have tested about 1000000 of Christoph Lameter's
> patches if someone had told me. Yes, it's useful.
>

We always assumed it might be reasonable for a distro to want a single
installer kernel for all machines. So having a combined numa not numa
capable kernel always seemed like a good idea.

>>Andrew, please drop:
>>
>> x86_64-mm-i386-numa-summit-check.patch
>
>
> bang.
>
>
>>(which has nothing to do with x86_64 anyway)
>
>
> True.
>
> I guess the concern here is that we don't want people building these
> frankenkernels and then sending us bug reports against them.
>
> So it is perhaps reasonable to do this panic, but only if !CONFIG_EMBEDDED?
> (It really is time to start renaming CONFIG_EMBEDDED to CONFIG_DONT_DO_THIS
> or something).

How about CONFIG_EXPERIMENTAL?

-apw

2006-05-15 18:14:23

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


* Andi Kleen <[email protected]> wrote:

> > i still strongly oppose the original Andi hack... numerous reasons were
> > given not to apply it (it's nice to simulate/trigger rarer features on
> > mainstream hardware too, and this ability to boot NUMA on my flat x86
> > testbox found at least one other NUMA bug already). Furthermore, the
> > crash i reported was fixed by the NUMA patchset. Andrew, please drop:
>
> The problem is that it's not regularly used on a wide range of boxes
> so it will eventually break again. We had this cycle several times
> already.

so it will find new bugs. Since you wrote that patch the ability to try
numa on 'flat' x86 hardware has found two bugs, one of which (the zone
alignment thing) could very well affect non-obsolete hardware too.

Andi, triggering bugs as widely as possible is the _whole point_ of QA
and diversity of testing! Do you disable the compilation of the floppy
driver just because it's obsolete hardware? Will you remove NUMA
emulation from x86_64 just because it regularly broke in the past?

Fact is, the more hardware a given feature can be tried on, the better
tested that feature becomes. I find it quite extreme that this point has
to even be argued...

Ingo

2006-05-15 18:14:00

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

On Monday 15 May 2006 20:08, Andrew Morton wrote:
> Ingo Molnar <[email protected]> wrote:
> >
> >
> > * Andy Whitcroft <[email protected]> wrote:
> >
> > > if (use_cyclone == 0) {
> > > /* Make sure user sees something */
> > > - static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else."
> > > + static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else.";
> > > early_printk(s);
> > > panic(s);
> > > }
> >
> > i still strongly oppose the original Andi hack... numerous reasons were
> > given not to apply it (it's nice to simulate/trigger rarer features on
> > mainstream hardware too, and this ability to boot NUMA on my flat x86
> > testbox found at least one other NUMA bug already). Furthermore, the
> > crash i reported was fixed by the NUMA patchset.
>
> I'll be darned. I never knew it was even possible to run x86 numa kernels
> on non-numa boxen. I'd have tested about 1000000 of Christoph Lameter's
> patches if someone had told me. Yes, it's useful.

If you want to use it for that I would suggest to port the numa emulation
code at least - two or four nodes tends to find more problems than a single
node.

But testing on a 64bit box - even with numa emulation - would be much
better because on 32bit ZONE_NORMAL often is node 0 only and you won't
get much numaness for kernel objects.

>
> > Andrew, please drop:
> >
> > x86_64-mm-i386-numa-summit-check.patch
>
> bang.

Ok.

>
> > (which has nothing to do with x86_64 anyway)
>
> True.
>
> I guess the concern here is that we don't want people building these
> frankenkernels and then sending us bug reports against them.

Well it will still increase the bug numbers you care so much about.

Another reason I don't like it is that it's ugly and reimplements
parts of ACPI on its own for no reason.

If people use it regularly for debugging maybe it won't bit rot
as quickly as it used to be, but there is a big difference between
theory and practice so we'll see.


> So it is perhaps reasonable to do this panic, but only if !CONFIG_EMBEDDED?
> (It really is time to start renaming CONFIG_EMBEDDED to CONFIG_DONT_DO_THIS
> or something).

Fine.

-Andi

2006-05-15 18:22:30

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

Andy Whitcroft <[email protected]> wrote:
>
> > So it is perhaps reasonable to do this panic, but only if !CONFIG_EMBEDDED?
> > (It really is time to start renaming CONFIG_EMBEDDED to CONFIG_DONT_DO_THIS
> > or something).
>
> How about CONFIG_EXPERIMENTAL?

Probably CONFIG_ADVANCED would be closer.

2006-05-15 18:24:37

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

On Monday 15 May 2006 20:24, Andrew Morton wrote:
> Andy Whitcroft <[email protected]> wrote:
> >
> > > So it is perhaps reasonable to do this panic, but only if !CONFIG_EMBEDDED?
> > > (It really is time to start renaming CONFIG_EMBEDDED to CONFIG_DONT_DO_THIS
> > > or something).
> >
> > How about CONFIG_EXPERIMENTAL?
>
> Probably CONFIG_ADVANCED would be closer.

Most people who recompile kernels probably think of themselves as advanced.
This doesn't mean they will get these things right.

-Andi

2006-05-15 18:29:01

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


* Andrew Morton <[email protected]> wrote:

> > (which has nothing to do with x86_64 anyway)
>
> True.
>
> I guess the concern here is that we don't want people building these
> frankenkernels and then sending us bug reports against them.

sure - lets simply turn it into a printk, as per the patch below.

it's not like we are being swamped with these bugreports, it seems i was
the only one who tried. So lets not over-react it. (and the panic was
the worst possible thing we could do.)

Ingo

---

warn users that running CONFIG_NUMA on non-x440 boxes is barely tested.

Signed-off-by: Ingo Molnar <[email protected]>

arch/i386/kernel/srat.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

Index: linux/arch/i386/kernel/srat.c
===================================================================
--- linux.orig/arch/i386/kernel/srat.c
+++ linux/arch/i386/kernel/srat.c
@@ -267,12 +267,9 @@ int __init get_memcfg_from_srat(void)
int i = 0;

extern int use_cyclone;
- if (use_cyclone == 0) {
- /* Make sure user sees something */
- static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else."
- early_printk(s);
- panic(s);
- }
+ /* Make sure user sees something */
+ if (use_cyclone == 0)
+ printk(KERN_WARN "WARNING: Not an IBM x440/NUMAQ and CONFIG_NUMA enabled!\n");

if (ACPI_FAILURE(acpi_find_root_pointer(ACPI_PHYSICAL_ADDRESSING,
rsdp_address))) {

2006-05-15 18:31:28

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


* Andi Kleen <[email protected]> wrote:

> > > (which has nothing to do with x86_64 anyway)
> >
> > True.
> >
> > I guess the concern here is that we don't want people building these
> > frankenkernels and then sending us bug reports against them.
>
> Well it will still increase the bug numbers you care so much about.

so lets ... hide them? ahem? Unaligned NUMA zones were a bug waiting to
hit us on some other platform.

Ingo

2006-05-15 18:32:17

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

Andi Kleen <[email protected]> wrote:
>
> >
> > I'll be darned. I never knew it was even possible to run x86 numa kernels
> > on non-numa boxen. I'd have tested about 1000000 of Christoph Lameter's
> > patches if someone had told me. Yes, it's useful.
>
> If you want to use it for that I would suggest to port the numa emulation
> code at least - two or four nodes tends to find more problems than a single
> node.
>
> But testing on a 64bit box - even with numa emulation - would be much
> better because on 32bit ZONE_NORMAL often is node 0 only and you won't
> get much numaness for kernel objects.

That's an excellent point - most developers who are likely to want to test
NUMA have x86_64 boxes and x86_64 has NUMA-emulation-on-SMP. I'd
semi-forgotten that it existed.

This rather weakens the reasons for retaining support for
NUMA-on-non-summit-x86. Ingo?

> > I guess the concern here is that we don't want people building these
> > frankenkernels and then sending us bug reports against them.
>
> Well it will still increase the bug numbers you care so much about.

Not really. If a bug affects something we don't care about (like this)
I'll just ignore it. I care about the number of busted machines out there,
not the bug counts...

> Another reason I don't like it is that it's ugly and reimplements
> parts of ACPI on its own for no reason.

So shouldn't such a patch remove that code rather than panicing?

2006-05-15 18:38:05

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


>
> > Another reason I don't like it is that it's ugly and reimplements
> > parts of ACPI on its own for no reason.
>
> So shouldn't such a patch remove that code rather than panicing?

I would be for remove, but apparently we have one or two users in
IBM that run their x440s (32bit only) with CONFIG_NUMA. No distributions
do so though and I would expect x440s to usually run distributions
because they are quite expensive machines.

My arguments for remove:
- The code is very hackish - it was written before the proper ACPI
infrastructure is in place - and NUMA on 32bit in general needs a lot
of hacks because of the limited ZONE_NORMAL.
- NUMA on 32bit is kind of broken by design.
- It isn't used much.
- It breaks often
- It tends to not work on Opterons and hits the users who try it there.

Short of remove I would settle on the panic on non Summit.

Also if you only wanted NUMA emulation for testing it could be also
done much much simpler removing near all of i386/*/srat.c. But with
the inherent 32bit NUMA limitations I have my doubts on its great
usefulness.

-Andi

2006-05-15 18:43:31

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


* Andrew Morton <[email protected]> wrote:

> > But testing on a 64bit box - even with numa emulation - would be much
> > better because on 32bit ZONE_NORMAL often is node 0 only and you won't
> > get much numaness for kernel objects.
>
> That's an excellent point - most developers who are likely to want to
> test NUMA have x86_64 boxes and x86_64 has NUMA-emulation-on-SMP. I'd
> semi-forgotten that it existed.
>
> This rather weakens the reasons for retaining support for
> NUMA-on-non-summit-x86. Ingo?

my 64-bit boxes (half of the testbed) are busy ones used for daily
testing that i just cannot keep running for days doing stress-tests.
Neither can they wait 10 minutes to boot up an allyesconfig kernel. So
at least as long as my testconfig is concerned, 32-bit boxes and i386
NUMA still has some place.

(and i'm not at all arguing it's a big thing - it's a minor thing. But i
absolutely resist Andi's approach on conceptual grounds. It's backwards,
for the reasons i outlined before. Had Andi's patch been in place the
zone alignment bug had probably not been found - simple as that.
Reducing choice artificially is the kind of thing that decreases the
kernel's quality. Improving the quality of the kernel starts with making
sure everyone understands how to achieve it - and Andi is one of the
largest and most important contributors so i'd really like to make sure
he understands my point :-)

Ingo

2006-05-15 18:49:40

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

* Andi Kleen <[email protected]> wrote:

> > So shouldn't such a patch remove that code rather than panicing?
>
> I would be for remove, but apparently we have one or two users in IBM
> that run their x440s (32bit only) with CONFIG_NUMA. No distributions
> do so though and I would expect x440s to usually run distributions
> because they are quite expensive machines.
>
> My arguments for remove:
> - The code is very hackish - it was written before the proper ACPI
> infrastructure is in place - and NUMA on 32bit in general needs a lot
> of hacks because of the limited ZONE_NORMAL.

works fine here now. The whole NUMA code is still quite hackish in
general, (including most of arch/x86_64/*/*.c), so i'd not judge based
on that.

> - NUMA on 32bit is kind of broken by design.

well. 32bit itself is broken by design, if you consider RAM larger than
say 1GB.

> - It isn't used much.

it's an enabler of a feature-set that i couldnt test on these boxes
otherwise. Look at it like the highmem= boot option. Or consider it a
primitive form of NUMA emulation.

> - It breaks often

Martin says he's daily testing it in his grid.

> - It tends to not work on Opterons and hits the users who try it
> there.

maybe due to the zone alignment problem?

Ingo

2006-05-15 18:49:57

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

Ingo Molnar <[email protected]> wrote:
>
>
> * Andrew Morton <[email protected]> wrote:
>
> > > (which has nothing to do with x86_64 anyway)
> >
> > True.
> >
> > I guess the concern here is that we don't want people building these
> > frankenkernels and then sending us bug reports against them.
>
> sure - lets simply turn it into a printk, as per the patch below.
>
> it's not like we are being swamped with these bugreports, it seems i was
> the only one who tried. So lets not over-react it. (and the panic was
> the worst possible thing we could do.)
>
> Ingo
>
> ---
>
> warn users that running CONFIG_NUMA on non-x440 boxes is barely tested.
>
> Signed-off-by: Ingo Molnar <[email protected]>
>
> arch/i386/kernel/srat.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> Index: linux/arch/i386/kernel/srat.c
> ===================================================================
> --- linux.orig/arch/i386/kernel/srat.c
> +++ linux/arch/i386/kernel/srat.c
> @@ -267,12 +267,9 @@ int __init get_memcfg_from_srat(void)
> int i = 0;
>
> extern int use_cyclone;

argh.

If we didn't do this lazy-ass put-the-declaration-in-the-C-file thing, we'd
have noticed that the declaration of use_cyclone is in
include/asm-i386/mach-summit/mach_mpparse.h.

use_cyclone isn't defined if !CONFIG_X86_CYCLONE_TIMER.
arch/i386/kernel/srat.c is only compiled if X86_SUMMIT || X86_GENERICARCH.

<tries to break it>

I have a config here which has NUMA=y, ACPI_SRAT=y, X86_SUMMIT=n (and
X86_SUMMIT_NUMA=y!!) and, for some reason it set X86_CYCLONE_TIMER=y, which
is dumb. But it will manage to link with this patch applied.

But still, referencing a variable which is implemented in
arch/i386/kernel/timers/timer_cyclone.c from within arch/i386/kernel/srat.c
is asking for trouble, no?

2006-05-15 18:56:36

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


> But still, referencing a variable which is implemented in
> arch/i386/kernel/timers/timer_cyclone.c from within arch/i386/kernel/srat.c
> is asking for trouble, no?

Probably, but where would you define it instead? It kind of belongs
into timer_cyclone.c

-Andi

2006-05-15 18:57:07

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


* Andrew Morton <[email protected]> wrote:

> If we didn't do this lazy-ass put-the-declaration-in-the-C-file thing,
> we'd have noticed that the declaration of use_cyclone is in
> include/asm-i386/mach-summit/mach_mpparse.h.

updated patch below. Or lets drop the original patch that adds the
panic?

Ingo

---

Signed-off-by: Ingo Molnar <[email protected]>

arch/i386/kernel/srat.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

Index: linux/arch/i386/kernel/srat.c
===================================================================
--- linux.orig/arch/i386/kernel/srat.c
+++ linux/arch/i386/kernel/srat.c
@@ -266,13 +266,12 @@ int __init get_memcfg_from_srat(void)
int tables = 0;
int i = 0;

+#ifdef CONFIG_X86_CYCLONE_TIMER
extern int use_cyclone;
- if (use_cyclone == 0) {
- /* Make sure user sees something */
- static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else."
- early_printk(s);
- panic(s);
- }
+ /* Make sure user sees something */
+ if (use_cyclone == 0)
+#endif
+ printk(KERN_WARN "WARNING: Not an IBM x440/NUMAQ and CONFIG_NUMA enabled!\n");

if (ACPI_FAILURE(acpi_find_root_pointer(ACPI_PHYSICAL_ADDRESSING,
rsdp_address))) {

2006-05-15 19:00:29

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


* Andi Kleen <[email protected]> wrote:

>
> > But still, referencing a variable which is implemented in
> > arch/i386/kernel/timers/timer_cyclone.c from within arch/i386/kernel/srat.c
> > is asking for trouble, no?
>
> Probably, but where would you define it instead? It kind of belongs
> into timer_cyclone.c

there should be a asm-i386/cyclone.h that properly defines this. (and to
make things more consistent, it could make it 0 even if
!CONFIG_X86_CYCLONE_TIMER)

Ingo

2006-05-15 19:06:53

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


* Ingo Molnar <[email protected]> wrote:

> updated patch below. Or lets drop the original patch that adds the
> panic?

another update: s/KERN_WARN/KERN_WARNING ...

Ingo

---

re-enable dummy NUMA on i386.

Signed-off-by: Ingo Molnar <[email protected]>

arch/i386/kernel/srat.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

Index: linux/arch/i386/kernel/srat.c
===================================================================
--- linux.orig/arch/i386/kernel/srat.c
+++ linux/arch/i386/kernel/srat.c
@@ -266,13 +266,12 @@ int __init get_memcfg_from_srat(void)
int tables = 0;
int i = 0;

+#ifdef CONFIG_X86_CYCLONE_TIMER
extern int use_cyclone;
- if (use_cyclone == 0) {
- /* Make sure user sees something */
- static const char s[] __initdata = "Not an IBM x440/NUMAQ. Don't use i386 CONFIG_NUMA anywhere else."
- early_printk(s);
- panic(s);
- }
+ /* Make sure user sees something */
+ if (use_cyclone == 0)
+#endif
+ printk(KERN_WARNING "WARNING: Not an IBM x440/NUMAQ and CONFIG_NUMA enabled!\n");

if (ACPI_FAILURE(acpi_find_root_pointer(ACPI_PHYSICAL_ADDRESSING,
rsdp_address))) {

2006-05-15 19:11:39

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


> I think you are right: there are very few end users running with
> CONFIG_NUMA on normal x86. But, there is a disproportionately large
> number of developers who do it. There are quite a few IBM (and maybe
> more via OSDL) developers who's only access to real NUMA hardware is x86
> NUMAQs and Summit machines. When somebody says "foo is broken on NUMA",
> I go right to an x86 box.
> Anyway, I'd like to think that we've contributed enough to the generic
> NUMA code to have earned our keep and allow our little x86 NUMA "hacks"
> to remain.

Yes that is why i did the "only work on Summit" patch as compromise.
With that you can have your hacks, but it won't impact anybody else.

> x86 is a legacy architecture now anyway, right? ;)
I wish everybody would agree on that @)

-Andi

2006-05-15 19:26:37

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


* Andi Kleen <[email protected]> wrote:

> > x86 is a legacy architecture now anyway, right? ;)
> I wish everybody would agree on that @)

as far as i'm concerned x86 is obsolete: my main devel and testboxes are
64-bit throughout, and i develop for 64-bit by default.

Nevertheless for hard-to-debug bugs i prefer if they can be reproduced
and debugged on 32-bit too, because x86_64 debugging is still quite a
PITA and wastes alot of time: for example it has no support for exact
kernel stacktraces. Also, the printout of the backtrace is butt-ugly and
as un-ergonomic to the human eye as it gets - who came up with that
"two-maybe-one function entries per-line" nonsense? [Whoever did it he
never had to look at (and make sense of) hundreds of stacktraces in a
row.]

Also, the majority of kernel bugs still get reported on 32-bit and most
of the testers are on 32-bit. So x86_64 is nice but it still needs some
work, mainly in terms of debuggability.

Ingo

2006-05-15 19:37:09

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

Ingo Molnar <[email protected]> wrote:
>
> Nevertheless for hard-to-debug bugs i prefer if they can be reproduced
> and debugged on 32-bit too, because x86_64 debugging is still quite a
> PITA and wastes alot of time: for example it has no support for exact
> kernel stacktraces. Also, the printout of the backtrace is butt-ugly and
> as un-ergonomic to the human eye as it gets

Yes, I find x86_64 traces significantly harder to follow. And I miss the
display of the length of the functions (do_md_run+1208 instead of
do_md_run+1208/2043). The latter form makes it easier to work out
whereabouts in the function things happened.

That, plus the mix of hex and decimal numbers..

> who came up with that
> "two-maybe-one function entries per-line" nonsense? [Whoever did it he
> never had to look at (and make sense of) hundreds of stacktraces in a
> row.]

Plus they're wide enough to get usefully wordwrapped when someone mails
them to you.

2006-05-15 19:39:33

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


> Nevertheless for hard-to-debug bugs i prefer if they can be reproduced
> and debugged on 32-bit too, because x86_64 debugging is still quite a
> PITA and wastes alot of time: for example it has no support for exact
> kernel stacktraces.

Hopefully soon.

I think i386 only gained it very recently, so it can't be _that_ big
a problem.

The real issue is too deeply nested code like the callback hell
we have in some subsystems. Better would be to eliminate that. 2.4
was much nicer in this regard and there has been quite a lot of
unnecessary complications in this area when the kernel went to 2.6.

> Also, the printout of the backtrace is butt-ugly and
> as un-ergonomic to the human eye as it gets - who came up with that
> "two-maybe-one function entries per-line" nonsense? [Whoever did it he
> never had to look at (and make sense of) hundreds of stacktraces in a
> row.]

The original goal was to make it fit as much as possible on
the screen when you don't have a serial/net/fireconsole.
But arguably it's less and less useful because the kernel
has gotten so huge that most backtraces are very long and scroll
away anyways.

-Andi

2006-05-15 19:45:19

by Martin Bligh

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

Andrew Morton wrote:
> Andy Whitcroft <[email protected]> wrote:
>
>>>So it is perhaps reasonable to do this panic, but only if !CONFIG_EMBEDDED?
>>>(It really is time to start renaming CONFIG_EMBEDDED to CONFIG_DONT_DO_THIS
>>>or something).
>>
>>How about CONFIG_EXPERIMENTAL?
>
>
> Probably CONFIG_ADVANCED would be closer.

It defaults to off already - people have to explicitly enable it.

Plus the original point was to be able to build one kernel that'd work
across NUMA and non-NUMA boxes.

M.

2006-05-15 19:47:08

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


[... feels the love ...]

On Monday 15 May 2006 21:39, Andrew Morton wrote:
> Ingo Molnar <[email protected]> wrote:
> >
> > Nevertheless for hard-to-debug bugs i prefer if they can be reproduced
> > and debugged on 32-bit too, because x86_64 debugging is still quite a
> > PITA and wastes alot of time: for example it has no support for exact
> > kernel stacktraces. Also, the printout of the backtrace is butt-ugly and
> > as un-ergonomic to the human eye as it gets
>
> Yes, I find x86_64 traces significantly harder to follow. And I miss the
> display of the length of the functions (do_md_run+1208 instead of
> do_md_run+1208/2043). The latter form makes it easier to work out
> whereabouts in the function things happened.
>
> That, plus the mix of hex and decimal numbers..
>
> > who came up with that
> > "two-maybe-one function entries per-line" nonsense? [Whoever did it he
> > never had to look at (and make sense of) hundreds of stacktraces in a
> > row.]
>
> Plus they're wide enough to get usefully wordwrapped when someone mails
> them to you.

Hmm, I didn't realize they were _that_ unpopular. If you got the i386
like space wasting backtraces would you guys all switch your development machines
to x86-64 ? @)

-Andi

2006-05-15 19:57:03

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

Andi Kleen <[email protected]> wrote:
>
>
> [... feels the love ...]

heh.

> On Monday 15 May 2006 21:39, Andrew Morton wrote:
> > Ingo Molnar <[email protected]> wrote:
> > >
> > > Nevertheless for hard-to-debug bugs i prefer if they can be reproduced
> > > and debugged on 32-bit too, because x86_64 debugging is still quite a
> > > PITA and wastes alot of time: for example it has no support for exact
> > > kernel stacktraces. Also, the printout of the backtrace is butt-ugly and
> > > as un-ergonomic to the human eye as it gets
> >
> > Yes, I find x86_64 traces significantly harder to follow. And I miss the
> > display of the length of the functions (do_md_run+1208 instead of
> > do_md_run+1208/2043). The latter form makes it easier to work out
> > whereabouts in the function things happened.
> >
> > That, plus the mix of hex and decimal numbers..
> >
> > > who came up with that
> > > "two-maybe-one function entries per-line" nonsense? [Whoever did it he
> > > never had to look at (and make sense of) hundreds of stacktraces in a
> > > row.]
> >
> > Plus they're wide enough to get usefully wordwrapped when someone mails
> > them to you.
>
> Hmm, I didn't realize they were _that_ unpopular. If you got the i386
> like space wasting backtraces would you guys all switch your development machines
> to x86-64 ? @)
>

Developers use serial consoles for such things. (I discovered
`console=uart,...' yesterday. It works nicely as an earlyprintk on ia64..)

It's reports-from-the-field which are the problem.

A lot of these problems can be address by simple cranking up the VGA screen
resolution, but I discovered that I don't know how to do that - I've always
used `vga=extended', but that doesn't work on an EFI-booted ia64 box.

Does anyone know what the magic option is to make the vga console use 50
rows?

2006-05-15 20:02:42

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

On Monday 15 May 2006 21:59, Andrew Morton wrote:

> > On Monday 15 May 2006 21:39, Andrew Morton wrote:
> > > Ingo Molnar <[email protected]> wrote:
> > > >
> > > > Nevertheless for hard-to-debug bugs i prefer if they can be reproduced
> > > > and debugged on 32-bit too, because x86_64 debugging is still quite a
> > > > PITA and wastes alot of time: for example it has no support for exact
> > > > kernel stacktraces. Also, the printout of the backtrace is butt-ugly and
> > > > as un-ergonomic to the human eye as it gets
> > >
> > > Yes, I find x86_64 traces significantly harder to follow. And I miss the
> > > display of the length of the functions (do_md_run+1208 instead of
> > > do_md_run+1208/2043). The latter form makes it easier to work out
> > > whereabouts in the function things happened.
> > >
> > > That, plus the mix of hex and decimal numbers..
> > >
> > > > who came up with that
> > > > "two-maybe-one function entries per-line" nonsense? [Whoever did it he
> > > > never had to look at (and make sense of) hundreds of stacktraces in a
> > > > row.]
> > >
> > > Plus they're wide enough to get usefully wordwrapped when someone mails
> > > them to you.
> >
> > Hmm, I didn't realize they were _that_ unpopular. If you got the i386
> > like space wasting backtraces would you guys all switch your development machines
> > to x86-64 ? @)
> >
>
> Developers use serial consoles for such things. (I discovered
> `console=uart,...' yesterday. It works nicely as an earlyprintk on ia64..)

I can also recommend firescope + firewire cards. It's not early
yet, but I hope eventually. But it can work without the target still
being alive and also does on most laptops.

> It's reports-from-the-field which are the problem.

In my experience the biggest problem in the field is that most
of it scrolls away. That is why I tweaked the x86-64 format to be as space
efficient as possible. That's also why the "executive summary" was added.

But Ingo has a point that it usually doesn't help anyways because backtraces
tend to be so overlong now after the code got through 20 callbacks before
it can do something actually useful.


> A lot of these problems can be address by simple cranking up the VGA screen
> resolution, but I discovered that I don't know how to do that - I've always
> used `vga=extended', but that doesn't work on an EFI-booted ia64 box.
>
> Does anyone know what the magic option is to make the vga console use 50
> rows?

I use vga=0x0f07

It's a butt ugly font, but it's the smallest I could find without using
the slow fbcon.

If you can't remember the hex number use vga=ask

-Andi

2006-05-15 20:46:59

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

On Mon, 2006-05-15 at 20:37 +0200, Andi Kleen wrote:
> My arguments for remove:
> - The code is very hackish - it was written before the proper ACPI
> infrastructure is in place - and NUMA on 32bit in general needs a lot
> of hacks because of the limited ZONE_NORMAL.
> - NUMA on 32bit is kind of broken by design.
> - It isn't used much.
> - It breaks often
> - It tends to not work on Opterons and hits the users who try it
> there.

I think you are right: there are very few end users running with
CONFIG_NUMA on normal x86. But, there is a disproportionately large
number of developers who do it. There are quite a few IBM (and maybe
more via OSDL) developers who's only access to real NUMA hardware is x86
NUMAQs and Summit machines. When somebody says "foo is broken on NUMA",
I go right to an x86 box.

Anyway, I'd like to think that we've contributed enough to the generic
NUMA code to have earned our keep and allow our little x86 NUMA "hacks"
to remain. x86 is a legacy architecture now anyway, right? ;)

-- Dave

2006-05-15 23:12:51

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

Followup to: <[email protected]>
By author: Andi Kleen <[email protected]>
In newsgroup: linux.dev.kernel
>
> > x86 is a legacy architecture now anyway, right? ;)
> I wish everybody would agree on that @)
>

It's going to live on for a very long time, though. Intel is still
shipping some very fast 64-bit-deficient silicon. Once that's gone,
it's going to live on for decades in the embedded world.

-hpa

2006-05-16 07:06:30

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


* Andi Kleen <[email protected]> wrote:

> > Nevertheless for hard-to-debug bugs i prefer if they can be reproduced
> > and debugged on 32-bit too, because x86_64 debugging is still quite a
> > PITA and wastes alot of time: for example it has no support for exact
> > kernel stacktraces.
>
> Hopefully soon.

i've already implemented it for FRAME_POINTERS (i really needed it to
not go insane when looking at lock validator output).

As you suggested a few weeks ago the real solution would be a dwarf
parser. Maybe ia64's could be taken? Will post the patch for the
FRAME_POINTERS solution soon. Sample output:

[<ffffffff8020af4c>] __show_trace+0x3a/0x66
[<ffffffff8020b36b>] show_trace+0x17/0x1a
[<ffffffff80207e36>] show_regs+0x36/0x3c
[<ffffffff80207e75>] smp_show_regs+0x39/0x52
[<ffffffff8021559e>] smp_nmi_callback+0x6a/0x85
[<ffffffff802163f2>] do_nmi+0x69/0x91
[<ffffffff80601dca>] nmi+0x7e/0x85
[<ffffffffffffffff>] 0xffffffffffffffff
[<ffffffff80601ad0>] _spin_unlock_irqrestore+0x3e/0x47
[<ffffffff8024424c>] prepare_to_wait+0x63/0x6d
[<ffffffff8022ff0c>] do_syslog+0xf1/0x3ca
[<ffffffff802ba3ed>] kmsg_read+0x3a/0x46
[<ffffffff8027db72>] vfs_read+0xe6/0x191
[<ffffffff8027e79d>] sys_read+0x44/0x82
[<ffffffff80209b11>] system_call+0x7d/0x83
[<ffffffffffffffff>] 0xffffffffffffffff

(and it works fine across irq/exception stacks too.)

> I think i386 only gained it very recently, so it can't be _that_ big a
> problem.

i certainly used exact backtraces on i386 for many many years. Not sure
whether those patches were all upstream though. It's also the
combination of effects that makes the difference between i386 and x86_64
so striking.

furthermore, the kernel's debugging infrastructure improved
significantly, and we get more and more stackdumps to interpret [instead
of hard to debug corruptions, etc.].

> The real issue is too deeply nested code like the callback hell we
> have in some subsystems. Better would be to eliminate that. 2.4 was
> much nicer in this regard and there has been quite a lot of
> unnecessary complications in this area when the kernel went to 2.6.

i have no problems with interpreting occasional non-exact backtraces,
but it is certainly non-obvious, and when you are looking at backtraces
en masse, the unnecessary repetitive task can get really distracting and
frustrating.

Exact backtraces on the other hand almost immediately create a unique
and reliable "visual fingerprint", and if you have looked at enough of
them, you almost recognize them just from looking at the shape of them.
It's a completely different 'experience'. (and userspace developers will
laugh out loud at us now i suspect ...)

> > Also, the printout of the backtrace is butt-ugly and as un-ergonomic
> > to the human eye as it gets - who came up with that "two-maybe-one
> > function entries per-line" nonsense? [Whoever did it he never had to
> > look at (and make sense of) hundreds of stacktraces in a row.]
>
> The original goal was to make it fit as much as possible on the screen
> when you don't have a serial/net/fireconsole. But arguably it's less
> and less useful because the kernel has gotten so huge that most
> backtraces are very long and scroll away anyways.

yeah.

Ingo

2006-05-16 09:22:14

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error


> As you suggested a few weeks ago the real solution would be a dwarf
> parser. Maybe ia64's could be taken?

Ia64's would be a lot of work.

> (and it works fine across irq/exception stacks too.)

It didn't work at all through the old locks/semaphore stubs.

> > I think i386 only gained it very recently, so it can't be _that_ big a
> > problem.
>
> i certainly used exact backtraces on i386 for many many years.

The patch into mainline went in in mid 2004.

http://www.kernel.org/git/?p=linux/kernel/git/tglx/history.git;a=commit;h=066479e379f387e5b1da0f1149fe0b97bac58888


-Andi

2006-05-17 00:39:31

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

On Mon, May 15, 2006 at 04:06:18PM -0700, H. Peter Anvin wrote:
> Followup to: <[email protected]>
> By author: Andi Kleen <[email protected]>
> In newsgroup: linux.dev.kernel
> >
> > > x86 is a legacy architecture now anyway, right? ;)
> > I wish everybody would agree on that @)
> >
>
> It's going to live on for a very long time, though. Intel is still
> shipping some very fast 64-bit-deficient silicon. Once that's gone,
> it's going to live on for decades in the embedded world.

There's also a surprising number of people still running
their shiny new x86-64's in 32 bit mode. (I suspect because
they're reliant upon applications that aren't 64-bit clean
that for whatever reason don't run in 32-bit emulation).

I'd be surprised if any of the major distros stopped shipping
a 32-bit x86 release for several years.

Dave

--
http://www.codemonkey.org.uk

2006-05-17 01:21:44

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] x86 NUMA panic compile error

Dave Jones wrote:
> On Mon, May 15, 2006 at 04:06:18PM -0700, H. Peter Anvin wrote:
> > Followup to: <[email protected]>
> > By author: Andi Kleen <[email protected]>
> > In newsgroup: linux.dev.kernel
> > >
> > > > x86 is a legacy architecture now anyway, right? ;)
> > > I wish everybody would agree on that @)
> > >
> >
> > It's going to live on for a very long time, though. Intel is still
> > shipping some very fast 64-bit-deficient silicon. Once that's gone,
> > it's going to live on for decades in the embedded world.
>
> There's also a surprising number of people still running
> their shiny new x86-64's in 32 bit mode. (I suspect because
> they're reliant upon applications that aren't 64-bit clean
> that for whatever reason don't run in 32-bit emulation).
>
> I'd be surprised if any of the major distros stopped shipping
> a 32-bit x86 release for several years.
>

Yup. In fact, I wish Fedora had the 32-bit Firefox, at least as an option. I keep having
to add the 32-bit repos for that reason alone :(

-hpa