Re: [PATCH 2/3] acpi, memory-hotplug: Extend movablemem_map rangesto the end of node.

From: Tang Chen
Date: Sun Jan 27 2013 - 20:54:23 EST


Hi Andrew,

On 01/26/2013 08:36 AM, Andrew Morton wrote:

The patch generates a bunch of rejects, partly due to linux-next
changes but I think I fixed everything up OK.

Thank you for your fixing. :)


index 4ddf497..f841d0e 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -141,11 +141,16 @@ static inline int save_add_info(void) {return 1;}
static inline int save_add_info(void) {return 0;}
#endif

+#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
+extern struct movablemem_map movablemem_map;
+#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */

Well.

a) we shouldn't put extern declarations in C files - put them in
headers so we can be assured that all compilation units agree on the
type.

OK, got it. :)


b) the ifdefs are unneeded - a unused extern declaration is OK (as
long as the type itself is always defined!)

I think struct movablemem_map is defined in mm.h, protected by CONFIG_HAVE_MEMBLOCK_NODE_MAP.
So the declaration needs to be protected by CONFIG_HAVE_MEMBLOCK_NODE_MAP.


c) movablemem_map is already declared in memblock.h.

Hum, yes, it is defined in mm.h, included by memblock.h :)


So I zapped the above three lines.

@@ -178,9 +185,57 @@ 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",
+ printk(KERN_INFO "SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx] %s\n",
node, pxm,
- (unsigned long long) start, (unsigned long long) end - 1);
+ (unsigned long long) start, (unsigned long long) end - 1,
+ hotpluggable ? "Hot Pluggable": "");
+
+#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
+ int overlap;
+ unsigned long start_pfn, end_pfn;

no, we don't put declarations of locals in the middle of C statements
like this:

arch/x86/mm/srat.c: In function 'acpi_numa_memory_affinity_init':
arch/x86/mm/srat.c:185: warning: ISO C90 forbids mixed declarations and code

Did your compiler not emit this warning?

Sorry, I think it did, but I missed it.
THanks for fixing that. :)


I fixed this by moving the code into a new function
"handle_movablemem". Feel free to suggest a more appropriate name!


From: Andrew Morton<akpm@xxxxxxxxxxxxxxxxxxxx>
Subject: acpi-memory-hotplug-extend-movablemem_map-ranges-to-the-end-of-node-fix

clean up code, fix build warning

Cc: "Brown, Len"<len.brown@xxxxxxxxx>
Cc: "H. Peter Anvin"<hpa@xxxxxxxxx>
Cc: Ingo Molnar<mingo@xxxxxxx>
Cc: Jiang Liu<jiang.liu@xxxxxxxxxx>
Cc: Jianguo Wu<wujianguo@xxxxxxxxxx>
Cc: KOSAKI Motohiro<kosaki.motohiro@xxxxxxxxxxxxxx>
Cc: Kamezawa Hiroyuki<kamezawa.hiroyu@xxxxxxxxxxxxxx>
Cc: Lai Jiangshan<laijs@xxxxxxxxxxxxxx>
Cc: Len Brown<lenb@xxxxxxxxxx>
Cc: Tang Chen<tangchen@xxxxxxxxxxxxxx>
Cc: Thomas Gleixner<tglx@xxxxxxxxxxxxx>
Cc: Wu Jianguo<wujianguo@xxxxxxxxxx>
Cc: Yasuaki Ishimatsu<isimatu.yasuaki@xxxxxxxxxxxxxx>
Signed-off-by: Andrew Morton<akpm@xxxxxxxxxxxxxxxxxxxx>
---

arch/x86/mm/srat.c | 93 ++++++++++++++++++++++---------------------
1 file changed, 49 insertions(+), 44 deletions(-)

diff -puN arch/x86/mm/srat.c~acpi-memory-hotplug-extend-movablemem_map-ranges-to-the-end-of-node-fix arch/x86/mm/srat.c
--- a/arch/x86/mm/srat.c~acpi-memory-hotplug-extend-movablemem_map-ranges-to-the-end-of-node-fix
+++ a/arch/x86/mm/srat.c
@@ -142,50 +142,8 @@ static inline int save_add_info(void) {r
#endif

#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
-extern struct movablemem_map movablemem_map;
-#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
-
-/* Callback for parsing of the Proximity Domain<-> Memory Area mappings */
-int __init
-acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
+static void __init handle_movablemem(int node, u64 start, u64 end)
{
- u64 start, end;
- u32 hotpluggable;
- 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;
- hotpluggable = ma->flags& ACPI_SRAT_MEM_HOT_PLUGGABLE;
- if (hotpluggable&& !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] %s\n",
- node, pxm,
- (unsigned long long) start, (unsigned long long) end - 1,
- hotpluggable ? "Hot Pluggable": "");
-
-#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
int overlap;
unsigned long start_pfn, end_pfn;

@@ -229,7 +187,54 @@ acpi_numa_memory_affinity_init(struct ac
*/
insert_movablemem_map(start_pfn, end_pfn);
}
-#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
+}
+#else /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
+static inline void handle_movablemem(int node, u64 start, u64 end)
+{
+}
+#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
+
+/* 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;
+ u32 hotpluggable;
+ 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;
+ hotpluggable = ma->flags& ACPI_SRAT_MEM_HOT_PLUGGABLE;
+ if (hotpluggable&& !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] %s\n",
+ node, pxm,
+ (unsigned long long) start, (unsigned long long) end - 1,
+ hotpluggable ? "Hot Pluggable": "");
+
+ handle_movablemem(node, start, end);

return 0;
out_err_bad_srat:
diff -puN include/linux/mm.h~acpi-memory-hotplug-extend-movablemem_map-ranges-to-the-end-of-node-fix include/linux/mm.h
diff -puN mm/page_alloc.c~acpi-memory-hotplug-extend-movablemem_map-ranges-to-the-end-of-node-fix mm/page_alloc.c
_


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/