[PATCH 0/5] acpi: do some changes for numa info

[PATCH 0/5] acpi: do some changes for numa info

[liguang]

just do some trivial changes to make acpi's numa info
operation more cleaner.

Li Guang(5)
	numa: avoid export acpi_numa variable
	acpi/numa: check if parsing acpi numa info disabled earlier
	acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c
	acpi: add clock_domain field to acpi_srat_cpu_affinity
	remove include asm/acpi.h in process_driver.c


 arch/x86/include/asm/acpi.h   	 |    3 ++-
 arch/x86/kernel/acpi/Makefile 	 |    1 +
 arch/x86/mm/Makefile            |    1 -
 arch/x86/mm/numa.c          	 |    2 +-
 arch/x86/mm/srat.c          	 |  207 +++++--------------------------------
 arch/x86/xen/enlighten.c    	 |    2 +-
 drivers/acpi/processor_driver.c |    1 -
 drivers/acpi/numa.c 			 	 |    2 ++
 include/acpi/actbl1.h 			 |    2 +-
 8 files changed, 13 insertions(+), 208 deletions(-)
 

[PATCH 1/5] numa: avoid export acpi_numa variable

[liguang]

acpi_numa is used to prevent srat table
being parsed, seems a little miss-named,
if 'noacpi' was specified by cmdline and
CONFIG_ACPI_NUMA was enabled, acpi_numa
will be operated directly from everywhere
it needed to disable/enable numa in acpi
mode which was a bad thing, so, try to
export a fuction to get srat table
enable/disable info.

Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 arch/x86/include/asm/acpi.h |    3 ++-
 arch/x86/mm/numa.c          |    2 +-
 arch/x86/mm/srat.c          |   10 +++++-----
 arch/x86/xen/enlighten.c    |    2 +-
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 0c44630..c4e0875 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -181,7 +181,8 @@ static inline void disable_acpi(void) { }
 #define ARCH_HAS_POWER_INIT	1
 
 #ifdef CONFIG_ACPI_NUMA
-extern int acpi_numa;
+extern void bad_srat(void);
+extern bool srat_disabled(void);
 extern int x86_acpi_numa_init(void);
 #endif /* CONFIG_ACPI_NUMA */
 
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index 2d125be..92986d8 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -47,7 +47,7 @@ static __init int numa_setup(char *opt)
 #endif
 #ifdef CONFIG_ACPI_NUMA
 	if (!strncmp(opt, "noacpi", 6))
-		acpi_numa = -1;
+		bad_srat();
 #endif
 	return 0;
 }
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index 4ddf497..a837c95 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -24,22 +24,22 @@
 #include <asm/apic.h>
 #include <asm/uv/uv.h>
 
-int acpi_numa __initdata;
+static bool acpi_numa __initdata;
 
 static __init int setup_node(int pxm)
 {
 	return acpi_map_pxm_to_node(pxm);
 }
 
-static __init void bad_srat(void)
+void __init bad_srat(void)
 {
 	printk(KERN_ERR "SRAT: SRAT not used.\n");
-	acpi_numa = -1;
+	acpi_numa = false;
 }
 
-static __init inline int srat_disabled(void)
+bool __init srat_disabled(void)
 {
-	return acpi_numa < 0;
+	return acpi_numa;
 }
 
 /* Callback for SLIT parsing */
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 138e566..94c8c70 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1415,7 +1415,7 @@ asmlinkage void __init xen_start_kernel(void)
 	 * any NUMA information the kernel tries to get from ACPI will
 	 * be meaningless.  Prevent it from trying.
 	 */
