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

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

[liguang]

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

ChangeLog

v2->v3
 1. rebase on linux-next
 2. bring back lost Makefile changes
 spotted by David Rientjes <rientjes@google.com>
 spotted by Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

v1->v2
 1. fix-up several coding issues
 2. finish srat.c change
 spotted by David Rientjes <rientjes@google.com>

Li Guang(4)
        numa: avoid export acpi_numa variable
        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		|    2 +-
 arch/x86/kernel/acpi/Makefile		|    1 +
 arch/x86/kernel/acpi/srat.c 		|  212 +++++++++++++++++++++++++++++++++++++++++++---
 arch/x86/mm/Makefile			|    1 -
 arch/x86/mm/srat.c          		|  198 -------------------------------------------
 arch/x86/mm/numa.c          		|    2 +-
 arch/x86/xen/enlighten.c    		|    2 +-
 include/acpi/actbl1.h       		|    2 +-
 drivers/acpi/processor_driver.c 	|    1 -

 9 files changed, 212 insertions(+), 208 deletions(-)
 create mode 100644 arch/x86/kernel/acpi/srat.c
 delete mode 100644 arch/x86/mm/srat.c

 

[PATCH v3 1/4] 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/kernel/acpi/srat.c   |  198 +++++++++++++++++++++++++++++++++++++++++
 arch/x86/mm/Makefile          |    1 -
 arch/x86/mm/srat.c            |  198 -----------------------------------------
 4 files changed, 199 insertions(+), 199 deletions(-)
 create mode 100644 arch/x86/kernel/acpi/srat.c
 delete mode 100644 arch/x86/mm/srat.c

diff --git a/arch/x86/kernel/acpi/Makefile b/arch/x86/kernel/acpi/Makefile
index 163b225..98cea92 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/kernel/acpi/srat.c b/arch/x86/kernel/acpi/srat.c
new file mode 100644
index 0000000..cdd0da9
--- /dev/null
+++ b/arch/x86/kernel/acpi/srat.c
@@ -0,0 +1,198 @@
+/*
+ * 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>
+
+int acpi_numa __initdata;
+
+static __init int setup_node(int pxm)
+{
+	return acpi_map_pxm_to_node(pxm);
+}
+
+static __init void bad_srat(void)
+{
+	printk(KERN_ERR "SRAT: SRAT not used.\n");
+	acpi_numa = -1;
+}
+
+static __init inline int srat_disabled(void)
+{
+	return acpi_numa < 0;
+}
+
+/* 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 (srat_disabled())
+		return;
+	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 (srat_disabled())
+		return;
+	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 (srat_disabled())
+		goto out_err;
+	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity))
+		goto out_err_bad_srat;
+	if ((ma->flags & ACPI_SRAT_MEM_ENABLED) == 0)
+		goto out_err;
+	if ((ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) && !save_add_info())
+		goto out_err;
+
+	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");
+		goto out_err_bad_srat;
+	}
+
+	if (numa_add_memblk(node, start, end) < 0)
+		goto out_err_bad_srat;
+
+	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;
+out_err_bad_srat:
+	bad_srat();
+out_err:
+	return -1;
+}
+
+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;
+}
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 cdd0da9..0000000
--- a/arch/x86/mm/srat.c
+++ /dev/null
@@ -1,198 +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>
-
-int acpi_numa __initdata;
-
-static __init int setup_node(int pxm)
-{
-	return acpi_map_pxm_to_node(pxm);
-}
-
-static __init void bad_srat(void)
-{
-	printk(KERN_ERR "SRAT: SRAT not used.\n");
-	acpi_numa = -1;
-}
-
-static __init inline int srat_disabled(void)
-{
-	return acpi_numa < 0;
-}
-
-/* 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 (srat_disabled())
-		return;
-	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 (srat_disabled())
-		return;
-	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 (srat_disabled())
-		goto out_err;
-	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity))
-		goto out_err_bad_srat;
-	if ((ma->flags & ACPI_SRAT_MEM_ENABLED) == 0)
-		goto out_err;
-	if ((ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) && !save_add_info())
-		goto out_err;
-
-	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");
-		goto out_err_bad_srat;
-	}
-
-	if (numa_add_memblk(node, start, end) < 0)
-		goto out_err_bad_srat;
-
-	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;
-out_err_bad_srat:
-	bad_srat();
-out_err:
-	return -1;
-}
-
-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 v3 2/4] 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 |    2 +-
 arch/x86/kernel/acpi/srat.c |   17 +++++++++++------
 arch/x86/mm/numa.c          |    2 +-
 arch/x86/xen/enlighten.c    |    2 +-
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index b31bf97..449e12a 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -177,7 +177,7 @@ static inline void disable_acpi(void) { }
 #define ARCH_HAS_POWER_INIT	1
 
 #ifdef CONFIG_ACPI_NUMA
-extern int acpi_numa;
+extern void disable_acpi_numa(void);
 extern int x86_acpi_numa_init(void);
 #endif /* CONFIG_ACPI_NUMA */
 
