[PATCH] mm: allocate section_map for sparse_init

[PATCH] mm: allocate section_map for sparse_init

[Yinghai Lu]

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

[PATCH] mm: allocate section_map for sparse_init

allocate section_map in bootmem instead of using __initdata.

need to apply it after
        [PATCH] mm: fix boundary checking in free_bootmem_core
        [PATCH] mm: make mem_map allocation continuous.


Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vmemmap_3.patch --]
[-- Type: text/x-patch; name=vmemmap_3.patch, Size: 1540 bytes --]

Index: linux-2.6/mm/sparse.c
===================================================================
--- linux-2.6.orig/mm/sparse.c
+++ linux-2.6/mm/sparse.c
@@ -285,8 +285,6 @@ struct page __init *sparse_early_mem_map
 	return NULL;
 }
 
-/* section_map pointer array is 64k */
-static __initdata struct page *section_map[NR_MEM_SECTIONS];
 /*
  * Allocate the accumulated non-linear sections, allocate a mem_map
  * for each and record the physical to section mapping.
@@ -296,6 +294,9 @@ void __init sparse_init(void)
 	unsigned long pnum;
 	struct page *map;
 	unsigned long *usemap;
+	struct page **section_map;
+	int size;
+	int node;
 
 	/*
 	 * map is using big page (aka 2M in x86 64 bit)
@@ -305,13 +306,17 @@ void __init sparse_init(void)
 	 * then in big system, the memmory will have a lot hole...
 	 * here try to allocate 2M pages continously.
 	 */
+	size = sizeof(struct page *) * NR_MEM_SECTIONS;
+	section_map = alloc_bootmem(size);
+	if (!section_map)
+		panic("can not allocate section_map\n");
+
 	for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
 		if (!present_section_nr(pnum))
 			continue;
 		section_map[pnum] = sparse_early_mem_map_alloc(pnum);
 	}
 
-
 	for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
 		if (!present_section_nr(pnum))
 			continue;
@@ -327,6 +332,9 @@ void __init sparse_init(void)
 		sparse_init_one_section(__nr_to_section(pnum), pnum, map,
 								usemap);
 	}
+
+	for_each_online_node(node)
+		free_bootmem_node(NODE_DATA(node), __pa(section_map), size);
 }
 
 #ifdef CONFIG_MEMORY_HOTPLUG

Re: [PATCH] mm: allocate section_map for sparse_init

[Mel Gorman]

On (12/03/08 10:51), Yinghai Lu didst pronounce:
> [PATCH] mm: allocate section_map for sparse_init
> 
> allocate section_map in bootmem instead of using __initdata.
> 
> need to apply it after
>         [PATCH] mm: fix boundary checking in free_bootmem_core
>         [PATCH] mm: make mem_map allocation continuous.
> 
> 
> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>

> Index: linux-2.6/mm/sparse.c
> ===================================================================
> --- linux-2.6.orig/mm/sparse.c
> +++ linux-2.6/mm/sparse.c
> @@ -285,8 +285,6 @@ struct page __init *sparse_early_mem_map
>  	return NULL;
>  }
>  
> -/* section_map pointer array is 64k */
> -static __initdata struct page *section_map[NR_MEM_SECTIONS];
>  /*
>   * Allocate the accumulated non-linear sections, allocate a mem_map
>   * for each and record the physical to section mapping.
> @@ -296,6 +294,9 @@ void __init sparse_init(void)
>  	unsigned long pnum;
>  	struct page *map;
>  	unsigned long *usemap;
> +	struct page **section_map;
> +	int size;
> +	int node;
>  
>  	/*
>  	 * map is using big page (aka 2M in x86 64 bit)
> @@ -305,13 +306,17 @@ void __init sparse_init(void)
>  	 * then in big system, the memmory will have a lot hole...
>  	 * here try to allocate 2M pages continously.
>  	 */
> +	size = sizeof(struct page *) * NR_MEM_SECTIONS;
> +	section_map = alloc_bootmem(size);
> +	if (!section_map)
> +		panic("can not allocate section_map\n");
> +
>  	for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
>  		if (!present_section_nr(pnum))
>  			continue;
>  		section_map[pnum] = sparse_early_mem_map_alloc(pnum);
>  	}
>  
> -
>  	for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
>  		if (!present_section_nr(pnum))
>  			continue;
> @@ -327,6 +332,9 @@ void __init sparse_init(void)
>  		sparse_init_one_section(__nr_to_section(pnum), pnum, map,
>  								usemap);
>  	}
> +
> +	for_each_online_node(node)
> +		free_bootmem_node(NODE_DATA(node), __pa(section_map), size);