-	acpi_numa = -1;
+	bad_srat();
 #endif
 
 	/* Don't do the full vcpu_info placement stuff until we have a
-- 
1.7.2.5

[PATCH 2/5] acpi/numa: check if parsing acpi numa info disabled earlier

[liguang]

Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 arch/x86/mm/srat.c  |    6 ------
 drivers/acpi/numa.c |    2 ++
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index a837c95..78c67bd 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -60,8 +60,6 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
 	int pxm, node;
 	int apic_id;
 
-	if (srat_disabled())
-		return;
 	if (pa->header.length < sizeof(struct acpi_srat_x2apic_cpu_affinity)) {
 		bad_srat();
 		return;
@@ -100,8 +98,6 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
 	int pxm, node;
 	int apic_id;
 
-	if (srat_disabled())
-		return;
 	if (pa->header.length != sizeof(struct acpi_srat_cpu_affinity)) {
 		bad_srat();
 		return;
@@ -148,8 +144,6 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
 	u64 start, end;
 	int node, pxm;
 
-	if (srat_disabled())
-		return -1;
 	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
 		bad_srat();
 		return -1;
diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
index cb31298..1f51222 100644
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -262,6 +262,8 @@ static int __init acpi_parse_srat(struct acpi_table_header *table)
 	struct acpi_table_srat *srat;
 	if (!table)
 		return -EINVAL;
+	if (srat_disabled())
+		return -EACCES;
 
 	srat = (struct acpi_table_srat *)table;
 	acpi_srat_revision = srat->header.revision;
-- 
1.7.2.5

[PATCH 3/5] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

[liguang]

srat table should present only on acpi domain,
seems mm/ is not the right place for it.

Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 arch/x86/kernel/acpi/Makefile |    1 +
 arch/x86/mm/Makefile          |    1 -
 arch/x86/mm/srat.c            |  191 -----------------------------------------
 3 files changed, 1 insertions(+), 192 deletions(-)
 delete mode 100644 arch/x86/mm/srat.c

diff --git a/arch/x86/kernel/acpi/Makefile b/arch/x86/kernel/acpi/Makefile
index 163b225..1894c22 100644
--- a/arch/x86/kernel/acpi/Makefile
+++ b/arch/x86/kernel/acpi/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_ACPI)		+= boot.o
 obj-$(CONFIG_ACPI_SLEEP)	+= sleep.o wakeup_$(BITS).o
+obj-$(CONFIG_ACPI_NUMA)        += srat.o
 
 ifneq ($(CONFIG_ACPI_PROCESSOR),)
 obj-y				+= cstate.o
diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile
index 23d8e5f..d6f3692 100644
--- a/arch/x86/mm/Makefile
+++ b/arch/x86/mm/Makefile
@@ -24,7 +24,6 @@ obj-$(CONFIG_MMIOTRACE_TEST)	+= testmmiotrace.o
 
 obj-$(CONFIG_NUMA)		+= numa.o numa_$(BITS).o
 obj-$(CONFIG_AMD_NUMA)		+= amdtopology.o
-obj-$(CONFIG_ACPI_NUMA)		+= srat.o
 obj-$(CONFIG_NUMA_EMU)		+= numa_emulation.o
 
 obj-$(CONFIG_MEMTEST)		+= memtest.o
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
deleted file mode 100644
index 78c67bd..0000000
--- a/arch/x86/mm/srat.c
+++ /dev/null
@@ -1,191 +0,0 @@
-/*
- * ACPI 3.0 based NUMA setup
- * Copyright 2004 Andi Kleen, SuSE Labs.
- *
- * Reads the ACPI SRAT table to figure out what memory belongs to which CPUs.
- *
- * Called from acpi_numa_init while reading the SRAT and SLIT tables.
- * Assumes all memory regions belonging to a single proximity domain
- * are in one chunk. Holes between them will be included in the node.
- */
-
-#include <linux/kernel.h>
-#include <linux/acpi.h>
-#include <linux/mmzone.h>
-#include <linux/bitmap.h>
-#include <linux/module.h>
-#include <linux/topology.h>
-#include <linux/bootmem.h>
-#include <linux/memblock.h>
-#include <linux/mm.h>
-#include <asm/proto.h>
-#include <asm/numa.h>
-#include <asm/e820.h>
-#include <asm/apic.h>
-#include <asm/uv/uv.h>
-
-static bool acpi_numa __initdata;
-
-static __init int setup_node(int pxm)
-{
-	return acpi_map_pxm_to_node(pxm);
-}
-
-void __init bad_srat(void)
-{
-	printk(KERN_ERR "SRAT: SRAT not used.\n");
-	acpi_numa = false;
-}
-
-bool __init srat_disabled(void)
-{
-	return acpi_numa;
-}
-
-/* Callback for SLIT parsing */
-void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
-{
-	int i, j;
-
-	for (i = 0; i < slit->locality_count; i++)
-		for (j = 0; j < slit->locality_count; j++)
-			numa_set_distance(pxm_to_node(i), pxm_to_node(j),
-				slit->entry[slit->locality_count * i + j]);
-}
-
-/* Callback for Proximity Domain -> x2APIC mapping */
-void __init
-acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
-{
-	int pxm, node;
-	int apic_id;
-
-	if (pa->header.length < sizeof(struct acpi_srat_x2apic_cpu_affinity)) {
-		bad_srat();
-		return;
-	}
-	if ((pa->flags & ACPI_SRAT_CPU_ENABLED) == 0)
-		return;
-	pxm = pa->proximity_domain;
-	apic_id = pa->apic_id;
-	if (!apic->apic_id_valid(apic_id)) {
-		printk(KERN_INFO "SRAT: PXM %u -> X2APIC 0x%04x ignored\n",
-			 pxm, apic_id);
-		return;
-	}
-	node = setup_node(pxm);
-	if (node < 0) {
-		printk(KERN_ERR "SRAT: Too many proximity domains %x\n", pxm);
-		bad_srat();
-		return;
-	}
-
-	if (apic_id >= MAX_LOCAL_APIC) {
-		printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%04x -> Node %u skipped apicid that is too big\n", pxm, apic_id, node);
-		return;
-	}
-	set_apicid_to_node(apic_id, node);
-	node_set(node, numa_nodes_parsed);
-	acpi_numa = 1;
-	printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%04x -> Node %u\n",
-	       pxm, apic_id, node);
-}
-
-/* Callback for Proximity Domain -> LAPIC mapping */
-void __init
-acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
-{
-	int pxm, node;
-	int apic_id;
-
-	if (pa->header.length != sizeof(struct acpi_srat_cpu_affinity)) {
-		bad_srat();
-		return;
-	}
-	if ((pa->flags & ACPI_SRAT_CPU_ENABLED) == 0)
-		return;
-	pxm = pa->proximity_domain_lo;
-	if (acpi_srat_revision >= 2)
-		pxm |= *((unsigned int*)pa->proximity_domain_hi) << 8;
-	node = setup_node(pxm);
-	if (node < 0) {
-		printk(KERN_ERR "SRAT: Too many proximity domains %x\n", pxm);
-		bad_srat();
-		return;
-	}
-
-	if (get_uv_system_type() >= UV_X2APIC)
-		apic_id = (pa->apic_id << 8) | pa->local_sapic_eid;
-	else
-		apic_id = pa->apic_id;
-
-	if (apic_id >= MAX_LOCAL_APIC) {
-		printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%02x -> Node %u skipped apicid that is too big\n", pxm, apic_id, node);
-		return;
-	}
-
-	set_apicid_to_node(apic_id, node);
-	node_set(node, numa_nodes_parsed);
-	acpi_numa = 1;
-	printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%02x -> Node %u\n",
-	       pxm, apic_id, node);
-}
-
-#ifdef CONFIG_MEMORY_HOTPLUG
-static inline int save_add_info(void) {return 1;}
-#else
-static inline int save_add_info(void) {return 0;}
-#endif
-
-/* Callback for parsing of the Proximity Domain <-> Memory Area mappings */
-int __init
-acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
-{
-	u64 start, end;
-	int node, pxm;
-
-	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
-		bad_srat();
-		return -1;
-	}
-	if ((ma->flags & ACPI_SRAT_MEM_ENABLED) == 0)
-		return -1;
-
-	if ((ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) && !save_add_info())
-		return -1;
-	start = ma->base_address;
-	end = start + ma->length;
-	pxm = ma->proximity_domain;
-	if (acpi_srat_revision <= 1)
-		pxm &= 0xff;
-	node = setup_node(pxm);
-	if (node < 0) {
-		printk(KERN_ERR "SRAT: Too many proximity domains.\n");
-		bad_srat();
-		return -1;
-	}
-
-	if (numa_add_memblk(node, start, end) < 0) {
-		bad_srat();
-		return -1;
-	}
-
-	node_set(node, numa_nodes_parsed);
-
-	printk(KERN_INFO "SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]\n",
-	       node, pxm,
-	       (unsigned long long) start, (unsigned long long) end - 1);
-	return 0;
-}
-
-void __init acpi_numa_arch_fixup(void) {}
-
-int __init x86_acpi_numa_init(void)
-{
-	int ret;
-
-	ret = acpi_numa_init();
-	if (ret < 0)
-		return ret;
-	return srat_disabled() ? -EINVAL : 0;
-}
-- 
1.7.2.5

