no-arch-specific-mem_map-init

no-arch-specific-mem_map-init

[Andrew Morton]

Everyone OK with this?


From: Dave Hansen <haveblue@us.ibm.com>

So, this patch started out with me trying to keep from passing contiguous,
node-specific mem_map into free_area_init_node() and cousins.  Instead, I
relied on some calls to pfn_to_page().

This works fine and dandy when all you need is the pgdat->node_mem_map to
do pfn_to_page().  However, the non-NUMA/DISCONTIG architectures use the
real, global mem_map[] instead of a node_mem_map in the pfn_to_page()
calculation.  So, I ended up effectively trying to initialize mem_map from
itself, when it was NULL.  That was bad, and caused some very pretty colors
on someone's screen when he tested it.

So, I had to make sure to initialize the global mem_map[] before calling
into free_area_init_node().  Then, I realized how many architectures do
this on their own, and have comments like this:

        /* XXX: MRB-remove - this doesn't seem sane, should this be done somewhere else ?*/
        mem_map = NODE_DATA(0)->node_mem_map;

The following patch does what my first one did (don't pass mem_map into the
init functions), incorporates Jesse Barnes' ia64 fixes on top of that, and
gets rid of all but one of the global mem_map initializations (parisc is
weird).  It also magically removes more code than it adds.  It could be
smaller, but I shamelessly added some comments.  

Boot-tested on ppc64, i386 (NUMAQ, plain SMP, laptop), UML (i386).

Signed-off-by: Dave Hansen <haveblue@us.ibm.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 25-akpm/arch/arm/mm/init.c           |    4 ----
 25-akpm/arch/arm26/mm/init.c         |    2 --
 25-akpm/arch/cris/arch-v10/mm/init.c |    1 -
 25-akpm/arch/ia64/mm/contig.c        |    2 +-
 25-akpm/arch/m32r/mm/init.c          |    2 --
 25-akpm/arch/ppc64/mm/init.c         |    1 -
 25-akpm/arch/sh/mm/init.c            |    2 --
 25-akpm/arch/sh64/mm/init.c          |    3 ---
 25-akpm/arch/sparc/mm/srmmu.c        |    1 -
 25-akpm/arch/sparc/mm/sun4c.c        |    1 -
 25-akpm/arch/sparc64/mm/init.c       |    1 -
 25-akpm/arch/um/kernel/physmem.c     |    1 -
 25-akpm/arch/v850/kernel/setup.c     |    1 -
 25-akpm/mm/page_alloc.c              |   22 ++++++++++++++++------
 14 files changed, 17 insertions(+), 27 deletions(-)

diff -puN arch/arm26/mm/init.c~no-arch-specific-mem_map-init arch/arm26/mm/init.c
--- 25/arch/arm26/mm/init.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/arm26/mm/init.c	Mon Mar  7 16:43:24 2005
@@ -309,8 +309,6 @@ void __init paging_init(struct meminfo *
 	free_area_init_node(0, pgdat, zone_size,
 			bdata->node_boot_start >> PAGE_SHIFT, zhole_size);
 
-	mem_map = NODE_DATA(0)->node_mem_map;
-
 	/*
 	 * finish off the bad pages once
 	 * the mem_map is initialised
diff -puN arch/arm/mm/init.c~no-arch-specific-mem_map-init arch/arm/mm/init.c
--- 25/arch/arm/mm/init.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/arm/mm/init.c	Mon Mar  7 16:43:24 2005
@@ -501,10 +501,6 @@ void __init paging_init(struct meminfo *
 				bdata->node_boot_start >> PAGE_SHIFT, zhole_size);
 	}
 
-#ifndef CONFIG_DISCONTIGMEM
-	mem_map = contig_page_data.node_mem_map;
-#endif
-
 	/*
 	 * finish off the bad pages once
 	 * the mem_map is initialised
diff -puN arch/cris/arch-v10/mm/init.c~no-arch-specific-mem_map-init arch/cris/arch-v10/mm/init.c
--- 25/arch/cris/arch-v10/mm/init.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/cris/arch-v10/mm/init.c	Mon Mar  7 16:43:24 2005
@@ -184,7 +184,6 @@ paging_init(void)
 	 */
 
 	free_area_init_node(0, &contig_page_data, zones_size, PAGE_OFFSET >> PAGE_SHIFT, 0);
-	mem_map = contig_page_data.node_mem_map;
 }
 
 /* Initialize remaps of some I/O-ports. It is important that this
diff -puN arch/ia64/mm/contig.c~no-arch-specific-mem_map-init arch/ia64/mm/contig.c
--- 25/arch/ia64/mm/contig.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/ia64/mm/contig.c	Mon Mar  7 16:43:24 2005
@@ -280,7 +280,7 @@ paging_init (void)
 		vmem_map = (struct page *) vmalloc_end;
 		efi_memmap_walk(create_mem_map_page_table, NULL);
 
-		mem_map = contig_page_data.node_mem_map = vmem_map;
+		NODE_DATA(0)->node_mem_map = vmem_map;
 		free_area_init_node(0, &contig_page_data, zones_size,
 				    0, zholes_size);
 
diff -puN arch/m32r/mm/init.c~no-arch-specific-mem_map-init arch/m32r/mm/init.c
--- 25/arch/m32r/mm/init.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/m32r/mm/init.c	Mon Mar  7 16:43:24 2005
@@ -121,8 +121,6 @@ unsigned long __init zone_sizes_init(voi
 
 	free_area_init_node(0, NODE_DATA(0), zones_size, start_pfn, 0);
 
-	mem_map = contig_page_data.node_mem_map;
-
 	return 0;
 }
 #else	/* CONFIG_DISCONTIGMEM */