Why are you iterating every online node here instead of just calling
free_bootmem(__pa(section_map), size) ?

>  }
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG


-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

Re: [PATCH] mm: allocate section_map for sparse_init

[Yinghai Lu]

On Thu, Mar 13, 2008 at 4:19 AM, Mel Gorman <mel@csn.ul.ie> wrote:
> On (12/03/08 10:51), Yinghai Lu didst pronounce:
>
>
> > [PATCH] mm: allocate section_map for sparse_init
>  >
>  > allocate section_map in bootmem instead of using __initdata.
>  >
>  > need to apply it after
>  >         [PATCH] mm: fix boundary checking in free_bootmem_core
>  >         [PATCH] mm: make mem_map allocation continuous.
>  >
>  >
>  > Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
>
>  > Index: linux-2.6/mm/sparse.c
>  > ===================================================================
>  > --- linux-2.6.orig/mm/sparse.c
>  > +++ linux-2.6/mm/sparse.c
>  > @@ -285,8 +285,6 @@ struct page __init *sparse_early_mem_map
>  >       return NULL;
>  >  }
>  >
>  > -/* section_map pointer array is 64k */
>  > -static __initdata struct page *section_map[NR_MEM_SECTIONS];
>  >  /*
>  >   * Allocate the accumulated non-linear sections, allocate a mem_map
>  >   * for each and record the physical to section mapping.
>  > @@ -296,6 +294,9 @@ void __init sparse_init(void)
>  >       unsigned long pnum;
>  >       struct page *map;
>  >       unsigned long *usemap;
>  > +     struct page **section_map;
>  > +     int size;
>  > +     int node;
>  >
>  >       /*
>  >        * map is using big page (aka 2M in x86 64 bit)
>  > @@ -305,13 +306,17 @@ void __init sparse_init(void)
>  >        * then in big system, the memmory will have a lot hole...
>  >        * here try to allocate 2M pages continously.
>  >        */
>  > +     size = sizeof(struct page *) * NR_MEM_SECTIONS;
>  > +     section_map = alloc_bootmem(size);
>  > +     if (!section_map)
>  > +             panic("can not allocate section_map\n");
>  > +
>  >       for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
>  >               if (!present_section_nr(pnum))
>  >                       continue;
>  >               section_map[pnum] = sparse_early_mem_map_alloc(pnum);
>  >       }
>  >
>  > -
>  >       for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
>  >               if (!present_section_nr(pnum))
>  >                       continue;
>  > @@ -327,6 +332,9 @@ void __init sparse_init(void)
>  >               sparse_init_one_section(__nr_to_section(pnum), pnum, map,
>  >                                                               usemap);
>  >       }
>  > +
>  > +     for_each_online_node(node)
>  > +             free_bootmem_node(NODE_DATA(node), __pa(section_map), size);
>
>  Why are you iterating every online node here instead of just calling
>  free_bootmem(__pa(section_map), size) ?

free_bootmem will assume use bdata on NODE_DATA(0).

some cases: four nodes system: only have memory installed for node 1,
and node 3.
alloc_bootmem will loop to get bootmem from node1, because there is no
ram node0, and not NODE_DATA(0) ...

we may update free_bootmem to loop all bdata or all online nodes to
call free_bootmem_core...

YH

Re: [PATCH] mm: allocate section_map for sparse_init

[Mel Gorman]