[PATCH 4/5] acpi: add clock_domain field to acpi_srat_cpu_affinity

[liguang]

according to ACPI SPEC v5.0, page 152,
5.2.16.1 Processor Local APIC/SAPIC Affinity Structure,
the last member of it is clock_domain.

Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 include/acpi/actbl1.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 280fc45..98031a3 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -922,7 +922,7 @@ struct acpi_srat_cpu_affinity {
 	u32 flags;
 	u8 local_sapic_eid;
 	u8 proximity_domain_hi[3];
-	u32 reserved;		/* Reserved, must be zero */
+	u32 clock_domain;
 };
 
 /* Flags */
-- 
1.7.2.5

[PATCH 5/5] remove include asm/acpi.h in process_driver.c

[liguang]

process_driver.c include linux/acpi.h which already
include asm/acpi.h, so remove it.

Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 drivers/acpi/processor_driver.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index e83311b..752eedb 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -52,7 +52,6 @@
 #include <asm/uaccess.h>
 #include <asm/processor.h>
 #include <asm/smp.h>
-#include <asm/acpi.h>
 
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
-- 
1.7.2.5

Re: [PATCH 1/5] numa: avoid export acpi_numa variable

[David Rientjes]

On Mon, 4 Feb 2013, liguang wrote:

> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
> index 0c44630..c4e0875 100644
> --- a/arch/x86/include/asm/acpi.h
> +++ b/arch/x86/include/asm/acpi.h
> @@ -181,7 +181,8 @@ static inline void disable_acpi(void) { }
>  #define ARCH_HAS_POWER_INIT	1
>  
>  #ifdef CONFIG_ACPI_NUMA
> -extern int acpi_numa;
> +extern void bad_srat(void);
> +extern bool srat_disabled(void);

