[PATCH] x86: unify "numa=" command line option handling

[PATCH] x86: unify "numa=" command line option handling

[Jan Beulich]

In order to be able to suppress the use of SRAT tables that 32-bit
Linux can't deal with (possibly leading to a non-bootable system,
without disabling ACPI altogether), move the "numa=" option handling
to common code.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

---
 arch/x86/include/asm/numa_32.h |    2 ++
 arch/x86/include/asm/numa_64.h |    1 +
 arch/x86/mm/numa.c             |   22 ++++++++++++++++++++++
 arch/x86/mm/numa_64.c          |   24 +++++-------------------
 arch/x86/mm/srat_32.c          |    1 -
 5 files changed, 30 insertions(+), 20 deletions(-)

--- 2.6.37/arch/x86/include/asm/numa_32.h
+++ 2.6.37-x86-numa-option/arch/x86/include/asm/numa_32.h
@@ -1,6 +1,8 @@
 #ifndef _ASM_X86_NUMA_32_H
 #define _ASM_X86_NUMA_32_H
 
+extern int numa_off;
+
 extern int pxm_to_nid(int pxm);
 extern void numa_remove_cpu(int cpu);
 
--- 2.6.37/arch/x86/include/asm/numa_64.h
+++ 2.6.37-x86-numa-option/arch/x86/include/asm/numa_64.h
@@ -40,6 +40,7 @@ extern void __cpuinit numa_remove_cpu(in
 #ifdef CONFIG_NUMA_EMU
 #define FAKE_NODE_MIN_SIZE	((u64)64 << 20)
 #define FAKE_NODE_MIN_HASH_MASK	(~(FAKE_NODE_MIN_SIZE - 1UL))
+void numa_emu_cmdline(char *);
 #endif /* CONFIG_NUMA_EMU */
 #else
 static inline void init_cpu_to_node(void)		{ }
--- 2.6.37/arch/x86/mm/numa.c
+++ 2.6.37-x86-numa-option/arch/x86/mm/numa.c
@@ -2,6 +2,28 @@
 #include <linux/topology.h>
 #include <linux/module.h>
 #include <linux/bootmem.h>
+#include <asm/numa.h>
+#include <asm/acpi.h>
+
+int __initdata numa_off;
+
+static __init int numa_setup(char *opt)
+{
+	if (!opt)
+		return -EINVAL;
+	if (!strncmp(opt, "off", 3))
+		numa_off = 1;
+#ifdef CONFIG_NUMA_EMU
+	if (!strncmp(opt, "fake=", 5))
+		numa_emu_cmdline(opt + 5);
+#endif
+#ifdef CONFIG_ACPI_NUMA
+	if (!strncmp(opt, "noacpi", 6))
+		acpi_numa = -1;
+#endif
+	return 0;
+}
+early_param("numa", numa_setup);
 
 /*
  * Which logical CPUs are on which nodes
--- 2.6.37/arch/x86/mm/numa_64.c
+++ 2.6.37-x86-numa-option/arch/x86/mm/numa_64.c
@@ -30,7 +30,6 @@ s16 apicid_to_node[MAX_LOCAL_APIC] __cpu
 	[0 ... MAX_LOCAL_APIC-1] = NUMA_NO_NODE
 };
 
-int numa_off __initdata;
 static unsigned long __initdata nodemap_addr;
 static unsigned long __initdata nodemap_size;
 
@@ -263,6 +262,11 @@ static struct bootnode nodes[MAX_NUMNODE
 static struct bootnode physnodes[MAX_NUMNODES] __initdata;
 static char *cmdline __initdata;
 
+void __init numa_emu_cmdline(char *str)
+{
+	cmdline = str;
+}
+
 static int __init setup_physnodes(unsigned long start, unsigned long end,
 					int acpi, int k8)
 {
@@ -661,24 +665,6 @@ unsigned long __init numa_free_all_bootm
 	return pages;
 }
 
-static __init int numa_setup(char *opt)
-{
-	if (!opt)
-		return -EINVAL;
-	if (!strncmp(opt, "off", 3))
-		numa_off = 1;
-#ifdef CONFIG_NUMA_EMU
-	if (!strncmp(opt, "fake=", 5))
-		cmdline = opt + 5;
-#endif
-#ifdef CONFIG_ACPI_NUMA
-	if (!strncmp(opt, "noacpi", 6))
-		acpi_numa = -1;
-#endif
-	return 0;
-}
-early_param("numa", numa_setup);
-
 #ifdef CONFIG_NUMA
 
 static __init int find_near_online_node(int node)
--- 2.6.37/arch/x86/mm/srat_32.c
+++ 2.6.37-x86-numa-option/arch/x86/mm/srat_32.c
@@ -59,7 +59,6 @@ static struct node_memory_chunk_s __init
 static int __initdata num_memory_chunks; /* total number of memory chunks */
 static u8 __initdata apicid_to_pxm[MAX_APICID];
 
-int numa_off __initdata;
 int acpi_numa __initdata;
 
 static __init void bad_srat(void)

Re: [PATCH] x86: unify "numa=" command line option handling

[David Rientjes]

On Thu, 6 Jan 2011, Jan Beulich wrote:

> In order to be able to suppress the use of SRAT tables that 32-bit
> Linux can't deal with (possibly leading to a non-bootable system,
> without disabling ACPI altogether), move the "numa=" option handling
> to common code.
> 