diff -puN arch/ppc64/mm/init.c~no-arch-specific-mem_map-init arch/ppc64/mm/init.c
--- 25/arch/ppc64/mm/init.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/ppc64/mm/init.c	Mon Mar  7 16:43:24 2005
@@ -664,7 +664,6 @@ void __init paging_init(void)
 
 	free_area_init_node(0, &contig_page_data, zones_size,
 			    __pa(PAGE_OFFSET) >> PAGE_SHIFT, zholes_size);
-	mem_map = contig_page_data.node_mem_map;
 }
 #endif /* CONFIG_DISCONTIGMEM */
 
diff -puN arch/sh64/mm/init.c~no-arch-specific-mem_map-init arch/sh64/mm/init.c
--- 25/arch/sh64/mm/init.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/sh64/mm/init.c	Mon Mar  7 16:43:24 2005
@@ -124,9 +124,6 @@ void __init paging_init(void)
 	zones_size[ZONE_DMA] = MAX_LOW_PFN - START_PFN;
 	NODE_DATA(0)->node_mem_map = NULL;
 	free_area_init_node(0, NODE_DATA(0), zones_size, __MEMORY_START >> PAGE_SHIFT, 0);
-
-	/* XXX: MRB-remove - this doesn't seem sane, should this be done somewhere else ?*/
-	mem_map = NODE_DATA(0)->node_mem_map;
 }
 
 void __init mem_init(void)
diff -puN arch/sh/mm/init.c~no-arch-specific-mem_map-init arch/sh/mm/init.c
--- 25/arch/sh/mm/init.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/sh/mm/init.c	Mon Mar  7 16:43:24 2005
@@ -216,8 +216,6 @@ void __init paging_init(void)
 #endif
 	NODE_DATA(0)->node_mem_map = NULL;
 	free_area_init_node(0, NODE_DATA(0), zones_size, __MEMORY_START >> PAGE_SHIFT, 0);
-	/* XXX: MRB-remove - this doesn't seem sane, should this be done somewhere else ?*/
-	mem_map = NODE_DATA(0)->node_mem_map;
 
 #ifdef CONFIG_DISCONTIGMEM
 	/*
diff -puN arch/sparc64/mm/init.c~no-arch-specific-mem_map-init arch/sparc64/mm/init.c
--- 25/arch/sparc64/mm/init.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/sparc64/mm/init.c	Mon Mar  7 16:43:24 2005
@@ -1513,7 +1513,6 @@ void __init paging_init(void)
 
 		free_area_init_node(0, &contig_page_data, zones_size,
 				    phys_base >> PAGE_SHIFT, zholes_size);
-		mem_map = contig_page_data.node_mem_map;
 	}
 
 	device_scan();
diff -puN arch/sparc/mm/srmmu.c~no-arch-specific-mem_map-init arch/sparc/mm/srmmu.c
--- 25/arch/sparc/mm/srmmu.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/sparc/mm/srmmu.c	Mon Mar  7 16:43:24 2005
@@ -1343,7 +1343,6 @@ void __init srmmu_paging_init(void)
 
 		free_area_init_node(0, &contig_page_data, zones_size,
 				    pfn_base, zholes_size);
-		mem_map = contig_page_data.node_mem_map;
 	}
 }
 
diff -puN arch/sparc/mm/sun4c.c~no-arch-specific-mem_map-init arch/sparc/mm/sun4c.c
--- 25/arch/sparc/mm/sun4c.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/sparc/mm/sun4c.c	Mon Mar  7 16:43:24 2005
@@ -2117,7 +2117,6 @@ void __init sun4c_paging_init(void)
 
 		free_area_init_node(0, &contig_page_data, zones_size,
 				    pfn_base, zholes_size);
-		mem_map = contig_page_data.node_mem_map;
 	}
 
 	cnt = 0;
diff -puN arch/um/kernel/physmem.c~no-arch-specific-mem_map-init arch/um/kernel/physmem.c
--- 25/arch/um/kernel/physmem.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/um/kernel/physmem.c	Mon Mar  7 16:43:24 2005
@@ -294,7 +294,6 @@ int init_maps(unsigned long physmem, uns
 		INIT_LIST_HEAD(&p->lru);
 	}
 
-	mem_map = map;
 	max_mapnr = total_pages;
 	return(0);
 }
diff -puN arch/v850/kernel/setup.c~no-arch-specific-mem_map-init arch/v850/kernel/setup.c
--- 25/arch/v850/kernel/setup.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/arch/v850/kernel/setup.c	Mon Mar  7 16:43:24 2005
@@ -283,5 +283,4 @@ init_mem_alloc (unsigned long ram_start,
 	NODE_DATA(0)->node_mem_map = NULL;
 	free_area_init_node (0, NODE_DATA(0), zones_size,
 			     ADDR_TO_PAGE (PAGE_OFFSET), 0);
-	mem_map = NODE_DATA(0)->node_mem_map;
 }
diff -puN mm/page_alloc.c~no-arch-specific-mem_map-init mm/page_alloc.c
--- 25/mm/page_alloc.c~no-arch-specific-mem_map-init	Mon Mar  7 16:43:24 2005
+++ 25-akpm/mm/page_alloc.c	Mon Mar  7 16:43:24 2005
@@ -1712,14 +1712,25 @@ static void __init free_area_init_core(s
 	}
 }
 
-void __init node_alloc_mem_map(struct pglist_data *pgdat)
+static void __init alloc_node_mem_map(struct pglist_data *pgdat)
 {
 	unsigned long size;
 
-	size = (pgdat->node_spanned_pages + 1) * sizeof(struct page);
-	pgdat->node_mem_map = alloc_bootmem_node(pgdat, size);
+	/* Skip empty nodes */
+	if (!pgdat->node_spanned_pages)
+		return;
+
+	/* ia64 gets its own node_mem_map, before this, without bootmem */
+	if (!pgdat->node_mem_map) {
+		size = (pgdat->node_spanned_pages + 1) * sizeof(struct page);
+		pgdat->node_mem_map = alloc_bootmem_node(pgdat, size);
+	}
 #ifndef CONFIG_DISCONTIGMEM
-	mem_map = contig_page_data.node_mem_map;
+	/*
+	 * With no DISCONTIG, the global mem_map is just set as node 0's
+	 */
+	if (pgdat == NODE_DATA(0))
+		mem_map = NODE_DATA(0)->node_mem_map;
 #endif
 }
 
@@ -1731,8 +1742,7 @@ void __init free_area_init_node(int nid,
 	pgdat->node_start_pfn = node_start_pfn;
 	calculate_zone_totalpages(pgdat, zones_size, zholes_size);
 
-	if (!pfn_to_page(node_start_pfn))
-		node_alloc_mem_map(pgdat);
+	alloc_node_mem_map(pgdat);
 
 	free_area_init_core(pgdat, zones_size, zholes_size);
 }
_

Re: no-arch-specific-mem_map-init

[Martin J. Bligh]

> Everyone OK with this?

Makes sense to me at least, if it works OK.

