MAX_NR_NODES vs. MAX_NUMNODES
Subsystems (5)
RFC PATCH 1/1 MAX_NR_NODES vs. MAX_NUMNODES
There are two #defines to keep track of the maximum number of nodes in the system, and luckily they don't have even remotely similar values!! :( MAX_NR_NODES is about 85, and MAX_NUMNODES is 1, 4, 8, or 16 depending on config options and architectures. Maybe this confuses only me, in which case, please ignore this patch. For those who'd like only one definition of the maximum number of nodes in a system, read on!
This patch
1) Gets rid of the include/asm-xxx/numnodes.h files
2) Defines MAX_NR_NODES to the appropriate per-arch value in include/asm-xxx/param.h
3) changes all remaining occurences MAX_NUMNODES to MAX_NR_NODES throughout the kernel
I chose to keep MAX_NR_NODES because it (more) closely resembles NR_CPUS, and MAX_NR_MEMBLKS. (Frankly, I'd like to s/NR_CPUS/MAX_NR_CPUS for correctness, but I'd be lynched!! ;)
Anyone who is more familiar with some of the architectures I mucked with (arm, alpha, ppc64), please let me know if what I've done looks wrong.
Cheers!
-Matt
arch/alpha/mm/numa.c +3 -3
@@ -19,8 +19,8 @@#include <asm/hwrpb.h> #include <asm/pgalloc.h> -plat_pg_data_t *plat_node_data[MAX_NUMNODES ]; -bootmem_data_t plat_node_bdata[MAX_NUMNODES ]; +plat_pg_data_t *plat_node_data[MAX_NR_NODES ]; +bootmem_data_t plat_node_bdata[MAX_NR_NODES ];#undef DEBUG_DISCONTIG#ifdef DEBUG_DISCONTIG
@@ -243,7 +243,7 @@show_mem_layout (); numnodes =0 ; -for (nid =0 ; nid <MAX_NUMNODES ; nid++) +for (nid =0 ; nid <MAX_NR_NODES ; nid++)setup_memory_node (nid, kernel_end);#ifdef CONFIG_BLK_DEV_INITRD
arch/arm/mm/discontig.c +3 -3
@@ -15,7 +15,7 @@#include <linux/init.h> #include <linux/bootmem.h> -#if NR_NODES !=4 +#if MAX_NR_NODES !=4 #error Fix Me Please#endif
@@ -23,9 +23,9 @@* Our node_data structure for discontigous memory. */ -static bootmem_data_t node_bootmem_data[NR_NODES ]; +static bootmem_data_t node_bootmem_data[MAX_NR_NODES ]; -pg_data_t discontig_node_data[NR_NODES ] = { +pg_data_t discontig_node_data[MAX_NR_NODES ] = { { bdata: &node_bootmem_data[0 ] }, { bdata: &node_bootmem_data[1 ] }, { bdata: &node_bootmem_data[2 ] },
arch/arm/mm/init.c +3 -9
@@ -34,12 +34,6 @@#include <asm/mach/arch.h> #include <asm/mach/map.h> -#ifndef CONFIG_DISCONTIGMEM -#define NR_NODES 1 -#else -#define NR_NODES 4 -#endif -#ifdef CONFIG_CPU_32 #define TABLE_OFFSET (PTRS_PER_PTE)#else
@@ -179,7 +173,7 @@ {unsigned int i, bootmem_pages =0 , memend_pfn =0 ; -for (i =0 ; i <NR_NODES ; i++) { +for (i =0 ; i <MAX_NR_NODES ; i++) { np[i].start =-1U ; np[i].end =0 ; np[i].bootmap_pages =0 ;
@@ -208,7 +202,7 @@* we have, we're in trouble. (maybe we ought to * limit, instead of bugging?) */ -if (numnodes >NR_NODES ) +if (numnodes >MAX_NR_NODES )BUG (); }
@@ -366,7 +360,7 @@*/ void __init bootmem_init (struct meminfo *mi) { -struct node_info node_info[NR_NODES ], *np = node_info; +struct node_info node_info[MAX_NR_NODES ], *np = node_info;unsigned int bootmap_pages, bootmap_pfn, map_pg;int node, initrd_node;
arch/i386/kernel/numaq.c +6 -3
@@ -30,8 +30,8 @@#include <asm/numaq.h> /* These are needed before the pgdat's are created */ -unsigned long node_start_pfn[MAX_NUMNODES ]; -unsigned long node_end_pfn[MAX_NUMNODES ]; +unsigned long node_start_pfn[MAX_NR_NODES ]; +unsigned long node_end_pfn[MAX_NR_NODES ];#define MB_TO_PAGES (addr) ((addr) << (20 - PAGE_SHIFT))
@@ -50,7 +50,10 @@ (struct sys_cfg_data *)__va (SYS_CFG_DATA_PRIV_ADDR ); numnodes =0 ; -for (node =0 ; node <MAX_NUMNODES ; node++) { +/* This should be fixed. It is doing bitwise arithmetic with a 32 +* bit field, but the bounds were not being checked. +*/ +for (node =0 ; node <MAX_NR_NODES ; node++) {if (scd->quads_present31_0 & (1 << node)) { numnodes++; eq = &scd->eq[node];
arch/i386/mm/discontig.c +5 -5
@@ -33,7 +33,7 @@#include <asm/e820.h> #include <asm/setup.h> -struct pglist_data *node_data[MAX_NUMNODES ]; +struct pglist_data *node_data[MAX_NR_NODES ];bootmem_data_t node0_bdata;extern unsigned long find_max_low_pfn (void );
@@ -115,10 +115,10 @@#define LARGE_PAGE_BYTES (PTRS_PER_PTE * PAGE_SIZE) -unsigned long node_remap_start_pfn[MAX_NUMNODES ]; -unsigned long node_remap_size[MAX_NUMNODES ]; -unsigned long node_remap_offset[MAX_NUMNODES ]; -void *node_remap_start_vaddr[MAX_NUMNODES ]; +unsigned long node_remap_start_pfn[MAX_NR_NODES ]; +unsigned long node_remap_size[MAX_NR_NODES ]; +unsigned long node_remap_offset[MAX_NR_NODES ]; +void *node_remap_start_vaddr[MAX_NR_NODES ];extern void set_pmd_pfn (unsigned long vaddr,unsigned long pfn,pgprot_t flags);void __init remap_numa_kva (void )
arch/ppc64/mm/init.c +1 -1
@@ -522,7 +522,7 @@ {int nid; -for (nid =0 ; nid <MAX_NUMNODES ; nid++) { +for (nid =0 ; nid <MAX_NR_NODES ; nid++) {if (numa_node_exists[nid]) {printk ("freeing bootmem node %x\n" , nid); totalram_pages +=
arch/ppc64/mm/numa.c +5 -5
@@ -24,10 +24,10 @@int numa_cpu_lookup_table[NR_CPUS ] = { [0 ... (NR_CPUS -1 )] =-1 };int numa_memory_lookup_table[MAX_MEMORY >>MEMORY_INCREMENT_SHIFT ] = { [0 ... ((MAX_MEMORY >>MEMORY_INCREMENT_SHIFT ) -1 )] =-1 }; -int numa_node_exists[MAX_NUMNODES ]; +int numa_node_exists[MAX_NR_NODES ]; -struct pglist_data node_data[MAX_NUMNODES ]; -bootmem_data_t plat_node_bdata[MAX_NUMNODES ]; +struct pglist_data node_data[MAX_NR_NODES ]; +bootmem_data_t plat_node_bdata[MAX_NR_NODES ];static int __init parse_numa_properties (void ) {
@@ -45,7 +45,7 @@if (parse_numa_properties ())BUG (); -for (nid =0 ; nid <MAX_NUMNODES ; nid++) { +for (nid =0 ; nid <MAX_NR_NODES ; nid++) {unsigned long start, end;unsigned long start_paddr, end_paddr;int i;
@@ -152,7 +152,7 @@for (i =1 ; i <MAX_NR_ZONES ; i++) zones_size[i] =0 ; -for (nid =0 ; nid <MAX_NUMNODES ; nid++) { +for (nid =0 ; nid <MAX_NR_NODES ; nid++) {unsigned long start_pfn;unsigned long end_pfn;
include/asm-alpha/numnodes.h +0 -12
@@ -1,12 +0,0 @@ -#ifndef _ASM_MAX_NUMNODES_H -#define _ASM_MAX_NUMNODES_H - -/* -* Currently the Wildfire is the only discontigmem/NUMA capable Alpha core. -*/ -#if defined(CONFIG_ALPHA_WILDFIRE ) || defined(CONFIG_ALPHA_GENERIC ) -# include <asm/core_wildfire.h> -# define MAX_NUMNODES WILDFIRE_MAX_QBB -#endif - -#endif /* _ASM_MAX_NUMNODES_H */
include/asm-alpha/param.h +8 -0
@@ -7,6 +7,14 @@#include <linux/config.h> +/* +* Currently the Wildfire is the only discontigmem/NUMA capable Alpha core. +*/ +#if defined(CONFIG_ALPHA_WILDFIRE ) || defined(CONFIG_ALPHA_GENERIC ) +# include <asm/core_wildfire.h> +# define MAX_NR_NODES WILDFIRE_MAX_QBB +#endif +#ifndef HZ # ifndef CONFIG_ALPHA_RAWHIDE # define HZ 1024
include/asm-arm/arch-clps711x/memory.h +0 -2
@@ -109,8 +109,6 @@* node 3: 0xd8000000 - 0xdfffffff */ -#define NR_NODES 4 -/* * Given a kernel address, find the home node of the underlying memory. */
include/asm-arm/arch-clps711x/param.h +1 -0
@@ -17,3 +17,4 @@* along with this program; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +#define MAX_NR_NODES 4
include/asm-arm/arch-sa1100/memory.h +0 -2
@@ -74,8 +74,6 @@* node 3: 0xd8000000 - 0xdfffffff */ -#define NR_NODES 4 -/* * Given a kernel address, find the home node of the underlying memory. */
include/asm-arm/arch-sa1100/param.h +1 -0
@@ -1,3 +1,4 @@/* * linux/include/asm-arm/arch-sa1100/param.h */ +#define MAX_NR_NODES 4
include/asm-arm/memory.h +2 -2
@@ -88,12 +88,12 @@#define pfn_to_page (pfn) \ (PFN_TO_MAPBASE(pfn) + LOCAL_MAP_NR((pfn) << PAGE_SHIFT)) -#define pfn_valid (pfn) (PFN_TO_NID(pfn) < NR_NODES) +#define pfn_valid (pfn) (PFN_TO_NID(pfn) < MAX_NR_NODES)#define virt_to_page (kaddr) \ (ADDR_TO_MAPBASE(kaddr) + LOCAL_MAP_NR(kaddr)) -#define virt_addr_valid (kaddr) (KVADDR_TO_NID(kaddr) < NR_NODES) +#define virt_addr_valid (kaddr) (KVADDR_TO_NID(kaddr) < MAX_NR_NODES)/* * Common discontigmem stuff.
include/asm-arm/numnodes.h +0 -17
@@ -1,17 +0,0 @@ -/* -* linux/include/asm-arm/numnodes.h -* -* Copyright (C) 2002 Russell King -* -* This program is free software; you can redistribute it and/or modify -* it under the terms of the GNU General Public License version 2 as -* published by the Free Software Foundation. -*/ -#ifndef __ASM_ARM_NUMNODES_H -#define __ASM_ARM_NUMNODES_H - -#include <asm/memory.h> - -#define MAX_NUMNODES NR_NODES - -#endif
include/asm-arm/param.h +6 -0
@@ -13,6 +13,12 @@#include <asm/arch/param.h> /* for HZ */ #include <asm/proc/page.h> /* for EXEC_PAGE_SIZE */ +#ifdef CONFIG_DISCONTIGMEM +# ifndef MAX_NR_NODES +# define MAX_NR_NODES 4 +# endif +#endif /* CONFIG_DISCONTIGMEM */ +#ifndef __KERNEL_HZ#define __KERNEL_HZ 100#endif
include/asm-i386/numaq.h +1 -2
@@ -40,7 +40,6 @@#define pfn_to_pgdat (pfn) NODE_DATA(pfn_to_nid(pfn))#define PHYSADDR_TO_NID (pa) pfn_to_nid(pa >> PAGE_SHIFT) -#define MAX_NUMNODES 8extern int pfn_to_nid (unsigned long );extern void get_memcfg_numaq (void );#define get_memcfg_numa () get_memcfg_numaq()
@@ -167,7 +166,7 @@/* * memory configuration area for each quad */ - struct eachquadmem eq[MAX_NUMNODES]; /* indexed by quad id */ + struct eachquadmem eq[MAX_NR_NODES]; /* indexed by quad id */ };#endif /* CONFIG_X86_NUMAQ */
include/asm-i386/numnodes.h +0 -12
@@ -1,12 +0,0 @@ -#ifndef _ASM_MAX_NUMNODES_H -#define _ASM_MAX_NUMNODES_H - -#include <linux/config.h> - -#ifdef CONFIG_X86_NUMAQ -#include <asm/numaq.h> -#else -#define MAX_NUMNODES 1 -#endif /* CONFIG_X86_NUMAQ */ - -#endif /* _ASM_MAX_NUMNODES_H */
include/asm-i386/param.h +4 -0
@@ -1,6 +1,10 @@#ifndef _ASMi386_PARAM_H#define _ASMi386_PARAM_H +#ifdef CONFIG_DISCONTIGMEM +#define MAX_NR_NODES 8 +#endif +#ifdef __KERNEL__# define HZ 1000/* Internal kernel timer frequency */ # define USER_HZ 100/* .. some user interfaces are in "ticks" */
include/asm-ppc64/numnodes.h +0 -6
@@ -1,6 +0,0 @@ -#ifndef _ASM_MAX_NUMNODES_H -#define _ASM_MAX_NUMNODES_H - -#define MAX_NUMNODES 16 - -#endif /* _ASM_MAX_NUMNODES_H */
include/asm-ppc64/param.h +4 -0
@@ -8,6 +8,10 @@* 2 of the License, or (at your option) any later version. */ +#ifdef CONFIG_DISCONTIGMEM +#define MAX_NR_NODES 16 +#endif +#ifdef __KERNEL__# define HZ 1000/* Internal kernel timer frequency */ # define USER_HZ 100/* .. some user interfaces are in "ticks" */
include/linux/mmzone.h +7 -11
@@ -9,13 +9,8 @@#include <linux/list.h> #include <linux/wait.h> #include <linux/cache.h> +#include <linux/param.h> #include <asm/atomic.h> -#ifdef CONFIG_DISCONTIGMEM -#include <asm/numnodes.h> -#endif -#ifndef MAX_NUMNODES -#define MAX_NUMNODES 1 -#endif /* Free memory management - zoned buddy allocator. */ #ifndef CONFIG_FORCE_MAX_ZONEORDER
@@ -128,6 +123,11 @@#define ZONE_HIGHMEM 2#define MAX_NR_ZONES 3 +/* page->zone is currently 8 bits ... */ +#if MAX_NR_NODES > (255 /MAX_NR_ZONES ) +#error "MAX_NR_NODES too large!!" +#endif +/* * One allocation request operates on a zonelist. A zonelist * is a list of zones, the first one is the 'goal' of the
@@ -140,7 +140,7 @@* footprint of this construct is very small. */ struct zonelist { -struct zone *zones[MAX_NUMNODES *MAX_NR_ZONES +1 ];// NULL delimited +struct zone *zones[MAX_NR_NODES *MAX_NR_ZONES +1 ];// NULL delimited };#define GFP_ZONEMASK 0x0f
@@ -252,13 +252,9 @@extern struct pglist_data contig_page_data;#define NODE_DATA (nid) (&contig_page_data)#define NODE_MEM_MAP (nid) mem_map -#define MAX_NR_NODES 1#else /* CONFIG_DISCONTIGMEM */ -#include <asm/mmzone.h> -/* page->zone is currently 8 bits ... */ -#define MAX_NR_NODES (255 / MAX_NR_ZONES)#endif /* !CONFIG_DISCONTIGMEM */
include/linux/param.h +4 -0
@@ -3,4 +3,8 @@#include <asm/param.h> +#ifndef MAX_NR_NODES +#define MAX_NR_NODES 1 +#endif +#endif
Quoted 2 lines · Matthew Dobson
Anyone who is more familiar with some of the architectures I mucked with (arm, alpha, ppc64), please let me know if what I've done looks wrong.
Well, this breaks ARM. ARM needs MAX_NR_NODES even for the non-discontig mem case. Also, I really don't like the idea of re-using param.h for something else - if its going to hoover up random constants, then its going to create the usual mess, where if you change one constant that's used in 1% of files, 100% of files gets rebuilt.
That is why arm has asm/memory.h to contain everything related to memory translation and discontig memory.
It would be better if it remained in mmzone.h for non-arm, and the memory.h files for arm. I really never understood why numnodes.h was created when mmzone.h has works adequately well since 2.3.
Quoted 75 lines
diff -Nur --exclude-from=/usr/src/.dontdiff
linux-2.5.44-vanilla/include/asm-arm/arch-clps711x/memory.hlinux-2.5.44-max_numnodes_fix/include/asm-arm/arch-clps711x/memory.hinclude/asm-arm/arch-clps711x/memory.h +0
-2@@ -109,8 +109,6 @@* node 3: 0xd8000000 - 0xdfffffff */ -#define NR_NODES 4 -/* * Given a kernel address, find the home node of the underlying memory. */ diff -Nur --exclude-from=/usr/src/.dontdiff
linux-2.5.44-vanilla/include/asm-arm/arch-clps711x/param.hlinux-2.5.44-max_numnodes_fix/include/asm-arm/arch-clps711x/param.hinclude/asm-arm/arch-clps711x/param.h +1
-0@@ -17,3 +17,4 @@* along with this program; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +#define MAX_NR_NODES 4diff -Nur --exclude-from=/usr/src/.dontdiff
linux-2.5.44-vanilla/include/asm-arm/memory.hlinux-2.5.44-max_numnodes_fix/include/asm-arm/memory.hinclude/asm-arm/memory.h +2
-2@@ -88,12 +88,12 @@#define pfn_to_page (pfn) \ (PFN_TO_MAPBASE(pfn) + LOCAL_MAP_NR((pfn) << PAGE_SHIFT)) -#define pfn_valid (pfn) (PFN_TO_NID(pfn) < NR_NODES) +#define pfn_valid (pfn) (PFN_TO_NID(pfn) < MAX_NR_NODES)#define virt_to_page (kaddr) \ (ADDR_TO_MAPBASE(kaddr) + LOCAL_MAP_NR(kaddr)) -#define virt_addr_valid (kaddr) (KVADDR_TO_NID(kaddr) < NR_NODES) +#define virt_addr_valid (kaddr) (KVADDR_TO_NID(kaddr) < MAX_NR_NODES)/* * Common discontigmem stuff. diff -Nur --exclude-from=/usr/src/.dontdiff
linux-2.5.44-vanilla/include/asm-arm/numnodes.hlinux-2.5.44-max_numnodes_fix/include/asm-arm/numnodes.hinclude/asm-arm/numnodes.h +0
-17@@ -1,17 +0,0 @@ -/* -* linux/include/asm-arm/numnodes.h -* -* Copyright (C) 2002 Russell King -* -* This program is free software; you can redistribute it and/or modify -* it under the terms of the GNU General Public License version 2 as -* published by the Free Software Foundation. -*/ -#ifndef __ASM_ARM_NUMNODES_H -#define __ASM_ARM_NUMNODES_H - -#include <asm/memory.h> - -#define MAX_NUMNODES NR_NODES - -#endif diff -Nur --exclude-from=/usr/src/.dontdiff
linux-2.5.44-vanilla/include/asm-arm/param.hlinux-2.5.44-max_numnodes_fix/include/asm-arm/param.hinclude/asm-arm/param.h +6
-0@@ -13,6 +13,12 @@#include <asm/arch/param.h> /* for HZ */ #include <asm/proc/page.h> /* for EXEC_PAGE_SIZE */ +#ifdef CONFIG_DISCONTIGMEM +# ifndef MAX_NR_NODES +# define MAX_NR_NODES 4 +# endif +#endif /* CONFIG_DISCONTIGMEM */ +#ifndef __KERNEL_HZ#define __KERNEL_HZ 100#endif
Signature · 2 lines
Russell King ([email protected]) The developer of ARM Linux http://www.arm.linux.org.uk/personal/aboutme.html
Quoted 9 lines · Russell King
Quoted 1 line · Matthew Dobson
Anyone who is more familiar with some of the architectures I mucked with (arm, alpha, ppc64), please let me know if what I've done looks wrong.
Well, this breaks ARM. ARM needs
MAX_NR_NODESeven for the non-discontig mem case. Also, I really don't like the idea of re-usingparam.hfor something else - if its going to hoover up random constants, then its going to create the usual mess, where if you change one constant that's used in 1% of files, 100% of files gets rebuilt.
Hmmm... MAX_NR_NODES is *definitely* available in the non-discontig case. In include/linux/param.h, MAX_NR_NODES (ifndef'd) is defined to be 1. This is the generic case. linux/param.h first includes asm/param.h, so the architecture has the chance to define MAX_NR_NODES as appropriate for the specific arch. Also, for clps711x and sa1100, MAX_NR_NODES is defined to be 4, as it NR_NODES was before the change. Are you sure this is really broken? Have you tried out the patch? I'd do it myself, but I don't have any access to appropriate machines.
Quoted 2 lines
That is why arm has
asm/memory.hto contain everything related to memory translation and discontig memory.
This isn't *just* a discontig change. CONFIG_DISCONTIGMEM is a convenient option to key off of, but as the kernel becomes more and more NUMA aware, the number of nodes in the system becomes a useful bit of information to more and more of the code. *That* is why (along with a suggestion from wli) I put the #defines in param.h.
Quoted 3 lines
It would be better if it remained in
mmzone.hfor non-arm, and thememory.hfiles for arm. I really never understood whynumnodes.hwas created whenmmzone.hhas works adequately well since 2.3.
The entire point of this patch is to make this sort of thing *more* consistent. If this is really that bad for arm, then we can just forget the patch. I have no desire to increase the complexity of this, or at best keep it the same. As mentioned in the original post, I was trying to kill a bunch of (seemingly) unnecessary .h files (the numnodes.h's specifically), and remove the MAX_[NUM|NR_]NODES duality. If that can't be accomplished, then all this would do is move the confusion around, and I don't want that...
If you feel param.h is the wrong place, I originally had the #define's in asm/topology.h. I feel that is also an appropriate place, exists for every architecture, and my second choice. I could fairly easily retool the patch to use that if that is decided to be more appropriate.
If you could give the patch a second read-through, noting the points in my first paragraph, I would really appreciate it. If you still think it's broken, we can try and work out how to fix it, or we can drop it.
Cheers!
-Matt
Quoted 3 lines · Matthew Dobson
Hmmm...
MAX_NR_NODESis *definitely* available in the non-discontig case. Ininclude/linux/param.h,MAX_NR_NODES(ifndef'd) is defined to be 1.
I missed this bit.
Quoted 7 lines
Quoted 2 lines
That is why arm has
asm/memory.hto contain everything related to memory translation and discontig memory.This isn't *just* a discontig change.
CONFIG_DISCONTIGMEMis a convenient option to key off of, but as the kernel becomes more and more NUMA aware, the number of nodes in the system becomes a useful bit of information to more and more of the code. *That* is why (along with a suggestion from wli) I put the #defines inparam.h.
To me, it seems inappropriate to litter header files that contain nothing to do with memory management with stuff from memory management when other headers that already contain memory management bits get ignored. This type of practice is why we've ended up with #include hell in the kernel today.
Quoted 9 lines
Quoted 3 lines
It would be better if it remained in
mmzone.hfor non-arm, and thememory.hfiles for arm. I really never understood whynumnodes.hwas created whenmmzone.hhas works adequately well since 2.3.As mentioned in the original post, I was trying
to kill a bunch of (seemingly) unnecessary .h files (the numnodes.h's specifically), and remove the MAX_[NUM|NR_]NODES duality. If that can't be accomplished, then all this would do is move the confusion around, and I don't want that...
I'm in agreement with you here.
Quoted 2 lines
If you feel
param.his the wrong place, I originally had the #define's inasm/topology.h.
This seems like a good solution - linux/mmzone.h already includes this file, so it wouldn't be adding all that much to the #include hell. Also, asm-generic/topology.h contains stuff to do with numa/discontig already, so it seems perfect.
Signature · 2 lines
Russell King ([email protected]) The developer of ARM Linux http://www.arm.linux.org.uk/personal/aboutme.html
Quoted 8 lines · Russell King
Quoted 5 lines · Matthew Dobson
Quoted 3 lines
It would be better if it remained in
mmzone.hfor non-arm, and thememory.hfiles for arm. I really never understood whynumnodes.hwas created whenmmzone.hhas works adequately well since 2.3.As mentioned in the original post, I was trying to kill a bunch of (seemingly) unnecessary .h files (the numnodes.h's specifically), and remove the MAX_[NUM|NR_]NODES duality. If that can't be accomplished, then all this would do is move the confusion around, and I don't want that...
I'm in agreement with you here.
I knew we must have some motivation in common! ;)
Quoted 6 lines
Quoted 1 line
If you feel
param.his the wrong place, I originally had the #define's inasm/topology.h.This seems like a good solution -
linux/mmzone.halready includes this file, so it wouldn't be adding all that much to the #include hell. Also,asm-generic/topology.hcontains stuff to do with numa/discontig already, so it seems perfect.
Ok... I've revised the patch to put the #define's in topology.h. Please have a look and make sure I still haven't butchered things for arm. I think that <asm/topology.h> is a particularly reasonable place to define MAX_NR_NODES and other topology things! :) We'll see if this piques anyone's interest.
Quoted 1 line
Quoted 1 line
max_numnodes_fix.patch
Collapse the use of MAX_NUMNODES throughout the kernel into the already existing, far less used, yet far more appropriately named MAX_NR_NODES.
[mcd@arrakis src]$ diffstat ~/patches/max_numnodes_fix-2.5.44.patch
arch/alpha/mm/numa.c | 6 +++--- arch/arm/mm/discontig.c | 6 +++--- arch/arm/mm/init.c | 12 +++--------- arch/i386/kernel/numaq.c | 10 ++++++---- arch/i386/mm/discontig.c | 11 +++++------ arch/ppc64/mm/init.c | 2 +- arch/ppc64/mm/numa.c | 10 +++++----- include/asm-alpha/numnodes.h | 12 ------------ include/asm-alpha/topology.h | 10 ++++++++++ include/asm-arm/arch-clps711x/memory.h | 2 -- include/asm-arm/arch-sa1100/memory.h | 2 -- include/asm-arm/memory.h | 5 +++-- include/asm-arm/numnodes.h | 17 ----------------- include/asm-arm/topology.h | 4 ++++ include/asm-generic/topology.h | 4 ++++ include/asm-i386/numaq.h | 5 ++--- include/asm-i386/numnodes.h | 12 ------------ include/asm-i386/topology.h | 4 ++++ include/asm-ppc64/numnodes.h | 6 ------ include/asm-ppc64/topology.h | 6 ++++++ include/linux/mmzone.h | 19 +++++++------------ 21 files changed, 66 insertions(+), 99 deletions(-)
Kills 33 lines of code, and also kills 4 files. With (apparently) no change in functionality and possibly even a positive change in readability.
Cheers!
-Matt
arch/alpha/mm/numa.c +3 -3
@@ -19,8 +19,8 @@#include <asm/hwrpb.h> #include <asm/pgalloc.h> -plat_pg_data_t *plat_node_data[MAX_NUMNODES ]; -bootmem_data_t plat_node_bdata[MAX_NUMNODES ]; +plat_pg_data_t *plat_node_data[MAX_NR_NODES ]; +bootmem_data_t plat_node_bdata[MAX_NR_NODES ];#undef DEBUG_DISCONTIG#ifdef DEBUG_DISCONTIG
@@ -243,7 +243,7 @@show_mem_layout (); numnodes =0 ; -for (nid =0 ; nid <MAX_NUMNODES ; nid++) +for (nid =0 ; nid <MAX_NR_NODES ; nid++)setup_memory_node (nid, kernel_end);#ifdef CONFIG_BLK_DEV_INITRD
arch/arm/mm/discontig.c +3 -3
@@ -15,7 +15,7 @@#include <linux/init.h> #include <linux/bootmem.h> -#if NR_NODES !=4 +#if MAX_NR_NODES !=4 #error Fix Me Please#endif
@@ -23,9 +23,9 @@* Our node_data structure for discontigous memory. */ -static bootmem_data_t node_bootmem_data[NR_NODES ]; +static bootmem_data_t node_bootmem_data[MAX_NR_NODES ]; -pg_data_t discontig_node_data[NR_NODES ] = { +pg_data_t discontig_node_data[MAX_NR_NODES ] = { { bdata: &node_bootmem_data[0 ] }, { bdata: &node_bootmem_data[1 ] }, { bdata: &node_bootmem_data[2 ] },
arch/arm/mm/init.c +3 -9
@@ -34,12 +34,6 @@#include <asm/mach/arch.h> #include <asm/mach/map.h> -#ifndef CONFIG_DISCONTIGMEM -#define NR_NODES 1 -#else -#define NR_NODES 4 -#endif -#ifdef CONFIG_CPU_32 #define TABLE_OFFSET (PTRS_PER_PTE)#else
@@ -179,7 +173,7 @@ {unsigned int i, bootmem_pages =0 , memend_pfn =0 ; -for (i =0 ; i <NR_NODES ; i++) { +for (i =0 ; i <MAX_NR_NODES ; i++) { np[i].start =-1U ; np[i].end =0 ; np[i].bootmap_pages =0 ;
@@ -208,7 +202,7 @@* we have, we're in trouble. (maybe we ought to * limit, instead of bugging?) */ -if (numnodes >NR_NODES ) +if (numnodes >MAX_NR_NODES )BUG (); }
@@ -366,7 +360,7 @@*/ void __init bootmem_init (struct meminfo *mi) { -struct node_info node_info[NR_NODES ], *np = node_info; +struct node_info node_info[MAX_NR_NODES ], *np = node_info;unsigned int bootmap_pages, bootmap_pfn, map_pg;int node, initrd_node;
arch/i386/kernel/numaq.c +6 -4
@@ -26,12 +26,11 @@#include <linux/config.h> #include <linux/mm.h> #include <linux/bootmem.h> -#include <linux/mmzone.h> #include <asm/numaq.h> /* These are needed before the pgdat's are created */ -unsigned long node_start_pfn[MAX_NUMNODES ]; -unsigned long node_end_pfn[MAX_NUMNODES ]; +unsigned long node_start_pfn[MAX_NR_NODES ]; +unsigned long node_end_pfn[MAX_NR_NODES ];#define MB_TO_PAGES (addr) ((addr) << (20 - PAGE_SHIFT))
@@ -50,7 +49,10 @@ (struct sys_cfg_data *)__va (SYS_CFG_DATA_PRIV_ADDR ); numnodes =0 ; -for (node =0 ; node <MAX_NUMNODES ; node++) { +/* FIXME: This loop is doing bitwise operations with a 32 +* bit field, but the bounds are not being checked. +*/ +for (node =0 ; node <MAX_NR_NODES ; node++) {if (scd->quads_present31_0 & (1 << node)) { numnodes++; eq = &scd->eq[node];
arch/i386/mm/discontig.c +5 -6
@@ -25,7 +25,6 @@#include <linux/config.h> #include <linux/mm.h> #include <linux/bootmem.h> -#include <linux/mmzone.h> #include <linux/highmem.h> #ifdef CONFIG_BLK_DEV_RAM #include <linux/blk.h>
@@ -33,7 +32,7 @@#include <asm/e820.h> #include <asm/setup.h> -struct pglist_data *node_data[MAX_NUMNODES ]; +struct pglist_data *node_data[MAX_NR_NODES ];bootmem_data_t node0_bdata;extern unsigned long find_max_low_pfn (void );
@@ -115,10 +114,10 @@#define LARGE_PAGE_BYTES (PTRS_PER_PTE * PAGE_SIZE) -unsigned long node_remap_start_pfn[MAX_NUMNODES ]; -unsigned long node_remap_size[MAX_NUMNODES ]; -unsigned long node_remap_offset[MAX_NUMNODES ]; -void *node_remap_start_vaddr[MAX_NUMNODES ]; +unsigned long node_remap_start_pfn[MAX_NR_NODES ]; +unsigned long node_remap_size[MAX_NR_NODES ]; +unsigned long node_remap_offset[MAX_NR_NODES ]; +void *node_remap_start_vaddr[MAX_NR_NODES ];extern void set_pmd_pfn (unsigned long vaddr,unsigned long pfn,pgprot_t flags);void __init remap_numa_kva (void )
arch/ppc64/mm/init.c +1 -1
@@ -522,7 +522,7 @@ {int nid; -for (nid =0 ; nid <MAX_NUMNODES ; nid++) { +for (nid =0 ; nid <MAX_NR_NODES ; nid++) {if (numa_node_exists[nid]) {printk ("freeing bootmem node %x\n" , nid); totalram_pages +=
arch/ppc64/mm/numa.c +5 -5
@@ -24,10 +24,10 @@int numa_cpu_lookup_table[NR_CPUS ] = { [0 ... (NR_CPUS -1 )] =-1 };int numa_memory_lookup_table[MAX_MEMORY >>MEMORY_INCREMENT_SHIFT ] = { [0 ... ((MAX_MEMORY >>MEMORY_INCREMENT_SHIFT ) -1 )] =-1 }; -int numa_node_exists[MAX_NUMNODES ]; +int numa_node_exists[MAX_NR_NODES ]; -struct pglist_data node_data[MAX_NUMNODES ]; -bootmem_data_t plat_node_bdata[MAX_NUMNODES ]; +struct pglist_data node_data[MAX_NR_NODES ]; +bootmem_data_t plat_node_bdata[MAX_NR_NODES ];static int __init parse_numa_properties (void ) {
@@ -45,7 +45,7 @@if (parse_numa_properties ())BUG (); -for (nid =0 ; nid <MAX_NUMNODES ; nid++) { +for (nid =0 ; nid <MAX_NR_NODES ; nid++) {unsigned long start, end;unsigned long start_paddr, end_paddr;int i;
@@ -152,7 +152,7 @@for (i =1 ; i <MAX_NR_ZONES ; i++) zones_size[i] =0 ; -for (nid =0 ; nid <MAX_NUMNODES ; nid++) { +for (nid =0 ; nid <MAX_NR_NODES ; nid++) {unsigned long start_pfn;unsigned long end_pfn;
include/asm-alpha/numnodes.h +0 -12
@@ -1,12 +0,0 @@ -#ifndef _ASM_MAX_NUMNODES_H -#define _ASM_MAX_NUMNODES_H - -/* -* Currently the Wildfire is the only discontigmem/NUMA capable Alpha core. -*/ -#if defined(CONFIG_ALPHA_WILDFIRE ) || defined(CONFIG_ALPHA_GENERIC ) -# include <asm/core_wildfire.h> -# define MAX_NUMNODES WILDFIRE_MAX_QBB -#endif - -#endif /* _ASM_MAX_NUMNODES_H */
include/asm-alpha/topology.h +10 -0
@@ -1,6 +1,16 @@#ifndef _ASM_ALPHA_TOPOLOGY_H#define _ASM_ALPHA_TOPOLOGY_H +/* +* Currently the Wildfire is the only discontigmem/NUMA capable Alpha core. +*/ +#if defined(CONFIG_ALPHA_WILDFIRE ) || defined(CONFIG_ALPHA_GENERIC ) +# include <asm/core_wildfire.h> +# define MAX_NR_NODES WILDFIRE_MAX_QBB +#else +# define MAX_NR_NODES 1 +#endif +#ifdef CONFIG_NUMA #ifdef CONFIG_ALPHA_WILDFIRE /* With wildfire assume 4 CPUs per node */
include/asm-arm/arch-clps711x/memory.h +0 -2
@@ -109,8 +109,6 @@* node 3: 0xd8000000 - 0xdfffffff */ -#define NR_NODES 4 -/* * Given a kernel address, find the home node of the underlying memory. */
include/asm-arm/arch-sa1100/memory.h +0 -2
@@ -74,8 +74,6 @@* node 3: 0xd8000000 - 0xdfffffff */ -#define NR_NODES 4 -/* * Given a kernel address, find the home node of the underlying memory. */
include/asm-arm/memory.h +3 -2
@@ -14,6 +14,7 @@#include <linux/config.h> #include <asm/arch/memory.h> +#include <asm/topology.h> /* * PFNs are used to describe any physical page; this means
@@ -88,12 +89,12 @@#define pfn_to_page (pfn) \ (PFN_TO_MAPBASE(pfn) + LOCAL_MAP_NR((pfn) << PAGE_SHIFT)) -#define pfn_valid (pfn) (PFN_TO_NID(pfn) < NR_NODES) +#define pfn_valid (pfn) (PFN_TO_NID(pfn) < MAX_NR_NODES)#define virt_to_page (kaddr) \ (ADDR_TO_MAPBASE(kaddr) + LOCAL_MAP_NR(kaddr)) -#define virt_addr_valid (kaddr) (KVADDR_TO_NID(kaddr) < NR_NODES) +#define virt_addr_valid (kaddr) (KVADDR_TO_NID(kaddr) < MAX_NR_NODES)/* * Common discontigmem stuff.
include/asm-arm/numnodes.h +0 -17
@@ -1,17 +0,0 @@ -/* -* linux/include/asm-arm/numnodes.h -* -* Copyright (C) 2002 Russell King -* -* This program is free software; you can redistribute it and/or modify -* it under the terms of the GNU General Public License version 2 as -* published by the Free Software Foundation. -*/ -#ifndef __ASM_ARM_NUMNODES_H -#define __ASM_ARM_NUMNODES_H - -#include <asm/memory.h> - -#define MAX_NUMNODES NR_NODES - -#endif
include/asm-arm/topology.h +4 -0
@@ -1,6 +1,10 @@#ifndef _ASM_ARM_TOPOLOGY_H#define _ASM_ARM_TOPOLOGY_H +#if defined(CONFIG_DISCONTIGMEM ) || defined(CONFIG_ARCH_SA1100 ) || defined(CONFIG_ARCH_CLPS711X ) +#define MAX_NR_NODES 4 +#endif +#include <asm-generic/topology.h> #endif /* _ASM_ARM_TOPOLOGY_H */
include/asm-generic/topology.h +4 -0
@@ -27,6 +27,10 @@#ifndef _ASM_GENERIC_TOPOLOGY_H#define _ASM_GENERIC_TOPOLOGY_H +#ifndef MAX_NR_NODES +#define MAX_NR_NODES 1 +#endif +/* Other architectures wishing to use this simple topology API should fill in the below functions as appropriate in their own <asm/topology.h> file. */ #ifndef __cpu_to_node
include/asm-i386/numaq.h +2 -3
@@ -28,7 +28,7 @@#ifdef CONFIG_X86_NUMAQ -#include <asm/smpboot.h> +#include <asm/topology.h> /* * for now assume that 64Gb is max amount of RAM for whole system
@@ -40,7 +40,6 @@#define pfn_to_pgdat (pfn) NODE_DATA(pfn_to_nid(pfn))#define PHYSADDR_TO_NID (pa) pfn_to_nid(pa >> PAGE_SHIFT) -#define MAX_NUMNODES 8extern int pfn_to_nid (unsigned long );extern void get_memcfg_numaq (void );#define get_memcfg_numa () get_memcfg_numaq()
@@ -167,7 +166,7 @@/* * memory configuration area for each quad */ - struct eachquadmem eq[MAX_NUMNODES]; /* indexed by quad id */ + struct eachquadmem eq[MAX_NR_NODES]; /* indexed by quad id */ };#endif /* CONFIG_X86_NUMAQ */
include/asm-i386/numnodes.h +0 -12
@@ -1,12 +0,0 @@ -#ifndef _ASM_MAX_NUMNODES_H -#define _ASM_MAX_NUMNODES_H - -#include <linux/config.h> - -#ifdef CONFIG_X86_NUMAQ -#include <asm/numaq.h> -#else -#define MAX_NUMNODES 1 -#endif /* CONFIG_X86_NUMAQ */ - -#endif /* _ASM_MAX_NUMNODES_H */
include/asm-i386/topology.h +4 -0
@@ -27,6 +27,10 @@#ifndef _ASM_I386_TOPOLOGY_H#define _ASM_I386_TOPOLOGY_H +#ifdef CONFIG_NUMA +#define MAX_NR_NODES 16 +#endif +#ifdef CONFIG_X86_NUMAQ #include <asm/smpboot.h>
include/asm-ppc64/numnodes.h +0 -6
@@ -1,6 +0,0 @@ -#ifndef _ASM_MAX_NUMNODES_H -#define _ASM_MAX_NUMNODES_H - -#define MAX_NUMNODES 16 - -#endif /* _ASM_MAX_NUMNODES_H */
include/asm-ppc64/topology.h +6 -0
@@ -3,6 +3,12 @@#include <asm/mmzone.h> +#ifdef CONFIG_DISCONTIGMEM +#define MAX_NR_NODES 16 +#else +#define MAX_NR_NODES 1 +#endif +#ifdef CONFIG_NUMA static inline int __cpu_to_node (int cpu)
include/linux/mmzone.h +7 -12
@@ -10,12 +10,7 @@#include <linux/wait.h> #include <linux/cache.h> #include <asm/atomic.h> -#ifdef CONFIG_DISCONTIGMEM -#include <asm/numnodes.h> -#endif -#ifndef MAX_NUMNODES -#define MAX_NUMNODES 1 -#endif +#include <asm/topology.h> /* Free memory management - zoned buddy allocator. */ #ifndef CONFIG_FORCE_MAX_ZONEORDER
@@ -128,6 +123,11 @@#define ZONE_HIGHMEM 2#define MAX_NR_ZONES 3 +/* page->zone is currently 8 bits ... */ +#if MAX_NR_NODES > (255 /MAX_NR_ZONES ) +#error "MAX_NR_NODES too large!!" +#endif +/* * One allocation request operates on a zonelist. A zonelist * is a list of zones, the first one is the 'goal' of the
@@ -140,7 +140,7 @@* footprint of this construct is very small. */ struct zonelist { -struct zone *zones[MAX_NUMNODES *MAX_NR_ZONES +1 ];// NULL delimited +struct zone *zones[MAX_NR_NODES *MAX_NR_ZONES +1 ];// NULL delimited };#define GFP_ZONEMASK 0x0f
@@ -244,7 +244,6 @@#define MAX_NR_MEMBLKS 1#endif /* CONFIG_NUMA */ -#include <asm/topology.h> /* Returns the number of the current Node. */ #define numa_node_id () (__cpu_to_node(smp_processor_id()))
@@ -252,13 +251,9 @@extern struct pglist_data contig_page_data;#define NODE_DATA (nid) (&contig_page_data)#define NODE_MEM_MAP (nid) mem_map -#define MAX_NR_NODES 1#else /* CONFIG_DISCONTIGMEM */ -#include <asm/mmzone.h> -/* page->zone is currently 8 bits ... */ -#define MAX_NR_NODES (255 / MAX_NR_ZONES)#endif /* !CONFIG_DISCONTIGMEM */
Hi Matt,
Quoted 6 lines
This patch
1) Gets rid of theinclude/asm-xxx/numnodes.hfiles
2) DefinesMAX_NR_NODESto the appropriate per-arch value ininclude/asm-xxx/param.h
3) changes all remaining occurencesMAX_NUMNODEStoMAX_NR_NODESthroughout the kernel
Looks good for ppc64.
Anton (using 20 hours of flights trying to catch up on email)