On (13/03/08 11:02), Yinghai Lu didst pronounce:
> On Thu, Mar 13, 2008 at 4:19 AM, Mel Gorman <mel@csn.ul.ie> wrote:
> > On (12/03/08 10:51), Yinghai Lu didst pronounce:
> >
> >
> > > [PATCH] mm: allocate section_map for sparse_init
> >  >
> >  > allocate section_map in bootmem instead of using __initdata.
> >  >
> >  > need to apply it after
> >  >         [PATCH] mm: fix boundary checking in free_bootmem_core
> >  >         [PATCH] mm: make mem_map allocation continuous.
> >  >
> >  >
> >  > Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
> >
> >  > Index: linux-2.6/mm/sparse.c
> >  > ===================================================================
> >  > --- linux-2.6.orig/mm/sparse.c
> >  > +++ linux-2.6/mm/sparse.c
> >  > @@ -285,8 +285,6 @@ struct page __init *sparse_early_mem_map
> >  >       return NULL;
> >  >  }
> >  >
> >  > -/* section_map pointer array is 64k */
> >  > -static __initdata struct page *section_map[NR_MEM_SECTIONS];
> >  >  /*
> >  >   * Allocate the accumulated non-linear sections, allocate a mem_map
> >  >   * for each and record the physical to section mapping.
> >  > @@ -296,6 +294,9 @@ void __init sparse_init(void)
> >  >       unsigned long pnum;
> >  >       struct page *map;
> >  >       unsigned long *usemap;
> >  > +     struct page **section_map;
> >  > +     int size;
> >  > +     int node;
> >  >
> >  >       /*
> >  >        * map is using big page (aka 2M in x86 64 bit)
> >  > @@ -305,13 +306,17 @@ void __init sparse_init(void)
> >  >        * then in big system, the memmory will have a lot hole...
> >  >        * here try to allocate 2M pages continously.
> >  >        */
> >  > +     size = sizeof(struct page *) * NR_MEM_SECTIONS;
> >  > +     section_map = alloc_bootmem(size);
> >  > +     if (!section_map)
> >  > +             panic("can not allocate section_map\n");
> >  > +
> >  >       for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
> >  >               if (!present_section_nr(pnum))
> >  >                       continue;
> >  >               section_map[pnum] = sparse_early_mem_map_alloc(pnum);
> >  >       }
> >  >
> >  > -
> >  >       for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
> >  >               if (!present_section_nr(pnum))
> >  >                       continue;
> >  > @@ -327,6 +332,9 @@ void __init sparse_init(void)
> >  >               sparse_init_one_section(__nr_to_section(pnum), pnum, map,
> >  >                                                               usemap);
> >  >       }
> >  > +
> >  > +     for_each_online_node(node)
> >  > +             free_bootmem_node(NODE_DATA(node), __pa(section_map), size);
> >
> >  Why are you iterating every online node here instead of just calling
> >  free_bootmem(__pa(section_map), size) ?
> 
> free_bootmem will assume use bdata on NODE_DATA(0).
> 

True. The unwritten assumption is that NODE_DATA(0) always has memory
for allocate or free.

> some cases: four nodes system: only have memory installed for node 1,
> and node 3.
> alloc_bootmem will loop to get bootmem from node1, because there is no
> ram node0, and not NODE_DATA(0) ...
> 
> we may update free_bootmem to loop all bdata or all online nodes to
> call free_bootmem_core...
> 

Alternatively, update free_bootmem so that it figures out which node it is
meant to be freeing to based on the PFN of the page currently being freed.

It still doesn't seem right to try freeing on all online nodes as it looks like
free_bootmem_core() should flip bits outside the range of its node_bootmem_map
which could cause tricky bugs.

As things currently stand with bootmem, what you should be doing is using
for_each_online_node() to figure out which node you can allocate from,
remembering that node and calling free_bootmem_node() once. If you fix
free_bootmem() though, you should only need to call free_bootmem() once
in this patch.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

Re: [PATCH] mm: allocate section_map for sparse_init

[Yinghai Lu]