> From: Dave Hansen <haveblue@us.ibm.com>
> 
> So, this patch started out with me trying to keep from passing contiguous,
> node-specific mem_map into free_area_init_node() and cousins.  Instead, I
> relied on some calls to pfn_to_page().
> 
> This works fine and dandy when all you need is the pgdat->node_mem_map to
> do pfn_to_page().  However, the non-NUMA/DISCONTIG architectures use the
> real, global mem_map[] instead of a node_mem_map in the pfn_to_page()
> calculation.  So, I ended up effectively trying to initialize mem_map from
> itself, when it was NULL.  That was bad, and caused some very pretty colors
> on someone's screen when he tested it.
> 
> So, I had to make sure to initialize the global mem_map[] before calling
> into free_area_init_node().  Then, I realized how many architectures do
> this on their own, and have comments like this:
> 
>         /* XXX: MRB-remove - this doesn't seem sane, should this be done somewhere else ?*/
>         mem_map = NODE_DATA(0)->node_mem_map;
> 
> The following patch does what my first one did (don't pass mem_map into the
> init functions), incorporates Jesse Barnes' ia64 fixes on top of that, and
> gets rid of all but one of the global mem_map initializations (parisc is
> weird).  It also magically removes more code than it adds.  It could be
> smaller, but I shamelessly added some comments.  

Re: no-arch-specific-mem_map-init

[Paul Mackerras]

Andrew Morton writes:

> Everyone OK with this?

I don't see any problem with it.

Paul.

Re: no-arch-specific-mem_map-init

[Paul Mundt]

