Re: [PATCH] sparsemem interaction with memory add bug fixes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <[email protected]> and Joel Schopp <[email protected]>.
> 
> Signed-off-by: Mike Kravetz <[email protected]>
> 
> 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;
 }

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux