i386: srat and numaq cleanup

Cleanup the i386 NUMA code by creating inline no-op functions for get_memcfg_numaq/srat() and get_zholes_size_numaq/srat().

---

Applies on top of linux-2.6.14-rc2-git8-mhp1

 arch/i386/kernel/srat.c   |   10 ++++++++--
 include/asm-i386/mmzone.h |   26 +++++++++++++++++---------
 include/asm-i386/numaq.h  |   10 ++++++++--
 include/asm-i386/srat.h   |   15 ++++++++++-----
 4 files changed, 43 insertions(+), 18 deletions(-)

@@ -56,6 +56,7 @@ struct node_memory_chunk_s {
 static struct node_memory_chunk_s node_memory_chunk[MAXCHUNKS];
 
 static int num_memory_chunks;		/* total number of memory chunks */
+static int has_srat;
 static int zholes_size_init;
 static unsigned long zholes_size[MAX_NUMNODES * MAX_NR_ZONES];
 

@@ -317,7 +318,7 @@ out_fail:
 	return 0;
 }
 
-int __init get_memcfg_from_srat(void)
+int __init get_memcfg_srat(void)
 {
 	struct acpi_table_header *header = NULL;
 	struct acpi_table_rsdp *rsdp = NULL;

@@ -403,6 +404,8 @@ int __init get_memcfg_from_srat(void)
 			continue;
 
 		/* we've found the srat table. don't need to look at any more tables */
+		has_srat = 1;
+
 		return acpi20_parse_srat((struct acpi_table_srat *)header);
 	}
 out_err:

@@ -449,8 +452,11 @@ static void __init get_zholes_init(void)
 	}
 }
 
-unsigned long * __init get_zholes_size(int nid)
+unsigned long * __init get_zholes_size_srat(int nid)
 {
+	if (!has_srat)
+		return NULL;
+
 	if (!zholes_size_init) {
 		zholes_size_init++;
 		get_zholes_init();

@@ -12,11 +12,8 @@
 extern struct pglist_data *node_data[];
 #define NODE_DATA(nid)	(node_data[nid])
 
-#ifdef CONFIG_X86_NUMAQ
-	#include <asm/numaq.h>
-#else	/* summit or generic arch */
-	#include <asm/srat.h>
-#endif
+#include <asm/numaq.h>
+#include <asm/srat.h>
 
 extern int get_memcfg_numa_flat(void );
 /*

@@ -26,17 +23,28 @@ extern int get_memcfg_numa_flat(void );
  */
 static inline void get_memcfg_numa(void)
 {
-#ifdef CONFIG_X86_NUMAQ
 	if (get_memcfg_numaq())
 		return;
-#elif defined(CONFIG_ACPI_SRAT)
-	if (get_memcfg_from_srat())
+
+	if (get_memcfg_srat())
 		return;
-#endif
 
 	get_memcfg_numa_flat();
 }
 
+static inline unsigned long *get_zholes_size(int nid)
+{
+	unsigned long *ret;
+
+	if ((ret = get_zholes_size_numaq(nid)))
+		return ret;
+
+	if ((ret = get_zholes_size_srat(nid)))
+		return ret;
+
+	return NULL;
+}
+
 extern int early_pfn_to_nid(unsigned long pfn);
 extern void __init remap_numa_kva(void);
 extern unsigned long calculate_numa_remap_pages(void);

@@ -155,10 +155,16 @@ struct sys_cfg_data {
         struct	eachquadmem eq[MAX_NUMNODES];	/* indexed by quad id */
 };
 
-static inline unsigned long *get_zholes_size(int nid)
+#else /* CONFIG_X86_NUMAQ */
+
+static inline int get_memcfg_numaq(void) { return 0; }
+
+#endif /* CONFIG_X86_NUMAQ */
+
+static inline unsigned long *get_zholes_size_numaq(int nid)
 {
 	return NULL;
 }
-#endif /* CONFIG_X86_NUMAQ */
+
 #endif /* NUMAQ_H */
 

@@ -27,11 +27,16 @@
 #ifndef _ASM_SRAT_H_
 #define _ASM_SRAT_H_
 
-#ifndef CONFIG_ACPI_SRAT
-#error CONFIG_ACPI_SRAT not defined, and srat.h header has been included
-#endif
+#ifdef CONFIG_ACPI_SRAT
 
-extern int get_memcfg_from_srat(void);
-extern unsigned long *get_zholes_size(int);
+extern int get_memcfg_srat(void);
+extern unsigned long *get_zholes_size_srat(int);
+
+#else /* CONFIG_ACPI_SRAT */
+
+static inline int get_memcfg_srat(void) { return 0; }
+static inline unsigned long *get_zholes_size_srat(int nid) { return NULL; }
+
+#endif /* CONFIG_ACPI_SRAT */
 
 #endif /* _ASM_SRAT_H_ */

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I'm highly suspicious of any "cleanup" that adds more code than it deletes. What does this clean up?

This patch is a little bit confused. It makes the get_zholes_size_srat() always safe to call at runtime. However, it still creates a compile-time stub version of it as well.

In addition, you already have the srat.c-local zholes_size_init, but you still add the has_srat variable. Seems a bit superfluous.

Calling get_zholes_size_numaq() at runtime is unnecessary. The NUMA-Q is not supported with the ARCH_GENERIC code.

-- Dave

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hehe, I realized that it added code when I generated the diffstat, but I believe that the code gets cleaner.

The patch removes #ifdefs from get_memcfg_numa() and introduces an inline get_zholes_size(). The #ifdefs are moved down one level to the files srat.h and numaq.h and empty inline functions are added. These empty inline function are probably the reason for the added lines.

Without this patch get_zholes_size() is defined in either srat.c or numaq.h. With this patch get_zholes_size() is turned into an inline function like get_memcfg_numa(), and this function calls all machine-specific implementations.

This new behavior requires that the machine-specific code keeps track if it has been initialized or not, and only lets get_zholes_size_xxx() return non-NULL if initialized.

Nah, zholes_size_init is used to call get_zholes_init() just once. has_srat is used to determine if get_memcfg_numa() did succeed.

Yes. But does this patch do anything that makes that harder?

Another side effect of the patch is that get_zholes_size() always gets defined, regardless of if numaq or srat is enabled or not. That is useful for the NUMA emulation code.

Thanks,

/ magnus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

It does remove two #ifdefs, but it adds two #else blocks in other places.

I also noticed that acpi20_parse_srat() can fail. So, has_srat may belong in that function, not in get_memcfg_from_srat()

Why ever have this block?

Yes, that is better.

That is correct. I just kept it there to make the srat and numaq code more similar, but I'd be happy to remove it. If you still consider this as a cleanup, please let me know and I will generate a new patch.

Thanks,

/ magnus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I'd pull it. No need to add new code just for parity.

Thanks,

-- Dave

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>