[-- Attachment #1: Type: text/plain, Size: 597 bytes --]

On Wed, Mar 09, 2005 at 10:58:33PM -0800, Andrew Morton wrote:
> From: Dave Hansen <haveblue@us.ibm.com>
> 
> So, I had to make sure to initialize the global mem_map[] before calling
> into free_area_init_node().  Then, I realized how many architectures do
> this on their own, and have comments like this:
> 
>         /* XXX: MRB-remove - this doesn't seem sane, should this be done somewhere else ?*/
>         mem_map = NODE_DATA(0)->node_mem_map;
> 
I don't see any problems with this patch either. At least it works fine
for sh64 as well (and sh, where this comment originated).

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: no-arch-specific-mem_map-init

[Matthew Wilcox]

On Wed, Mar 09, 2005 at 10:58:33PM -0800, Andrew Morton wrote:
> Everyone OK with this?

Since I don't understand VM and he's touching everything except my port,
no ;-P

> From: Dave Hansen <haveblue@us.ibm.com>
> The following patch does what my first one did (don't pass mem_map into the
> init functions), incorporates Jesse Barnes' ia64 fixes on top of that, and
> gets rid of all but one of the global mem_map initializations (parisc is
> weird).  It also magically removes more code than it adds.  It could be
> smaller, but I shamelessly added some comments.  

I'm not quite sure why parisc is 'weird'.  Here's the code block in question:

void __init mem_init(void)
{
        high_memory = __va((max_pfn << PAGE_SHIFT));

#ifndef CONFIG_DISCONTIGMEM
        max_mapnr = page_to_pfn(virt_to_page(high_memory - 1)) + 1;
        mem_map = zone_table[ZONE_DMA]->zone_mem_map;
        totalram_pages += free_all_bootmem();
#else
        {
                int i;

                for (i = 0; i < npmem_ranges; i++)
                        totalram_pages += free_all_bootmem_node(NODE_DATA(i));
        }
#endif

Something that may not be intuitively obvious to someone with i386 biases
is that on PA-RISC (at least in the case of CONFIG_DISCONTIGMEM=n), all
memory is in ZONE_DMA.  Does that help divine what the right answer is
for parisc?

Randolph, you touched this code last ... can you help Dave understand it?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: no-arch-specific-mem_map-init

[David Howells]

Matthew Wilcox <matthew@wil.cx> wrote:

> 
> Something that may not be intuitively obvious to someone with i386 biases
> is that on PA-RISC (at least in the case of CONFIG_DISCONTIGMEM=n), all
> memory is in ZONE_DMA.  Does that help divine what the right answer is
> for parisc?

It's probably assuming that since ZONE_DMA is zone 0, the DMA capable zone is
at the bottom of any chunk of memory (node).

David

Re: no-arch-specific-mem_map-init

[Matthew Wilcox]

On Thu, Mar 10, 2005 at 02:30:46PM +0000, David Howells wrote:
> Matthew Wilcox <matthew@wil.cx> wrote:
> 
> > 
> > Something that may not be intuitively obvious to someone with i386 biases
> > is that on PA-RISC (at least in the case of CONFIG_DISCONTIGMEM=n), all
> > memory is in ZONE_DMA.  Does that help divine what the right answer is
> > for parisc?
> 
> It's probably assuming that since ZONE_DMA is zone 0, the DMA capable zone is
> at the bottom of any chunk of memory (node).

Oh, and we don't currently support any of the NUMA PA-RISC boxes, so
there's only the one node ...

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: no-arch-specific-mem_map-init

[Randolph Chung]

> > Something that may not be intuitively obvious to someone with i386 biases
> > is that on PA-RISC (at least in the case of CONFIG_DISCONTIGMEM=n), all
> > memory is in ZONE_DMA.  Does that help divine what the right answer is
> > for parisc?
> 
> It's probably assuming that since ZONE_DMA is zone 0, the DMA capable zone is
> at the bottom of any chunk of memory (node).

All memory on any PARISC boxes (with or without DISCONTIGMEM) falls into
ZONE_DMA, since PARISC boxes have IOMMUs (at last the ones that we care
about for this discussion).

So, 
mem_map = zone_table[ZONE_DMA]->zone_mem_map;
and
mem_map = NODE_DATA(0)->node_mem_map;

should give you the same result on parisc.

Does that help? If you want to change it so we use the latter form of
initialization, that should be ok for us.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: no-arch-specific-mem_map-init

[Martin J. Bligh]

>         mem_map = zone_table[ZONE_DMA]->zone_mem_map;
...
> Something that may not be intuitively obvious to someone with i386 biases
> is that on PA-RISC (at least in the case of CONFIG_DISCONTIGMEM=n), all
> memory is in ZONE_DMA.  Does that help divine what the right answer is
> for parisc?

In which case, you should be fine, you should have

	NODE_DATA(0)->node_mem_map == zone_table[ZONE_DMA]->zone_mem_map

I believe. In free_area_init_core(), we do:

	zone->zone_mem_map = pfn_to_page(zone_start_pfn);

So as long as zone_start_pfn == node_start_pfn, they're equiv. As you say
all your mem is in ZONE_DMA, that should be true.

M.

Re: no-arch-specific-mem_map-init

[Dave Hansen]

On Thu, 2005-03-10 at 07:26 -0800, Martin J. Bligh wrote:
> >         mem_map = zone_table[ZONE_DMA]->zone_mem_map;
> ...
> > Something that may not be intuitively obvious to someone with i386 biases
> > is that on PA-RISC (at least in the case of CONFIG_DISCONTIGMEM=n), all
> > memory is in ZONE_DMA.  Does that help divine what the right answer is
> > for parisc?
> 
> In which case, you should be fine, you should have
> 
> 	NODE_DATA(0)->node_mem_map == zone_table[ZONE_DMA]->zone_mem_map
> 
> I believe. In free_area_init_core(), we do:
> 
> 	zone->zone_mem_map = pfn_to_page(zone_start_pfn);
> 
> So as long as zone_start_pfn == node_start_pfn, they're equiv. As you say
> all your mem is in ZONE_DMA, that should be true.

Now that I look at it again, mem_init() happens way after
paging_init()->free_area_init_node()->alloc_node_mem_map().  So, mem_map
should already be initialized by the time mem_init() gets called.  It
should be safe to just remove the

	mem_map = zone_table[ZONE_DMA]->zone_mem_map;

line in mem_init().

Now that I have your attention, could you stick a printk in there, and
double check that I'm right?  

-- Dave

P.S.  I do a lot of ppc64 work, too, so I have a bias for that as
well. :)

Re: no-arch-specific-mem_map-init

[Matthew Wilcox]

On Thu, Mar 10, 2005 at 07:58:38AM -0800, Dave Hansen wrote:
> Now that I look at it again, mem_init() happens way after
> paging_init()->free_area_init_node()->alloc_node_mem_map().  So, mem_map
> should already be initialized by the time mem_init() gets called.  It
> should be safe to just remove the
> 
> 	mem_map = zone_table[ZONE_DMA]->zone_mem_map;
> 
> line in mem_init().
> 
> Now that I have your attention, could you stick a printk in there, and
> double check that I'm right?  

Yep, I checked, both pointers have value 0x10bbb000, so I just took out
the assignment and committed that to our tree.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

RE: no-arch-specific-mem_map-init

[Luck, Tony]

>Everyone OK with this?

Boot tested on ia64 with tiger and generic configs.

I remain nervous though because this has been an area where
the complexity of ia64 config options has bitten me before.

-Tony

Re: no-arch-specific-mem_map-init

[Russell King]

On Wed, Mar 09, 2005 at 10:58:33PM -0800, Andrew Morton wrote:
> Everyone OK with this?

Looks sane and builds ok.  I'm happy.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core