diff --git a/arch/x86/kernel/acpi/srat.c b/arch/x86/kernel/acpi/srat.c
index cdd0da9..8d7f3f8 100644
--- a/arch/x86/kernel/acpi/srat.c
+++ b/arch/x86/kernel/acpi/srat.c
@@ -24,22 +24,27 @@
 #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 disable_acpi_numa(void)
 {
-	printk(KERN_ERR "SRAT: SRAT not used.\n");
-	acpi_numa = -1;
+	acpi_numa = false;
 }
 
-static __init inline int srat_disabled(void)
+static void __init bad_srat(void)
 {
-	return acpi_numa < 0;
+	disable_acpi_numa();
+	printk(KERN_ERR "SRAT: SRAT will not be used.\n");
+}
+
+static bool __init srat_disabled(void)
+{
+	return acpi_numa;
 }
 
 /* Callback for SLIT parsing */
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index 245a4ba..1ebc907 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;
+		disable_acpi_numa();
 #endif
 	return 0;
 }
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 657eca9..3636bc6 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1447,7 +1447,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;
+	disable_acpi_numa();
 #endif
 
 	/* Don't do the full vcpu_info placement stuff until we have a
-- 
1.7.2.5

[PATCH v3 3/4] 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.

Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Acked-by: David Rientjes <rientjes@google.com>
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 0bd750e..e21d22b 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 v3 4/4] 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.

Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Acked-by: David Rientjes <rientjes@google.com>
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 d57e32c..2f3bc09 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -53,7 +53,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 v3 2/4] numa: avoid export acpi_numa variable

[Yasuaki Ishimatsu]

2013/02/05 16:36, liguang wrote:
> 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>
> ---

Hmm. Did you test the patch?
By the patch, srat_disable() returns false(0) or 1. As a result,
acpi_numa_x2apic_affinity_init(), acpi_numa_processor_affinity_init()
and acpi_numa_memory_affinity_init() go wrong at following if sentence.

---
	if (srat_disable())
		return -1;
---

When you change a return value of function, you should check othe
functions which use it carefully.

And if you use acpi_numa as bool, you should use "acpi_numa = true"
instead of "acpi_numa = 1"

Thanks,
Yasuaki Ishimatsu

>   arch/x86/include/asm/acpi.h |    2 +-
>   arch/x86/kernel/acpi/srat.c |   17 +++++++++++------
>   arch/x86/mm/numa.c          |    2 +-
>   arch/x86/xen/enlighten.c    |    2 +-
>   4 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
> index b31bf97..449e12a 100644
> --- a/arch/x86/include/asm/acpi.h
> +++ b/arch/x86/include/asm/acpi.h
> @@ -177,7 +177,7 @@ static inline void disable_acpi(void) { }
>   #define ARCH_HAS_POWER_INIT	1
>   
>   #ifdef CONFIG_ACPI_NUMA
> -extern int acpi_numa;
> +extern void disable_acpi_numa(void);
>   extern int x86_acpi_numa_init(void);
>   #endif /* CONFIG_ACPI_NUMA */
>   
> diff --git a/arch/x86/kernel/acpi/srat.c b/arch/x86/kernel/acpi/srat.c
> index cdd0da9..8d7f3f8 100644
> --- a/arch/x86/kernel/acpi/srat.c
> +++ b/arch/x86/kernel/acpi/srat.c
> @@ -24,22 +24,27 @@
>   #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 disable_acpi_numa(void)
>   {
> -	printk(KERN_ERR "SRAT: SRAT not used.\n");
> -	acpi_numa = -1;
> +	acpi_numa = false;
>   }
>   
> -static __init inline int srat_disabled(void)
> +static void __init bad_srat(void)
>   {
> -	return acpi_numa < 0;
> +	disable_acpi_numa();
> +	printk(KERN_ERR "SRAT: SRAT will not be used.\n");
> +}
> +
> +static bool __init srat_disabled(void)
> +{
> +	return acpi_numa;
>   }
>   
>   /* Callback for SLIT parsing */
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index 245a4ba..1ebc907 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;
> +		disable_acpi_numa();
>   #endif
>   	return 0;
>   }
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 657eca9..3636bc6 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1447,7 +1447,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;
> +	disable_acpi_numa();
>   #endif
>   
>   	/* Don't do the full vcpu_info placement stuff until we have a
> 

Re: [PATCH v3 1/4] acpi: move x86/mm/srat.c to x86/kernel/acpi/srat.c

[Yasuaki Ishimatsu]

2013/02/05 16:35, 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>
> ---

Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Thanks,
Yasuaki Ishimatsu

>   arch/x86/kernel/acpi/Makefile |    1 +
>   arch/x86/kernel/acpi/srat.c   |  198 +++++++++++++++++++++++++++++++++++++++++
>   arch/x86/mm/Makefile          |    1 -
>   arch/x86/mm/srat.c            |  198 -----------------------------------------
>   4 files changed, 199 insertions(+), 199 deletions(-)
>   create mode 100644 arch/x86/kernel/acpi/srat.c
>   delete mode 100644 arch/x86/mm/srat.c
> 
> diff --git a/arch/x86/kernel/acpi/Makefile b/arch/x86/kernel/acpi/Makefile
> index 163b225..98cea92 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/kernel/acpi/srat.c b/arch/x86/kernel/acpi/srat.c
> new file mode 100644
> index 0000000..cdd0da9
> --- /dev/null
> +++ b/arch/x86/kernel/acpi/srat.c
> @@ -0,0 +1,198 @@
> +/*
> + * 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>
> +
> +int acpi_numa __initdata;
> +
> +static __init int setup_node(int pxm)
> +{
> +	return acpi_map_pxm_to_node(pxm);
> +}
> +
> +static __init void bad_srat(void)
> +{
> +	printk(KERN_ERR "SRAT: SRAT not used.\n");
> +	acpi_numa = -1;
> +}
> +
> +static __init inline int srat_disabled(void)
> +{
> +	return acpi_numa < 0;
> +}
> +
> +/* 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 (srat_disabled())
> +		return;
> +	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 (srat_disabled())
> +		return;
> +	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 (srat_disabled())
> +		goto out_err;
> +	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity))
> +		goto out_err_bad_srat;
> +	if ((ma->flags & ACPI_SRAT_MEM_ENABLED) == 0)
> +		goto out_err;
> +	if ((ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) && !save_add_info())
> +		goto out_err;
> +
> +	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");
> +		goto out_err_bad_srat;
> +	}
> +
> +	if (numa_add_memblk(node, start, end) < 0)
> +		goto out_err_bad_srat;
> +
> +	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;
> +out_err_bad_srat:
> +	bad_srat();
> +out_err:
> +	return -1;
> +}
> +
> +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;
> +}
> 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 cdd0da9..0000000
> --- a/arch/x86/mm/srat.c
> +++ /dev/null
> @@ -1,198 +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>
> -
> -int acpi_numa __initdata;
> -
> -static __init int setup_node(int pxm)
> -{
> -	return acpi_map_pxm_to_node(pxm);
> -}
> -
> -static __init void bad_srat(void)
> -{
> -	printk(KERN_ERR "SRAT: SRAT not used.\n");
> -	acpi_numa = -1;
> -}
> -
> -static __init inline int srat_disabled(void)
> -{
> -	return acpi_numa < 0;
> -}
> -
> -/* 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 (srat_disabled())
> -		return;
> -	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 (srat_disabled())
> -		return;
> -	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 (srat_disabled())
> -		goto out_err;
> -	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity))
> -		goto out_err_bad_srat;
> -	if ((ma->flags & ACPI_SRAT_MEM_ENABLED) == 0)
> -		goto out_err;
> -	if ((ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) && !save_add_info())
> -		goto out_err;
> -
> -	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");
> -		goto out_err_bad_srat;
> -	}
> -
> -	if (numa_add_memblk(node, start, end) < 0)
> -		goto out_err_bad_srat;
> -
> -	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;
> -out_err_bad_srat:
> -	bad_srat();
> -out_err:
> -	return -1;
> -}
> -
> -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;
> -}
> 

Re: [PATCH v3 2/4] numa: avoid export acpi_numa variable

[li guang]

在 2013-02-05二的 16:55 +0900，Yasuaki Ishimatsu写道：
> 2013/02/05 16:36, liguang wrote:
> > 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>
> > ---
> 
> Hmm. Did you test the patch?
> By the patch, srat_disable() returns false(0) or 1. As a result,
> acpi_numa_x2apic_affinity_init(), acpi_numa_processor_affinity_init()
> and acpi_numa_memory_affinity_init() go wrong at following if sentence.
> 
> ---
> 	if (srat_disable())
> 		return -1;
> ---
> 
> When you change a return value of function, you should check othe
> functions which use it carefully.
> 
> And if you use acpi_numa as bool, you should use "acpi_numa = true"
> instead of "acpi_numa = 1"

Good observation!
maybe I'm a little hurry to go for dining, :)
Thanks! will fix.

> 
> Thanks,
> Yasuaki Ishimatsu
> 
> >   arch/x86/include/asm/acpi.h |    2 +-
> >   arch/x86/kernel/acpi/srat.c |   17 +++++++++++------
> >   arch/x86/mm/numa.c          |    2 +-
> >   arch/x86/xen/enlighten.c    |    2 +-
> >   4 files changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
> > index b31bf97..449e12a 100644
> > --- a/arch/x86/include/asm/acpi.h
> > +++ b/arch/x86/include/asm/acpi.h
> > @@ -177,7 +177,7 @@ static inline void disable_acpi(void) { }
> >   #define ARCH_HAS_POWER_INIT	1
> >   
> >   #ifdef CONFIG_ACPI_NUMA
> > -extern int acpi_numa;
> > +extern void disable_acpi_numa(void);
> >   extern int x86_acpi_numa_init(void);
> >   #endif /* CONFIG_ACPI_NUMA */
> >   
> > diff --git a/arch/x86/kernel/acpi/srat.c b/arch/x86/kernel/acpi/srat.c
> > index cdd0da9..8d7f3f8 100644
> > --- a/arch/x86/kernel/acpi/srat.c
> > +++ b/arch/x86/kernel/acpi/srat.c
> > @@ -24,22 +24,27 @@
> >   #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 disable_acpi_numa(void)
> >   {
> > -	printk(KERN_ERR "SRAT: SRAT not used.\n");
> > -	acpi_numa = -1;
> > +	acpi_numa = false;
> >   }
> >   
> > -static __init inline int srat_disabled(void)
> > +static void __init bad_srat(void)
> >   {
> > -	return acpi_numa < 0;
> > +	disable_acpi_numa();
> > +	printk(KERN_ERR "SRAT: SRAT will not be used.\n");
> > +}
> > +
> > +static bool __init srat_disabled(void)
> > +{
> > +	return acpi_numa;
> >   }
> >   
> >   /* Callback for SLIT parsing */
> > diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> > index 245a4ba..1ebc907 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;
> > +		disable_acpi_numa();
> >   #endif
> >   	return 0;
> >   }
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index 657eca9..3636bc6 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -1447,7 +1447,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;
> > +	disable_acpi_numa();
> >   #endif
> >   
> >   	/* Don't do the full vcpu_info placement stuff until we have a
> > 
> 
> 

Re: [PATCH v3 2/4] numa: avoid export acpi_numa variable

[Yasuaki Ishimatsu]

2013/02/05 16:55, Yasuaki Ishimatsu wrote:
> 2013/02/05 16:36, liguang wrote:
>> 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>
>> ---
> 
> Hmm. Did you test the patch?
> By the patch, srat_disable() returns false(0) or 1. As a result,
> acpi_numa_x2apic_affinity_init(), acpi_numa_processor_affinity_init()
> and acpi_numa_memory_affinity_init() go wrong at following if sentence.
> 
> ---
> 	if (srat_disable())
> 		return -1;
> ---
> 
> When you change a return value of function, you should check othe
> functions which use it carefully.
> 
> And if you use acpi_numa as bool, you should use "acpi_numa = true"
> instead of "acpi_numa = 1"


x86_acpi_numa_init() also goes wrong.

> 
> Thanks,
> Yasuaki Ishimatsu
> 
>>    arch/x86/include/asm/acpi.h |    2 +-
>>    arch/x86/kernel/acpi/srat.c |   17 +++++++++++------
>>    arch/x86/mm/numa.c          |    2 +-
>>    arch/x86/xen/enlighten.c    |    2 +-
>>    4 files changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
>> index b31bf97..449e12a 100644
>> --- a/arch/x86/include/asm/acpi.h
>> +++ b/arch/x86/include/asm/acpi.h
>> @@ -177,7 +177,7 @@ static inline void disable_acpi(void) { }
>>    #define ARCH_HAS_POWER_INIT	1
>>    
>>    #ifdef CONFIG_ACPI_NUMA
>> -extern int acpi_numa;
>> +extern void disable_acpi_numa(void);
>>    extern int x86_acpi_numa_init(void);
>>    #endif /* CONFIG_ACPI_NUMA */
>>    
>> diff --git a/arch/x86/kernel/acpi/srat.c b/arch/x86/kernel/acpi/srat.c
>> index cdd0da9..8d7f3f8 100644
>> --- a/arch/x86/kernel/acpi/srat.c
>> +++ b/arch/x86/kernel/acpi/srat.c
>> @@ -24,22 +24,27 @@
>>    #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 disable_acpi_numa(void)
>>    {
>> -	printk(KERN_ERR "SRAT: SRAT not used.\n");
>> -	acpi_numa = -1;
>> +	acpi_numa = false;
>>    }
>>    
>> -static __init inline int srat_disabled(void)
>> +static void __init bad_srat(void)
>>    {
>> -	return acpi_numa < 0;
>> +	disable_acpi_numa();
>> +	printk(KERN_ERR "SRAT: SRAT will not be used.\n");
>> +}
>> +

>> +static bool __init srat_disabled(void)
>> +{
>> +	return acpi_numa;

return !acpi_numa

Thanks,
Yasuaki Ishimatsu

>>    }
>>    
>>    /* Callback for SLIT parsing */
>> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
>> index 245a4ba..1ebc907 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;
>> +		disable_acpi_numa();
>>    #endif
>>    	return 0;
>>    }
>> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>> index 657eca9..3636bc6 100644
>> --- a/arch/x86/xen/enlighten.c
>> +++ b/arch/x86/xen/enlighten.c
>> @@ -1447,7 +1447,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;
>> +	disable_acpi_numa();
>>    #endif
>>    
>>    	/* Don't do the full vcpu_info placement stuff until we have a
>>
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>