Messages in this thread Patch in this message | | | Date | Wed, 12 Apr 2006 14:22:56 +0100 | From | Andy Whitcroft <> | Subject | Re: [PATCH] sparsemem interaction with memory add bug fixes |
| |
Mike Kravetz wrote: > This patch fixes two bugs with the way sparsemem interacts with memory > add. They are: > - memory leak if memmap for section already exists > - calling alloc_bootmem_node() after boot > These bugs were discovered and a first cut at the fixes were provided by > Arnd Bergmann <arnd@arndb.de> and Joel Schopp <jschopp@us.ibm.com>. > > Signed-off-by: Mike Kravetz <kravetz@us.ibm.com> > > diff -Naupr linux-2.6.17-rc1-mm2/mm/sparse.c linux-2.6.17-rc1-mm2.work/mm/sparse.c > --- linux-2.6.17-rc1-mm2/mm/sparse.c 2006-04-03 03:22:10.000000000 +0000 > +++ linux-2.6.17-rc1-mm2.work/mm/sparse.c 2006-04-11 23:32:10.000000000 +0000 > @@ -32,7 +32,10 @@ static struct mem_section *sparse_index_ > unsigned long array_size = SECTIONS_PER_ROOT * > sizeof(struct mem_section); > > - section = alloc_bootmem_node(NODE_DATA(nid), array_size); > + if (system_state == SYSTEM_RUNNING) > + section = kmalloc_node(array_size, GFP_KERNEL, nid); > + else > + section = alloc_bootmem_node(NODE_DATA(nid), array_size); > > if (section) > memset(section, 0, array_size); > @@ -281,9 +284,9 @@ int sparse_add_one_section(struct zone * > > ret = sparse_init_one_section(ms, section_nr, memmap); > > +out: > if (ret <= 0) > __kfree_section_memmap(memmap, nr_pages); > -out: > pgdat_resize_unlock(pgdat, &flags); > return ret; > }
First change looks sane. For the second it makes it obvious that we are freeing the alloc'd section within the pgdat resize lock. Doesn't seem to make any sense to do that to me? Perhaps it should be more like the attached.
-apw
sparse.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff -upN reference/mm/sparse.c current/mm/sparse.c --- reference/mm/sparse.c +++ current/mm/sparse.c @@ -281,9 +281,9 @@ int sparse_add_one_section(struct zone * ret = sparse_init_one_section(ms, section_nr, memmap); - if (ret <= 0) - __kfree_section_memmap(memmap, nr_pages); out: pgdat_resize_unlock(pgdat, &flags); + if (ret <= 0) + __kfree_section_memmap(memmap, nr_pages); return ret; } | |