Re: Patch "percpu: fix a memory leak in pcpu_extend_area_map()" has been added to the 2.6.35-stable tree

Re: Patch "percpu: fix a memory leak in pcpu_extend_area_map()" has been added to the 2.6.35-stable tree

[Stefan Lippers-Hollmann]

[-- Attachment #1: Type: Text/Plain, Size: 1625 bytes --]

Hi

On Thursday 16 September 2010, gregkh@suse.de wrote:
> This is a note to let you know that I've just added the patch titled
>     percpu: fix a memory leak in pcpu_extend_area_map()
> to the 2.6.35-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
[...]
> From a002d148426f40bc2b7dc066982eb177cdebeaaa Mon Sep 17 00:00:00 2001
> From: Huang Shijie <shijie8@gmail.com>
> Date: Sun, 8 Aug 2010 14:39:07 +0200
> Subject: percpu: fix a memory leak in pcpu_extend_area_map()
> 
> From: Huang Shijie <shijie8@gmail.com>
> 
> commit a002d148426f40bc2b7dc066982eb177cdebeaaa upstream.
> 
> The original code did not free the old map.  This patch fixes it.
> 
> tj: use @old as memcpy source instead of @chunk->map, and indentation
>     and description update

This patch, as part of the current -stable queue-2.6.35, breaks booting for
me on nforce4/ AMD64 X2. Unfortunately it fails even before the serial 
console is initialized and the only messages shown on screen are:

Decompressing Linux... Parsing ELF... done.
Booting kernel.

[...]
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -390,7 +390,9 @@ static int pcpu_extend_area_map(struct p
>  		goto out_unlock;
>  
>  	old_size = chunk->map_alloc * sizeof(chunk->map[0]);
> -	memcpy(new, chunk->map, old_size);
> +	old = chunk->map;
> +
> +	memcpy(new, old, old_size);
>  
>  	/*
>  	 * map_alloc < PCPU_DFL_MAP_ALLOC indicates that the chunk is

Reverting just this patch from the current queue-2.6.35 fixes the issue for
me; gzipped kernel config (amd64) attached.

Regards
	Stefan Lippers-Hollmann

[-- Attachment #2: config-2.6.35-4.slh.11.9-aptosid-amd64.gz --]
[-- Type: application/x-gzip, Size: 28163 bytes --]

Re: Patch "percpu: fix a memory leak in pcpu_extend_area_map()" has been added to the 2.6.35-stable tree

[Greg KH]

On Thu, Sep 16, 2010 at 11:39:02PM +0200, Stefan Lippers-Hollmann wrote:
> Hi
> 
> On Thursday 16 September 2010, gregkh@suse.de wrote:
> > This is a note to let you know that I've just added the patch titled
> >     percpu: fix a memory leak in pcpu_extend_area_map()
> > to the 2.6.35-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> [...]
> > From a002d148426f40bc2b7dc066982eb177cdebeaaa Mon Sep 17 00:00:00 2001
> > From: Huang Shijie <shijie8@gmail.com>
> > Date: Sun, 8 Aug 2010 14:39:07 +0200
> > Subject: percpu: fix a memory leak in pcpu_extend_area_map()
> > 
> > From: Huang Shijie <shijie8@gmail.com>
> > 
> > commit a002d148426f40bc2b7dc066982eb177cdebeaaa upstream.
> > 
> > The original code did not free the old map.  This patch fixes it.
> > 
> > tj: use @old as memcpy source instead of @chunk->map, and indentation
> >     and description update
> 
> This patch, as part of the current -stable queue-2.6.35, breaks booting for
> me on nforce4/ AMD64 X2. Unfortunately it fails even before the serial 
> console is initialized and the only messages shown on screen are:
> 
> Decompressing Linux... Parsing ELF... done.
> Booting kernel.
> 
> [...]
> > --- a/mm/percpu.c
> > +++ b/mm/percpu.c
> > @@ -390,7 +390,9 @@ static int pcpu_extend_area_map(struct p
> >  		goto out_unlock;
> >  
> >  	old_size = chunk->map_alloc * sizeof(chunk->map[0]);
> > -	memcpy(new, chunk->map, old_size);
> > +	old = chunk->map;
> > +
> > +	memcpy(new, old, old_size);
> >  
> >  	/*
> >  	 * map_alloc < PCPU_DFL_MAP_ALLOC indicates that the chunk is
> 
> Reverting just this patch from the current queue-2.6.35 fixes the issue for
> me; gzipped kernel config (amd64) attached.

Does Linus's tree also cause the same problems for you?

thanks,

greg k-h

Re: Patch "percpu: fix a memory leak in pcpu_extend_area_map()" has been added to the 2.6.35-stable tree

[Stefan Lippers-Hollmann]

Hi

On Friday 17 September 2010, Greg KH wrote:
> On Thu, Sep 16, 2010 at 11:39:02PM +0200, Stefan Lippers-Hollmann wrote:
[...]
> > On Thursday 16 September 2010, gregkh@suse.de wrote:
> > > This is a note to let you know that I've just added the patch titled
> > >     percpu: fix a memory leak in pcpu_extend_area_map()
> > > to the 2.6.35-stable tree which can be found at:
> > >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > [...]
> > > From a002d148426f40bc2b7dc066982eb177cdebeaaa Mon Sep 17 00:00:00 2001
> > > From: Huang Shijie <shijie8@gmail.com>
> > > Date: Sun, 8 Aug 2010 14:39:07 +0200
> > > Subject: percpu: fix a memory leak in pcpu_extend_area_map()
> > > 
> > > From: Huang Shijie <shijie8@gmail.com>
> > > 
> > > commit a002d148426f40bc2b7dc066982eb177cdebeaaa upstream.
> > > 
> > > The original code did not free the old map.  This patch fixes it.
> > > 
> > > tj: use @old as memcpy source instead of @chunk->map, and indentation
> > >     and description update
> > 
> > This patch, as part of the current -stable queue-2.6.35, breaks booting for
> > me on nforce4/ AMD64 X2. Unfortunately it fails even before the serial 
> > console is initialized and the only messages shown on screen are:
> > 
> > Decompressing Linux... Parsing ELF... done.
> > Booting kernel.
[...]
> > > --- a/mm/percpu.c
> > > +++ b/mm/percpu.c
> > > @@ -390,7 +390,9 @@ static int pcpu_extend_area_map(struct p
> > >  		goto out_unlock;
> > >  
> > >  	old_size = chunk->map_alloc * sizeof(chunk->map[0]);
> > > -	memcpy(new, chunk->map, old_size);
> > > +	old = chunk->map;
> > > +
> > > +	memcpy(new, old, old_size);
> > >  
> > >  	/*
> > >  	 * map_alloc < PCPU_DFL_MAP_ALLOC indicates that the chunk is
> > 
> > Reverting just this patch from the current queue-2.6.35 fixes the issue for
> > me; gzipped kernel config (amd64) attached.
> 
> Does Linus's tree also cause the same problems for you?

Current linux-2.6 HEAD (v2.6.36-rc4-134-g03a7ab0) boots and works fine with
a config very closely derived from the previous one.

Regards
	Stefan Lippers-Hollmann

Re: Patch "percpu: fix a memory leak in pcpu_extend_area_map()" has been added to the 2.6.35-stable tree

[Huang Shijie]

hi :

    I think this maybe caused by the kfree/vfree.
    I do not have any AMD machine. Could you add some log to debug it
?  such as in pcpu_extend_area_map():

        ================================
        printk(KERN_INFO "[ %s ] line : %d\n", __func__, __LINE__);
	pcpu_mem_free(old, old_size);
        printk(KERN_INFO "[ %s ] line : %d\n", __func__, __LINE__);
        pcpu_mem_free(new, new_size);
        printk(KERN_INFO "[ %s ] line : %d\n", __func__, __LINE__);

        ================================
  The system will print the log before it hung.

Best Regards.
Huang Shijie


2010/9/17 Stefan Lippers-Hollmann <s.L-H@gmx.de>:
> Hi
>
> On Thursday 16 September 2010, gregkh@suse.de wrote:
>> This is a note to let you know that I've just added the patch titled
>>     percpu: fix a memory leak in pcpu_extend_area_map()
>> to the 2.6.35-stable tree which can be found at:
>>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> [...]
>> From a002d148426f40bc2b7dc066982eb177cdebeaaa Mon Sep 17 00:00:00 2001
>> From: Huang Shijie <shijie8@gmail.com>
>> Date: Sun, 8 Aug 2010 14:39:07 +0200
>> Subject: percpu: fix a memory leak in pcpu_extend_area_map()
>>
>> From: Huang Shijie <shijie8@gmail.com>
>>
>> commit a002d148426f40bc2b7dc066982eb177cdebeaaa upstream.
>>
>> The original code did not free the old map.  This patch fixes it.
>>
>> tj: use @old as memcpy source instead of @chunk->map, and indentation
>>     and description update
>
> This patch, as part of the current -stable queue-2.6.35, breaks booting for
> me on nforce4/ AMD64 X2. Unfortunately it fails even before the serial
> console is initialized and the only messages shown on screen are:
>
> Decompressing Linux... Parsing ELF... done.
> Booting kernel.
>
> [...]
>> --- a/mm/percpu.c
>> +++ b/mm/percpu.c
>> @@ -390,7 +390,9 @@ static int pcpu_extend_area_map(struct p
>>               goto out_unlock;
>>
>>       old_size = chunk->map_alloc * sizeof(chunk->map[0]);
>> -     memcpy(new, chunk->map, old_size);
>> +     old = chunk->map;
>> +
>> +     memcpy(new, old, old_size);
>>
>>       /*
>>        * map_alloc < PCPU_DFL_MAP_ALLOC indicates that the chunk is
>
> Reverting just this patch from the current queue-2.6.35 fixes the issue for
> me; gzipped kernel config (amd64) attached.
>
> Regards
>        Stefan Lippers-Hollmann
>

Re: Patch "percpu: fix a memory leak in pcpu_extend_area_map()" has been added to the 2.6.35-stable tree

[Stefan Lippers-Hollmann]

Hi

On Friday 17 September 2010, Huang Shijie wrote:
> hi :
> 
>     I think this maybe caused by the kfree/vfree.
>     I do not have any AMD machine. Could you add some log to debug it
> ?  such as in pcpu_extend_area_map():
> 
>         ================================
[...]
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -390,7 +390,9 @@ static int pcpu_extend_area_map(struct p
 		goto out_unlock;
 
 	old_size = chunk->map_alloc * sizeof(chunk->map[0]);
-	memcpy(new, chunk->map, old_size);
+	old = chunk->map;
+
+	memcpy(new, old, old_size);
 
 	/*
 	 * map_alloc < PCPU_DFL_MAP_ALLOC indicates that the chunk is
@@ -410,8 +412,11 @@ out_unlock:
 	 * pcpu_mem_free() might end up calling vfree() which uses
 	 * IRQ-unsafe lock and thus can't be called under pcpu_lock.
 	 */
+	printk(KERN_INFO "[ %s ] line : %d\n", __func__, __LINE__);
 	pcpu_mem_free(old, old_size);
+	printk(KERN_INFO "[ %s ] line : %d\n", __func__, __LINE__);
 	pcpu_mem_free(new, new_size);
+	printk(KERN_INFO "[ %s ] line : %d\n", __func__, __LINE__);
 
 	return 0;
 }
>         ================================
>   The system will print the log before it hung.

Unfortunately those printk lines aren't reached and the systems freezes
earlier.

> Best Regards.
> Huang Shijie
> 
> 
> 2010/9/17 Stefan Lippers-Hollmann <s.L-H@gmx.de>:
[...]
> >> to the 2.6.35-stable tree which can be found at:
> >>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > [...]
> >> From a002d148426f40bc2b7dc066982eb177cdebeaaa Mon Sep 17 00:00:00 2001
> >> From: Huang Shijie <shijie8@gmail.com>
> >> Date: Sun, 8 Aug 2010 14:39:07 +0200
> >> Subject: percpu: fix a memory leak in pcpu_extend_area_map()
[...]
> > This patch, as part of the current -stable queue-2.6.35, breaks booting for
> > me on nforce4/ AMD64 X2. Unfortunately it fails even before the serial
> > console is initialized and the only messages shown on screen are:
> >
> > Decompressing Linux... Parsing ELF... done.
> > Booting kernel.

I'm still stuck on those lines, before the system freezes, but I can 
reproduce the same issues in Intel Q9550/ ICH10 (x86_64) and Intel 
Pentium M (Banias)/ ICH4-M.

All systems use current Debian/ unstable (gcc-4.4 4.4.4-14, binutils 
2.20.1-14) and are booted by grub2 (grub-pc 1.98+20100804-4). 
Queue-2.6.35 with this patch backed out and current linux-2.6 HEAD 
(v2.6.36-rc4-134-g03a7ab0) are working fine on all tested systems.

Regards
	Stefan Lippers-Hollmann