I like the idea in general, but I think it may collide with Tejun's NUMA 
unification patchset for x86, which I assume will another revision once 
the merge window for .38 closes?  Or maybe he'd like to merge your patch 
with his series and push it as one?

Re: [PATCH] x86: unify "numa=" command line option handling

[Jan Beulich]

>>> On 06.01.11 at 21:36, David Rientjes <rientjes@google.com> wrote:
> On Thu, 6 Jan 2011, Jan Beulich wrote:
> 
>> In order to be able to suppress the use of SRAT tables that 32-bit
>> Linux can't deal with (possibly leading to a non-bootable system,
>> without disabling ACPI altogether), move the "numa=" option handling
>> to common code.
>> 
> 
> I like the idea in general, but I think it may collide with Tejun's NUMA 
> unification patchset for x86, which I assume will another revision once 
> the merge window for .38 closes?  Or maybe he'd like to merge your patch 
> with his series and push it as one?

I would assume that if unification is done, then the command line
option would become common too. Tejun?

However, the problem my patch addresses has been long standing
(I noted it with our .32 based kernel, but according to my looking at
the code it would go back to at least .27), so I'd like to ask for it to
be merged independently (and I should probably have copied stable
too), unless (quite unlikely) Tejun's merge is intended to also be
applied to stable kernels.

Jan

Re: [PATCH] x86: unify "numa=" command line option handling

[David Rientjes]

On Fri, 7 Jan 2011, Jan Beulich wrote:

> However, the problem my patch addresses has been long standing
> (I noted it with our .32 based kernel, but according to my looking at
> the code it would go back to at least .27), so I'd like to ask for it to
> be merged independently (and I should probably have copied stable
> too), unless (quite unlikely) Tejun's merge is intended to also be
> applied to stable kernels.
> 

I don't think this should be targeted to -stable since it's not a bugfix; 
this is adding a feature that allows you to disable acpi parsing of the 
SRAT on i386.  Since Tejun's patchset is mature, I think the unification 
should be handled there (if not done so already).

Re: [PATCH] x86: unify "numa=" command line option handling

[Jan Beulich]

