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<a...@linux-foundation.org>
Subject: acpi-memory-hotplug-extend-movablemem_map-ranges-to-the-end-of-node-fix

clean up code, fix build warning

Cc: "Brown, Len"<len.br...@intel.com>
Cc: "H. Peter Anvin"<h...@zytor.com>
Cc: Ingo Molnar<mi...@elte.hu>
Cc: Jiang Liu<jiang....@huawei.com>
Cc: Jianguo Wu<wujian...@huawei.com>
Cc: KOSAKI Motohiro<kosaki.motoh...@jp.fujitsu.com>
Cc: Kamezawa Hiroyuki<kamezawa.hir...@jp.fujitsu.com>
Cc: Lai Jiangshan<la...@cn.fujitsu.com>
Cc: Len Brown<l...@kernel.org>
Cc: Tang Chen<tangc...@cn.fujitsu.com>
Cc: Thomas Gleixner<t...@linutronix.de>
Cc: Wu Jianguo<wujian...@huawei.com>
Cc: Yasuaki Ishimatsu<isimatu.yasu...@jp.fujitsu.com>
Signed-off-by: Andrew Morton<a...@linux-foundation.org>
---

  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to