On Tue, Mar 18, 2008 at 2:41 AM, Mel Gorman <mel@csn.ul.ie> wrote:
> On (13/03/08 11:02), Yinghai Lu didst pronounce:
>
>
> > On Thu, Mar 13, 2008 at 4:19 AM, Mel Gorman <mel@csn.ul.ie> wrote:
>  > > On (12/03/08 10:51), Yinghai Lu didst pronounce:
>  > >
>  > >
>  > > > [PATCH] mm: allocate section_map for sparse_init
>  > >  >
>  > >  > allocate section_map in bootmem instead of using __initdata.
>  > >  >
>  > >  > need to apply it after
>  > >  >         [PATCH] mm: fix boundary checking in free_bootmem_core
>  > >  >         [PATCH] mm: make mem_map allocation continuous.
>  > >  >
>  > >  >
>  > >  > Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
>  > >
>  > >  > Index: linux-2.6/mm/sparse.c
>  > >  > ===================================================================
>  > >  > --- linux-2.6.orig/mm/sparse.c
>  > >  > +++ linux-2.6/mm/sparse.c
>  > >  > @@ -285,8 +285,6 @@ struct page __init *sparse_early_mem_map
>  > >  >       return NULL;
>  > >  >  }
>  > >  >
>  > >  > -/* section_map pointer array is 64k */
>  > >  > -static __initdata struct page *section_map[NR_MEM_SECTIONS];
>  > >  >  /*
>  > >  >   * Allocate the accumulated non-linear sections, allocate a mem_map
>  > >  >   * for each and record the physical to section mapping.
>  > >  > @@ -296,6 +294,9 @@ void __init sparse_init(void)
>  > >  >       unsigned long pnum;
>  > >  >       struct page *map;
>  > >  >       unsigned long *usemap;
>  > >  > +     struct page **section_map;
>  > >  > +     int size;
>  > >  > +     int node;
>  > >  >
>  > >  >       /*
>  > >  >        * map is using big page (aka 2M in x86 64 bit)
>  > >  > @@ -305,13 +306,17 @@ void __init sparse_init(void)
>  > >  >        * then in big system, the memmory will have a lot hole...
>  > >  >        * here try to allocate 2M pages continously.
>  > >  >        */
>  > >  > +     size = sizeof(struct page *) * NR_MEM_SECTIONS;
>  > >  > +     section_map = alloc_bootmem(size);
>  > >  > +     if (!section_map)
>  > >  > +             panic("can not allocate section_map\n");
>  > >  > +
>  > >  >       for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
>  > >  >               if (!present_section_nr(pnum))
>  > >  >                       continue;
>  > >  >               section_map[pnum] = sparse_early_mem_map_alloc(pnum);
>  > >  >       }
>  > >  >
>  > >  > -
>  > >  >       for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
>  > >  >               if (!present_section_nr(pnum))
>  > >  >                       continue;
>  > >  > @@ -327,6 +332,9 @@ void __init sparse_init(void)
>  > >  >               sparse_init_one_section(__nr_to_section(pnum), pnum, map,
>  > >  >                                                               usemap);
>  > >  >       }
>  > >  > +
>  > >  > +     for_each_online_node(node)
>  > >  > +             free_bootmem_node(NODE_DATA(node), __pa(section_map), size);
>  > >
>  > >  Why are you iterating every online node here instead of just calling
>  > >  free_bootmem(__pa(section_map), size) ?
>  >
>  > free_bootmem will assume use bdata on NODE_DATA(0).
>  >
>
>  True. The unwritten assumption is that NODE_DATA(0) always has memory
>  for allocate or free.
>
>
>  > some cases: four nodes system: only have memory installed for node 1,
>  > and node 3.
>  > alloc_bootmem will loop to get bootmem from node1, because there is no
>  > ram node0, and not NODE_DATA(0) ...
>  >
>  > we may update free_bootmem to loop all bdata or all online nodes to
>  > call free_bootmem_core...
>  >
>
>  Alternatively, update free_bootmem so that it figures out which node it is
>  meant to be freeing to based on the PFN of the page currently being freed.
>
>  It still doesn't seem right to try freeing on all online nodes as it looks like
>  free_bootmem_core() should flip bits outside the range of its node_bootmem_map
>  which could cause tricky bugs.
>
>  As things currently stand with bootmem, what you should be doing is using
>  for_each_online_node() to figure out which node you can allocate from,
>  remembering that node and calling free_bootmem_node() once. If you fix
>  free_bootmem() though, you should only need to call free_bootmem() once
>  in this patch.

Yes

with the new free_bootmem that loop bdata_list, we can call
free_bootmem.

YH