>>> On 07.01.11 at 10:58, David Rientjes <rientjes@google.com> wrote:
> On Fri, 7 Jan 2011, Jan Beulich wrote:
> 
>> However, the problem my patch addresses has been long standing
>> (I noted it with our .32 based kernel, but according to my looking at
>> the code it would go back to at least .27), so I'd like to ask for it to
>> be merged independently (and I should probably have copied stable
>> too), unless (quite unlikely) Tejun's merge is intended to also be
>> applied to stable kernels.
>> 
> 
> I don't think this should be targeted to -stable since it's not a bugfix; 
> this is adding a feature that allows you to disable acpi parsing of the 
> SRAT on i386.

How is this not a bug fix if it allows a system to boot that previously
didn't?

Jan

Re: [PATCH] x86: unify "numa=" command line option handling

[Ingo Molnar]

* Jan Beulich <JBeulich@novell.com> wrote:

> >>> On 07.01.11 at 10:58, David Rientjes <rientjes@google.com> wrote:
> > On Fri, 7 Jan 2011, Jan Beulich wrote:
> > 
> >> However, the problem my patch addresses has been long standing
> >> (I noted it with our .32 based kernel, but according to my looking at
> >> the code it would go back to at least .27), so I'd like to ask for it to
> >> be merged independently (and I should probably have copied stable
> >> too), unless (quite unlikely) Tejun's merge is intended to also be
> >> applied to stable kernels.
> >> 
> > 
> > I don't think this should be targeted to -stable since it's not a bugfix; 
> > this is adding a feature that allows you to disable acpi parsing of the 
> > SRAT on i386.
> 
> How is this not a bug fix if it allows a system to boot that previously
> didn't?

btw., that's an absolutely key piece of information that REALLY should have been 
included in the changelog of the first patch. It is more important than all of the 
changelog.

Thanks,

	Ingo

Re: [PATCH] x86: unify "numa=" command line option handling

[Jan Beulich]

>>> On 07.01.11 at 13:57, Ingo Molnar <mingo@elte.hu> wrote:

> * Jan Beulich <JBeulich@novell.com> wrote:
> 
>> >>> On 07.01.11 at 10:58, David Rientjes <rientjes@google.com> wrote:
>> > On Fri, 7 Jan 2011, Jan Beulich wrote:
>> > 
>> >> However, the problem my patch addresses has been long standing
>> >> (I noted it with our .32 based kernel, but according to my looking at
>> >> the code it would go back to at least .27), so I'd like to ask for it to
>> >> be merged independently (and I should probably have copied stable
>> >> too), unless (quite unlikely) Tejun's merge is intended to also be
>> >> applied to stable kernels.
>> >> 
>> > 
>> > I don't think this should be targeted to -stable since it's not a bugfix; 
>> > this is adding a feature that allows you to disable acpi parsing of the 
>> > SRAT on i386.
>> 
>> How is this not a bug fix if it allows a system to boot that previously
>> didn't?
> 
> btw., that's an absolutely key piece of information that REALLY should have 
> been 
> included in the changelog of the first patch. It is more important than all 
> of the 
> changelog.

Quoting that text: "In order to be able to suppress the use of SRAT
tables that 32-bit Linux can't deal with (possibly leading to a non-
bootable system, without disabling ACPI altogether), move the
"numa=" option handling to common code."

To me it says just that. And of course, not every system with a
not understood SRAT would be yielded non-bootable, hence the
wording "possibly leading to ...".

Jan

Re: [PATCH] x86: unify "numa=" command line option handling

[Ingo Molnar]

* Jan Beulich <JBeulich@novell.com> wrote:

> >>> On 07.01.11 at 13:57, Ingo Molnar <mingo@elte.hu> wrote:
> 
> > * Jan Beulich <JBeulich@novell.com> wrote:
> > 
> >> >>> On 07.01.11 at 10:58, David Rientjes <rientjes@google.com> wrote:
> >> > On Fri, 7 Jan 2011, Jan Beulich wrote:
> >> > 
> >> >> However, the problem my patch addresses has been long standing
> >> >> (I noted it with our .32 based kernel, but according to my looking at
> >> >> the code it would go back to at least .27), so I'd like to ask for it to
> >> >> be merged independently (and I should probably have copied stable
> >> >> too), unless (quite unlikely) Tejun's merge is intended to also be
> >> >> applied to stable kernels.
> >> >> 
> >> > 
> >> > I don't think this should be targeted to -stable since it's not a bugfix; 
> >> > this is adding a feature that allows you to disable acpi parsing of the 
> >> > SRAT on i386.
> >> 
> >> How is this not a bug fix if it allows a system to boot that previously
> >> didn't?
> > 
> > btw., that's an absolutely key piece of information that REALLY should have 
> > been 
> > included in the changelog of the first patch. It is more important than all 
> > of the 
> > changelog.
> 
> Quoting that text: "In order to be able to suppress the use of SRAT
> tables that 32-bit Linux can't deal with (possibly leading to a non-
> bootable system, without disabling ACPI altogether), move the
> "numa=" option handling to common code."
> 
> To me it says just that. And of course, not every system with a
> not understood SRAT would be yielded non-bootable, hence the
> wording "possibly leading to ...".


Your -stable comment above made it appear to me as if you knew about a specific 
system that crashed this way? As long as it's only theoretical i'm not sure it 
warrants a -stable backport.

Thanks,

	Ingo

Re: [PATCH] x86: unify "numa=" command line option handling

[Jan Beulich]

>>> On 07.01.11 at 15:22, Ingo Molnar <mingo@elte.hu> wrote:
> Your -stable comment above made it appear to me as if you knew about a 
> specific 
> system that crashed this way? As long as it's only theoretical i'm not sure 
> it 
> warrants a -stable backport.

Yes, I do have a system affected (which made me craft the patch
in the first place).

Jan

Re: [PATCH] x86: unify "numa=" command line option handling

[Ingo Molnar]

* Jan Beulich <JBeulich@novell.com> wrote:

> >>> On 07.01.11 at 15:22, Ingo Molnar <mingo@elte.hu> wrote:
> > Your -stable comment above made it appear to me as if you knew about a 
> > specific 
> > system that crashed this way? As long as it's only theoretical i'm not sure 
> > it 
> > warrants a -stable backport.
> 
> Yes, I do have a system affected (which made me craft the patch
> in the first place).

In that case it's very useful to start the commit with:

  System XYZ crashes during bootup due to a bug in numa= command line option 
  handling.

That will also cause me to add an immediate -stable backport tag from me, even if 
you dont add it. Keeping it all optional and theoretical with 'it may crash' wording 
just hides the essential piece of information that there's a real system affected by 
the bug.

Thanks,

	Ingo

Re: [PATCH] x86: unify "numa=" command line option handling

[David Rientjes]

On Fri, 7 Jan 2011, Ingo Molnar wrote:

> > > Your -stable comment above made it appear to me as if you knew about a 
> > > specific 
> > > system that crashed this way? As long as it's only theoretical i'm not sure 
> > > it 
> > > warrants a -stable backport.
> > 
> > Yes, I do have a system affected (which made me craft the patch
> > in the first place).
> 
> In that case it's very useful to start the commit with:
> 
>   System XYZ crashes during bootup due to a bug in numa= command line option 
>   handling.
> 
> That will also cause me to add an immediate -stable backport tag from me, even if 
> you dont add it. Keeping it all optional and theoretical with 'it may crash' wording 
> just hides the essential piece of information that there's a real system affected by 
> the bug.
> 

I still don't think it meets the stable kernel rules.  The changelog 
explicitly states that we do not want to disable ACPI completely, so the 
only numa= command line option that would possibly be useful on 32-bit is 
numa=off in this case.  If you're compiling a 32-bit kernel for a machine 
with SRAT entries that can't be parsed by the kernel, then there's still 
no explanation as to why CONFIG_NUMA=n won't fix it.

Re: [PATCH] x86: unify "numa=" command line option handling