So anything that wants to disable acpi_numa, such as xen, must call a 
function named bad_srat()?  That seems a little misnamed itself.

>  extern int x86_acpi_numa_init(void);
>  #endif /* CONFIG_ACPI_NUMA */
>  
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index 2d125be..92986d8 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -47,7 +47,7 @@ static __init int numa_setup(char *opt)
>  #endif
>  #ifdef CONFIG_ACPI_NUMA
>  	if (!strncmp(opt, "noacpi", 6))
> -		acpi_numa = -1;
> +		bad_srat();

This will print an error line saying "SRAT: SRAT not used." which wasn't 
emitted before.  I don't think that's the desire.

>  #endif
>  	return 0;
>  }
> diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
> index 4ddf497..a837c95 100644
> --- a/arch/x86/mm/srat.c
> +++ b/arch/x86/mm/srat.c
> @@ -24,22 +24,22 @@
>  #include <asm/apic.h>
>  #include <asm/uv/uv.h>
>  
> -int acpi_numa __initdata;
> +static bool acpi_numa __initdata;
>  

You've changed this to be type bool but didn't update the existing code 
that still does "acpi_numa = 1".

>  static __init int setup_node(int pxm)
>  {
>  	return acpi_map_pxm_to_node(pxm);
>  }
>  
> -static __init void bad_srat(void)
> +void __init bad_srat(void)
>  {
>  	printk(KERN_ERR "SRAT: SRAT not used.\n");
> -	acpi_numa = -1;
> +	acpi_numa = false;
>  }
>  
> -static __init inline int srat_disabled(void)
> +bool __init srat_disabled(void)
>  {
> -	return acpi_numa < 0;
> +	return acpi_numa;
>  }
>  
>  /* Callback for SLIT parsing */
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 138e566..94c8c70 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1415,7 +1415,7 @@ asmlinkage void __init xen_start_kernel(void)
>  	 * any NUMA information the kernel tries to get from ACPI will
>  	 * be meaningless.  Prevent it from trying.
>  	 */
> -	acpi_numa = -1;
> +	bad_srat();

Same as before.  It seems like you want to introduce a new function 
specifically to disable it:

	void acpi_numa_disable(void)
	{
		acpi_numa = false;
	}

and then bad_srat() should call into it?

Re: [PATCH 2/5] acpi/numa: check if parsing acpi numa info disabled earlier

[David Rientjes]

On Mon, 4 Feb 2013, liguang wrote:

> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>

Because there's no changelog, I have to read the patch to figure out what 
it's doing since the title isn't that helpful either.  Please provide a 
description of what problem you're trying to fix or what improvement 
you're trying to make so it's clear.

