This patch-set fix the following problems:
1. Kill useless function save_add_info() which will block us from using
numa when MEMORY_HOTPLUG is not configured.
2. acpi_table_parse() didn't check if @id is NULL.
3. Fix incorrect comment in acpi_table_parse(), and return -ENOENT if a
table is not found.
And also did some cleanup.
Tang Chen (6):
acpi, numa, mem_hotplug: Kill save_add_info().
acpi, numa, mem_hotplug: Print Hot-Pluggable Field in SRAT.
acpi cleanup: Use pr_info() instead of printk() in arch/x86/mm/srat.c
acpi cleanup: Use pr_err() instead of printk() in arch/x86/mm/srat.c
acpi: Check if @id is NULL in acpi_table_parse()
acpi: Return -ENOENT in acpi_table_parse() and fix wrong comment.
arch/x86/mm/srat.c | 38 +++++++++++++++++---------------------
drivers/acpi/tables.c | 9 +++++----
2 files changed, 22 insertions(+), 25 deletions(-)
Use pr_info() instead of printk() in arch/x86/mm/srat.c.
Signed-off-by: Tang Chen <[email protected]>
---
arch/x86/mm/srat.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index 71411aa..6286e89 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -71,8 +71,8 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
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);
+ pr_info("SRAT: PXM %u -> X2APIC 0x%04x ignored\n",
+ pxm, apic_id);
return;
}
node = setup_node(pxm);
@@ -83,13 +83,13 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
}
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);
+ pr_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",
+ pr_info("SRAT: PXM %u -> APIC 0x%04x -> Node %u\n",
pxm, apic_id, node);
}
@@ -124,14 +124,14 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
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);
+ pr_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",
+ pr_info("SRAT: PXM %u -> APIC 0x%02x -> Node %u\n",
pxm, apic_id, node);
}
--
1.7.1
strncmp() does not check if the params are NULL. In acpi_table_parse(),
if @id is NULL, the kernel will panic.
Signed-off-by: Tang Chen <[email protected]>
---
drivers/acpi/tables.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index d67a1fe..5a5263b 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -293,7 +293,7 @@ int __init acpi_table_parse(char *id, acpi_tbl_table_handler handler)
if (acpi_disabled)
return -ENODEV;
- if (!handler)
+ if (!id || !handler)
return -EINVAL;
if (strncmp(id, ACPI_SIG_MADT, 4) == 0)
--
1.7.1
Use pr_err() instead of printk() in arch/x86/mm/srat.c
Signed-off-by: Tang Chen <[email protected]>
---
arch/x86/mm/srat.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index 6286e89..32b9597 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -33,7 +33,7 @@ static __init int setup_node(int pxm)
static __init void bad_srat(void)
{
- printk(KERN_ERR "SRAT: SRAT not used.\n");
+ pr_err("SRAT: SRAT not used.\n");
acpi_numa = -1;
}
@@ -77,7 +77,7 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
}
node = setup_node(pxm);
if (node < 0) {
- printk(KERN_ERR "SRAT: Too many proximity domains %x\n", pxm);
+ pr_err("SRAT: Too many proximity domains %x\n", pxm);
bad_srat();
return;
}
@@ -113,7 +113,7 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
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);
+ pr_err("SRAT: Too many proximity domains %x\n", pxm);
bad_srat();
return;
}
@@ -160,7 +160,7 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
node = setup_node(pxm);
if (node < 0) {
- printk(KERN_ERR "SRAT: Too many proximity domains.\n");
+ pr_err("SRAT: Too many proximity domains.\n");
goto out_err_bad_srat;
}
--
1.7.1
The comment about return value of acpi_table_parse() is incorrect.
This patch fix it.
Furthermore, if the table is not found, return 1 means nothing, and
make it difficult to write the comment. So return -ENOENT when the
table is not found, and correct the comment.
Signed-off-by: Tang Chen <[email protected]>
---
drivers/acpi/tables.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 5a5263b..e6de24f 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -278,12 +278,13 @@ acpi_table_parse_madt(enum acpi_madt_type id,
/**
* acpi_table_parse - find table with @id, run @handler on it
- *
* @id: table id to find
* @handler: handler to run
*
* Scan the ACPI System Descriptor Table (STD) for a table matching @id,
- * run @handler on it. Return 0 if table found, return on if not.
+ * run @handler on it.
+ *
+ * Return 0 on success, -errno on failure.
*/
int __init acpi_table_parse(char *id, acpi_tbl_table_handler handler)
{
@@ -306,7 +307,7 @@ int __init acpi_table_parse(char *id, acpi_tbl_table_handler handler)
early_acpi_os_unmap_memory(table, tbl_size);
return 0;
} else
- return 1;
+ return -ENOENT;
}
/*
--
1.7.1
save_add_info() is defined as:
#ifdef CONFIG_MEMORY_HOTPLUG
static inline int save_add_info(void) {return 1;}
#else
static inline int save_add_info(void) {return 0;}
#endif
which means it is true when memory hotplug is configured.
In acpi_numa_memory_affinity_init(), it checks the memory hotplug
flag in SRAT memory affinity and save_add_info() like this:
if ((ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) && !save_add_info())
goto out_err;
......
node = setup_node(pxm);
numa_add_memblk(node, start, end);
......
which means if the memory range is hotpluggable, but memory hotplug is not
configured, it won't add these memory to numa_meminfo.
After this, numa_meminfo_cover_memory() will fail, which will finally cause
numa_init() to fail.
numa_init()
|->numa_register_memblks()
|->numa_meminfo_cover_memory()
When numa_init() fails, it will fallback to numa_init(dummy_numa_init), and
all numa architecture will not be setup.
This is nonsense. Even if memory hotplug is not configured, we can also use
numa architecture.
Actually, save_add_info() is added by commit 71efa8fdc55e70ec6687c897a30759f0a2c2ad7e
in 2006. And now it is useless.
So this patch kill save_add_info() and the nonsense checking.
Signed-off-by: Tang Chen <[email protected]>
---
arch/x86/mm/srat.c | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index cdd0da9..1613c02 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -135,12 +135,6 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
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)
@@ -154,8 +148,6 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
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;
--
1.7.1
The Hot-Pluggable field in SRAT suggests if the memory could be
hotplugged while the system is running. Print it as well when
parsing SRAT will help users to know which memory is hotpluggable.
Signed-off-by: Tang Chen <[email protected]>
Reviewed-by: Wanpeng Li <[email protected]>
Reviewed-by: Zhang Yanfei <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
arch/x86/mm/srat.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index 1613c02..71411aa 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -141,6 +141,7 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
{
u64 start, end;
int node, pxm;
+ u32 hotpluggable;
if (srat_disabled())
goto out_err;
@@ -152,6 +153,8 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
start = ma->base_address;
end = start + ma->length;
pxm = ma->proximity_domain;
+ hotpluggable = ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE;
+
if (acpi_srat_revision <= 1)
pxm &= 0xff;
@@ -166,9 +169,10 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
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);
+ pr_info("SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]%s\n",
+ node, pxm,
+ (unsigned long long) start, (unsigned long long) end - 1,
+ hotpluggable ? " hotplug" : "");
return 0;
out_err_bad_srat:
--
1.7.1
On Fri, 2013-08-16 at 15:06 +0800, Tang Chen wrote:
> arch/x86/mm/srat.c
I think it'd be better to use pr_fmt
with the conversions to pr_info and pr_err.
pr_fmt can prefix the appropriate srat: and
so the format strings do not need it.
Something like:
---
arch/x86/mm/srat.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index cdd0da9..350b4c5 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -9,6 +9,8 @@
* are in one chunk. Holes between them will be included in the node.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/kernel.h>
#include <linux/acpi.h>
#include <linux/mmzone.h>
@@ -33,7 +35,7 @@ static __init int setup_node(int pxm)
static __init void bad_srat(void)
{
- printk(KERN_ERR "SRAT: SRAT not used.\n");
+ pr_err("SRAT not used\n");
acpi_numa = -1;
}
@@ -71,26 +73,25 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
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);
+ pr_info("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);
+ pr_err("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);
+ pr_info("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);
+ pr_info("PXM %u -> APIC 0x%04x -> Node %u\n", pxm, apic_id, node);
}
/* Callback for Proximity Domain -> LAPIC mapping */
@@ -113,7 +114,7 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
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);
+ pr_err("Too many proximity domains %x\n", pxm);
bad_srat();
return;
}
@@ -124,15 +125,15 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
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);
+ pr_info("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);
+ pr_info("PXM %u -> APIC 0x%02x -> Node %u\n", pxm, apic_id, node);
}
#ifdef CONFIG_MEMORY_HOTPLUG
@@ -165,7 +166,7 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
node = setup_node(pxm);
if (node < 0) {
- printk(KERN_ERR "SRAT: Too many proximity domains.\n");
+ pr_err("Too many proximity domains\n");
goto out_err_bad_srat;
}
@@ -174,9 +175,9 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
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);
+ pr_info("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:
Hi Joe,
On 08/16/2013 03:25 PM, Joe Perches wrote:
> On Fri, 2013-08-16 at 15:06 +0800, Tang Chen wrote:
>> arch/x86/mm/srat.c
>
> I think it'd be better to use pr_fmt
> with the conversions to pr_info and pr_err.
>
> pr_fmt can prefix the appropriate srat: and
> so the format strings do not need it.
>
> Something like:
> ---
> arch/x86/mm/srat.c | 31 ++++++++++++++++---------------
> 1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
> index cdd0da9..350b4c5 100644
> --- a/arch/x86/mm/srat.c
> +++ b/arch/x86/mm/srat.c
> @@ -9,6 +9,8 @@
> * are in one chunk. Holes between them will be included in the node.
> */
>
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
OK, will update the patches.
Thanks. :)
Use pr_info() instead of printk() in arch/x86/mm/srat.c.
As Joe suggested, define pr_fmt(fmt) as KBUILD_MODNAME ": " fmt to
prefix message with "srat: ".
Signed-off-by: Tang Chen <[email protected]>
Signed-off-by: Joe Perches <[email protected]>
---
arch/x86/mm/srat.c | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index 71411aa..591f4bb 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -24,6 +24,8 @@
#include <asm/apic.h>
#include <asm/uv/uv.h>
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
int acpi_numa __initdata;
static __init int setup_node(int pxm)
@@ -71,8 +73,8 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
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);
+ pr_info("PXM %u -> X2APIC 0x%04x ignored\n",
+ pxm, apic_id);
return;
}
node = setup_node(pxm);
@@ -83,13 +85,13 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
}
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);
+ pr_info("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",
+ pr_info("PXM %u -> APIC 0x%04x -> Node %u\n",
pxm, apic_id, node);
}
@@ -124,14 +126,14 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
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);
+ pr_info("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",
+ pr_info("PXM %u -> APIC 0x%02x -> Node %u\n",
pxm, apic_id, node);
}
@@ -169,8 +171,7 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
node_set(node, numa_nodes_parsed);
- pr_info("SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]%s\n",
- node, pxm,
+ pr_info("Node %u PXM %u [mem %#010Lx-%#010Lx]%s\n", node, pxm,
(unsigned long long) start, (unsigned long long) end - 1,
hotpluggable ? " hotplug" : "");
--
1.7.1
Use pr_err() instead of printk() in arch/x86/mm/srat.c
Signed-off-by: Tang Chen <[email protected]>
Signed-off-by: Joe Perches <[email protected]>
---
arch/x86/mm/srat.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index 591f4bb..d1e3b95 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -35,7 +35,7 @@ static __init int setup_node(int pxm)
static __init void bad_srat(void)
{
- printk(KERN_ERR "SRAT: SRAT not used.\n");
+ pr_err("SRAT not used.\n");
acpi_numa = -1;
}
@@ -79,7 +79,7 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
}
node = setup_node(pxm);
if (node < 0) {
- printk(KERN_ERR "SRAT: Too many proximity domains %x\n", pxm);
+ pr_err("Too many proximity domains %x\n", pxm);
bad_srat();
return;
}
@@ -115,7 +115,7 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
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);
+ pr_err("Too many proximity domains %x\n", pxm);
bad_srat();
return;
}
@@ -162,7 +162,7 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
node = setup_node(pxm);
if (node < 0) {
- printk(KERN_ERR "SRAT: Too many proximity domains.\n");
+ pr_err("Too many proximity domains.\n");
goto out_err_bad_srat;
}
--
1.7.1
On Fri, 2013-08-16 at 15:06 +0800, Tang Chen wrote:
> save_add_info() is defined as:
>
> #ifdef CONFIG_MEMORY_HOTPLUG
> static inline int save_add_info(void) {return 1;}
> #else
> static inline int save_add_info(void) {return 0;}
> #endif
>
> which means it is true when memory hotplug is configured.
>
> In acpi_numa_memory_affinity_init(), it checks the memory hotplug
> flag in SRAT memory affinity and save_add_info() like this:
>
> if ((ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) && !save_add_info())
> goto out_err;
> ......
> node = setup_node(pxm);
> numa_add_memblk(node, start, end);
> ......
>
> which means if the memory range is hotpluggable, but memory hotplug is not
> configured, it won't add these memory to numa_meminfo.
>
> After this, numa_meminfo_cover_memory() will fail, which will finally cause
> numa_init() to fail.
>
> numa_init()
> |->numa_register_memblks()
> |->numa_meminfo_cover_memory()
>
> When numa_init() fails, it will fallback to numa_init(dummy_numa_init), and
> all numa architecture will not be setup.
>
> This is nonsense. Even if memory hotplug is not configured, we can also use
> numa architecture.
>
> Actually, save_add_info() is added by commit 71efa8fdc55e70ec6687c897a30759f0a2c2ad7e
> in 2006. And now it is useless.
>
> So this patch kill save_add_info() and the nonsense checking.
>
> Signed-off-by: Tang Chen <[email protected]>
Good catch!
Acked-by: Toshi Kani <[email protected]>
Thanks,
-Toshi
On Fri, 2013-08-16 at 15:06 +0800, Tang Chen wrote:
> The Hot-Pluggable field in SRAT suggests if the memory could be
> hotplugged while the system is running. Print it as well when
> parsing SRAT will help users to know which memory is hotpluggable.
>
> Signed-off-by: Tang Chen <[email protected]>
> Reviewed-by: Wanpeng Li <[email protected]>
> Reviewed-by: Zhang Yanfei <[email protected]>
> Acked-by: Tejun Heo <[email protected]>
Acked-by: Toshi Kani <[email protected]>
Thanks,
-Toshi
On Fri, 2013-08-16 at 15:06 +0800, Tang Chen wrote:
> Use pr_info() instead of printk() in arch/x86/mm/srat.c.
>
> Signed-off-by: Tang Chen <[email protected]>
Please fold patch 4/6 into this patch 3/6 since they both are printk()
cleanup on the same file. I do not see why they need to be separated.
With that change,
Acked-by: Toshi Kani <[email protected]>
Thanks,
-Toshi
On Fri, 2013-08-16 at 15:06 +0800, Tang Chen wrote:
> strncmp() does not check if the params are NULL. In acpi_table_parse(),
> if @id is NULL, the kernel will panic.
>
> Signed-off-by: Tang Chen <[email protected]>
Acked-by: Toshi Kani <[email protected]>
-Toshi
> ---
> drivers/acpi/tables.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> index d67a1fe..5a5263b 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -293,7 +293,7 @@ int __init acpi_table_parse(char *id, acpi_tbl_table_handler handler)
> if (acpi_disabled)
> return -ENODEV;
>
> - if (!handler)
> + if (!id || !handler)
> return -EINVAL;
>
> if (strncmp(id, ACPI_SIG_MADT, 4) == 0)
On Fri, 2013-08-16 at 15:06 +0800, Tang Chen wrote:
> The comment about return value of acpi_table_parse() is incorrect.
> This patch fix it.
>
> Furthermore, if the table is not found, return 1 means nothing, and
> make it difficult to write the comment. So return -ENOENT when the
> table is not found, and correct the comment.
I am OK with the change, but the above description is not very clear.
You should state that all callers only check if the function succeeded
or not. So, you are simplifying the semantics by returning -errno for
all failure cases.
Since you are making this change, I'd suggest you also update the stub
function in linux/acpi.h to return -ENODEV as well.
Thanks,
-Toshi
On 08/20/2013 03:29 AM, Toshi Kani wrote:
> On Fri, 2013-08-16 at 15:06 +0800, Tang Chen wrote:
>> The comment about return value of acpi_table_parse() is incorrect.
>> This patch fix it.
>>
>> Furthermore, if the table is not found, return 1 means nothing, and
>> make it difficult to write the comment. So return -ENOENT when the
>> table is not found, and correct the comment.
>
> I am OK with the change, but the above description is not very clear.
> You should state that all callers only check if the function succeeded
> or not. So, you are simplifying the semantics by returning -errno for
> all failure cases.
>
> Since you are making this change, I'd suggest you also update the stub
> function in linux/acpi.h to return -ENODEV as well.
OK, followed. And will merge patch 3 and 4 and resend them later.
Thanks for reviewing.