[Jan Beulich]

>>> On 07.01.11 at 20:55, David Rientjes <rientjes@google.com> wrote:
> I still don't think it meets the stable kernel rules.  The changelog 
> explicitly states that we do not want to disable ACPI completely, so the 
> only numa= command line option that would possibly be useful on 32-bit is 
> numa=off in this case.  If you're compiling a 32-bit kernel for a machine 
> with SRAT entries that can't be parsed by the kernel, then there's still 
> no explanation as to why CONFIG_NUMA=n won't fix it.

In distro kernels you have no control over the configuration, yet you
may want the kernel to boot on a particular machine.

Jan

Re: [PATCH] x86: unify "numa=" command line option handling

[Tejun Heo]

Hello, Jan.

On Fri, Jan 07, 2011 at 07:51:09AM +0000, Jan Beulich wrote:
> > I like the idea in general, but I think it may collide with Tejun's NUMA 
> > unification patchset for x86, which I assume will another revision once 
> > the merge window for .38 closes?  Or maybe he'd like to merge your patch 
> > with his series and push it as one?
> 
> I would assume that if unification is done, then the command line
> option would become common too. Tejun?

The unification isn't full and the parameter handling hasn't been
moved yet, so yeah, either your patch can go in first and I can
refresh my series on top of it or vice-versa.

Thanks.

-- 
tejun

Re: [PATCH] x86: unify "numa=" command line option handling

[Tejun Heo]

Hello,

On Thu, Jan 06, 2011 at 12:36:41PM -0800, David Rientjes wrote:
> I like the idea in general, but I think it may collide with Tejun's NUMA 
> unification patchset for x86, which I assume will another revision once 
> the merge window for .38 closes?  Or maybe he'd like to merge your patch 
> with his series and push it as one?

Peter, what's the status of the patchset?  Is there something which is
holding it from being merged into x86/numa?  Are you simply waiting
for the rc1 window to close?

Thanks.

-- 
tejun

Re: [PATCH] x86: unify "numa=" command line option handling

[David Rientjes]

On Fri, 7 Jan 2011, Tejun Heo wrote:

> > I like the idea in general, but I think it may collide with Tejun's NUMA 
> > unification patchset for x86, which I assume will another revision once 
> > the merge window for .38 closes?  Or maybe he'd like to merge your patch 
> > with his series and push it as one?
> 
> Peter, what's the status of the patchset?  Is there something which is
> holding it from being merged into x86/numa?  Are you simply waiting
> for the rc1 window to close?
> 

I don't think it applies cleanly anymore since d906f0eb was merged, but I 
agree with Jan that this new feature should probably be merged for .38.

Re: [PATCH] x86: unify "numa=" command line option handling

[Tejun Heo]

On Fri, Jan 07, 2011 at 12:36:41PM -0800, David Rientjes wrote:
> I don't think it applies cleanly anymore since d906f0eb was merged, but I 
> agree with Jan that this new feature should probably be merged for .38.

Okay, the series is already too late for this merge window at this
point anyway.  I'm not too happy how the merge process is progressing
but, well, it seems there needs to be another refresh.  Anyways,
please let me know when the series needs to be refreshed.

Thanks.

-- 
tejun

Re: [PATCH] x86: unify "numa=" command line option handling

[David Rientjes]

On Fri, 7 Jan 2011, Tejun Heo wrote:

> > I don't think it applies cleanly anymore since d906f0eb was merged, but I 
> > agree with Jan that this new feature should probably be merged for .38.
> 
> Okay, the series is already too late for this merge window at this
> point anyway.  I'm not too happy how the merge process is progressing
> but, well, it seems there needs to be another refresh.  Anyways,
> please let me know when the series needs to be refreshed.
> 

It would probably be best after -rc1 and then be on track for .39.  I know 
the unification is much desired from several different parties, I've 
referred a few different emails in the past couple months to your patchset 
and potential collisions :)