> ---
>  arch/x86/mm/srat.c  |    6 ------
>  drivers/acpi/numa.c |    2 ++
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
> index a837c95..78c67bd 100644
> --- a/arch/x86/mm/srat.c
> +++ b/arch/x86/mm/srat.c
> @@ -60,8 +60,6 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
>  	int pxm, node;
>  	int apic_id;
>  
> -	if (srat_disabled())
> -		return;
>  	if (pa->header.length < sizeof(struct acpi_srat_x2apic_cpu_affinity)) {
>  		bad_srat();
>  		return;
> @@ -100,8 +98,6 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>  	int pxm, node;
>  	int apic_id;
>  
> -	if (srat_disabled())
> -		return;
>  	if (pa->header.length != sizeof(struct acpi_srat_cpu_affinity)) {
>  		bad_srat();
>  		return;
> @@ -148,8 +144,6 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
>  	u64 start, end;
>  	int node, pxm;
>  
> -	if (srat_disabled())
> -		return -1;
>  	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
>  		bad_srat();
>  		return -1;
> diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
> index cb31298..1f51222 100644
> --- a/drivers/acpi/numa.c
> +++ b/drivers/acpi/numa.c
> @@ -262,6 +262,8 @@ static int __init acpi_parse_srat(struct acpi_table_header *table)
>  	struct acpi_table_srat *srat;
>  	if (!table)
>  		return -EINVAL;
> +	if (srat_disabled())
> +		return -EACCES;
>  
>  	srat = (struct acpi_table_srat *)table;
>  	acpi_srat_revision = srat->header.revision;

Nack, this isn't helpful since SRAT is only for x86 and other 
architectures use this code.  It would break the build on ia64 since it's 
obviously not going to have a function called srat_disabled().

And -EACCES would not be the appropriate return value, this has nothing to 
do with permissions.

Re: [PATCH 3/5] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

[David Rientjes]

On Mon, 4 Feb 2013, liguang wrote:

> srat table should present only on acpi domain,
> seems mm/ is not the right place for it.
> 
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
>  arch/x86/kernel/acpi/Makefile |    1 +
>  arch/x86/mm/Makefile          |    1 -
>  arch/x86/mm/srat.c            |  191 -----------------------------------------
>  3 files changed, 1 insertions(+), 192 deletions(-)
>  delete mode 100644 arch/x86/mm/srat.c
> 

I was excited for a moment because this would have been one great cleanup, 
but you never readded the code to arch/x86/kernel/acpi.  (You probably 
need to do "git add <newfile>" before generating the patch file.)  I do 
like the idea of moving it, though, if it's build tested thoroughly.

Re: [PATCH 1/5] numa: avoid export acpi_numa variable

[li guang]

在 2013-02-04一的 11:31 -0800，David Rientjes写道：
> On Mon, 4 Feb 2013, liguang wrote:
> 
> > diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
> > index 0c44630..c4e0875 100644
> > --- a/arch/x86/include/asm/acpi.h
> > +++ b/arch/x86/include/asm/acpi.h
> > @@ -181,7 +181,8 @@ static inline void disable_acpi(void) { }
> >  #define ARCH_HAS_POWER_INIT	1
> >  
> >  #ifdef CONFIG_ACPI_NUMA
> > -extern int acpi_numa;
> > +extern void bad_srat(void);
> > +extern bool srat_disabled(void);
> 
> So anything that wants to disable acpi_numa, such as xen, must call a 
> function named bad_srat()?  That seems a little misnamed itself.

I would assume every caller have the sense of what SRAT is.
(may be a little bold assumption)

> 
> >  extern int x86_acpi_numa_init(void);
> >  #endif /* CONFIG_ACPI_NUMA */
> >  
> > diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> > index 2d125be..92986d8 100644
> > --- a/arch/x86/mm/numa.c
> > +++ b/arch/x86/mm/numa.c
> > @@ -47,7 +47,7 @@ static __init int numa_setup(char *opt)
> >  #endif
> >  #ifdef CONFIG_ACPI_NUMA
> >  	if (!strncmp(opt, "noacpi", 6))
> > -		acpi_numa = -1;
> > +		bad_srat();
> 
> This will print an error line saying "SRAT: SRAT not used." which wasn't 
> emitted before.  I don't think that's the desire.

yes, you're right.

> 
> >  #endif
> >  	return 0;
> >  }
> > diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
> > index 4ddf497..a837c95 100644
> > --- a/arch/x86/mm/srat.c
> > +++ b/arch/x86/mm/srat.c
> > @@ -24,22 +24,22 @@
> >  #include <asm/apic.h>
> >  #include <asm/uv/uv.h>
> >  
> > -int acpi_numa __initdata;
> > +static bool acpi_numa __initdata;
> >  
> 
> You've changed this to be type bool but didn't update the existing code 
> that still does "acpi_numa = 1".

good catch!

> 
> >  static __init int setup_node(int pxm)
> >  {
> >  	return acpi_map_pxm_to_node(pxm);
> >  }
> >  
> > -static __init void bad_srat(void)
> > +void __init bad_srat(void)
> >  {
> >  	printk(KERN_ERR "SRAT: SRAT not used.\n");
> > -	acpi_numa = -1;
> > +	acpi_numa = false;
> >  }
> >  
> > -static __init inline int srat_disabled(void)
> > +bool __init srat_disabled(void)
> >  {
> > -	return acpi_numa < 0;
> > +	return acpi_numa;
> >  }
> >  
> >  /* Callback for SLIT parsing */
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index 138e566..94c8c70 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -1415,7 +1415,7 @@ asmlinkage void __init xen_start_kernel(void)
> >  	 * any NUMA information the kernel tries to get from ACPI will
> >  	 * be meaningless.  Prevent it from trying.
> >  	 */
> > -	acpi_numa = -1;
> > +	bad_srat();
> 
> Same as before.  It seems like you want to introduce a new function 
> specifically to disable it:
> 
> 	void acpi_numa_disable(void)
> 	{
> 		acpi_numa = false;
> 	}
> 
> and then bad_srat() should call into it?

good idea, Thanks!

Re: [PATCH 2/5] acpi/numa: check if parsing acpi numa info disabled earlier

[li guang]

在 2013-02-04一的 11:41 -0800，David Rientjes写道：
> On Mon, 4 Feb 2013, liguang wrote:
> 
> > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> 
> Because there's no changelog, I have to read the patch to figure out what 
> it's doing since the title isn't that helpful either.  Please provide a 
> description of what problem you're trying to fix or what improvement 
> you're trying to make so it's clear.
> 
> > ---
> >  arch/x86/mm/srat.c  |    6 ------
> >  drivers/acpi/numa.c |    2 ++
> >  2 files changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
> > index a837c95..78c67bd 100644
> > --- a/arch/x86/mm/srat.c
> > +++ b/arch/x86/mm/srat.c
> > @@ -60,8 +60,6 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
> >  	int pxm, node;
> >  	int apic_id;
> >  
> > -	if (srat_disabled())
> > -		return;
> >  	if (pa->header.length < sizeof(struct acpi_srat_x2apic_cpu_affinity)) {
> >  		bad_srat();
> >  		return;
> > @@ -100,8 +98,6 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >  	int pxm, node;
> >  	int apic_id;
> >  
> > -	if (srat_disabled())
> > -		return;
> >  	if (pa->header.length != sizeof(struct acpi_srat_cpu_affinity)) {
> >  		bad_srat();
> >  		return;
> > @@ -148,8 +144,6 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
> >  	u64 start, end;
> >  	int node, pxm;
> >  
> > -	if (srat_disabled())
> > -		return -1;
> >  	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity)) {
> >  		bad_srat();
> >  		return -1;
> > diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
> > index cb31298..1f51222 100644
> > --- a/drivers/acpi/numa.c
> > +++ b/drivers/acpi/numa.c
> > @@ -262,6 +262,8 @@ static int __init acpi_parse_srat(struct acpi_table_header *table)
> >  	struct acpi_table_srat *srat;
> >  	if (!table)
> >  		return -EINVAL;
> > +	if (srat_disabled())
> > +		return -EACCES;
> >  
> >  	srat = (struct acpi_table_srat *)table;
> >  	acpi_srat_revision = srat->header.revision;
> 
> Nack, this isn't helpful since SRAT is only for x86 and other 
> architectures use this code.  It would break the build on ia64 since it's 
> obviously not going to have a function called srat_disabled().
> 
> And -EACCES would not be the appropriate return value, this has nothing to 
> do with permissions.

Yes, you're right, will drop this change.

Re: [PATCH 3/5] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

[li guang]

在 2013-02-04一的 11:44 -0800，David Rientjes写道：
> On Mon, 4 Feb 2013, liguang wrote:
> 
> > srat table should present only on acpi domain,
> > seems mm/ is not the right place for it.
> > 
> > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> > ---
> >  arch/x86/kernel/acpi/Makefile |    1 +
> >  arch/x86/mm/Makefile          |    1 -
> >  arch/x86/mm/srat.c            |  191 -----------------------------------------
> >  3 files changed, 1 insertions(+), 192 deletions(-)
> >  delete mode 100644 arch/x86/mm/srat.c
> > 
> 
> I was excited for a moment because this would have been one great cleanup, 
> but you never readded the code to arch/x86/kernel/acpi.  (You probably 
> need to do "git add <newfile>" before generating the patch file.)  I do 
> like the idea of moving it, though, if it's build tested thoroughly.

OK, will git add it.