public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
* ext4 memory leak (was Re: [PATCH] x86: _edata should include all .data.* sections on X86_64)
       [not found]               ` <4A5C5FD0.3020204-M18mAb7Tlt0yCq4wW13eYl6hYfS7NtTn@public.gmane.org>
@ 2009-07-14 12:26                 ` Catalin Marinas
  2009-07-15  8:03                   ` Aneesh Kumar K.V
  0 siblings, 1 reply; 8+ messages in thread
From: Catalin Marinas @ 2009-07-14 12:26 UTC (permalink / raw)
  To: Alexey Fisher
  Cc: Pekka Enberg, Kernel Testers List,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Sam Ravnborg, Ingo Molnar, linux-ext4-u79uwXL29TY76Z2rM5mHXA

(I cc'ed linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org as well)

On Tue, 2009-07-14 at 12:37 +0200, Alexey Fisher wrote:
> this is complete trace from debug/kmemleak .
[...]
> i will compile now latest linux-arm.org/linux-2.6.git
> unreferenced object 0xffff880132c48890 (size 1024):
>    comm "exe", pid 1612, jiffies 4294894130
>    backtrace:
>      [<ffffffff810fbaca>] create_object+0x13a/0x2c0
>      [<ffffffff810fbd75>] kmemleak_alloc+0x25/0x60
>      [<ffffffff810f596b>] __kmalloc+0x11b/0x210
>      [<ffffffff811ae061>] ext4_mb_init+0x1b1/0x5c0
>      [<ffffffff8119f1e9>] ext4_fill_super+0x1e29/0x2720
>      [<ffffffff8110111f>] get_sb_bdev+0x16f/0x1b0
>      [<ffffffff81195413>] ext4_get_sb+0x13/0x20
>      [<ffffffff81100bf6>] vfs_kern_mount+0x76/0x180
>      [<ffffffff81100d6d>] do_kern_mount+0x4d/0x120
>      [<ffffffff81118ee7>] do_mount+0x307/0x8b0
>      [<ffffffff8111951f>] sys_mount+0x8f/0xe0
>      [<ffffffff8100b66b>] system_call_fastpath+0x16/0x1b
>      [<ffffffffffffffff>] 0xffffffffffffffff

After some investigation, this looks to me like a real leak.

I managed to reproduce something similar (though the size may differ, I
think depending on filesystem size - only tried with a 64MB loop
device):

unreferenced object 0xde468300 (size 32):
  comm "mount", pid 1445, jiffies 4294950074
  backtrace:
    [<c006d473>] __save_stack_trace+0x17/0x1c
    [<c006d545>] create_object+0xcd/0x188
    [<c01efe43>] kmemleak_alloc+0x1b/0x3c
    [<c006c013>] __kmalloc+0xd7/0xe4
    [<c00c1029>] ext4_mb_init+0x14d/0x374
    [<c00b7d7d>] ext4_fill_super+0x1385/0x16b4
    [<c0070891>] get_sb_bdev+0xa9/0xe4
    [<c00b574b>] ext4_get_sb+0xf/0x14
    [<c006fd3f>] vfs_kern_mount+0x33/0x64
    [<c006fda5>] do_kern_mount+0x25/0x8c
    [<c007e11f>] do_mount+0x47f/0x4c4
    [<c007e1b5>] sys_mount+0x51/0x80
    [<c0027c01>] ret_fast_syscall+0x1/0x40
    [<ffffffff>] 0xffffffff

The above block is the meta_group_info allocated in
ext4_mb_init_backend() and stored in sbi->s_group_info[i] (i = 0 in my
case). Adding printk's and and inspecting the memory at
sbi->s_group_info[] shows different value stored, not the pointer
reported as leak.

About the new pointer at sbi->s_group_info[0], kmemleak has this
information (via the dump= option in my branch; it isn't a leak report):

kmemleak: Object 0xdfebfa80 (size 128):
kmemleak:   comm "mount", pid 1445, jiffies 4294950075
kmemleak:   min_count = 1
kmemleak:   count = 1
kmemleak:   flags = 0x1
kmemleak:   backtrace:
     [<c006d473>] __save_stack_trace+0x17/0x1c
     [<c006d545>] create_object+0xcd/0x188
     [<c01efe43>] kmemleak_alloc+0x1b/0x3c
     [<c006c013>] __kmalloc+0xd7/0xe4
     [<c00c0df1>] ext4_mb_add_groupinfo+0x29/0x114
     [<c00c107f>] ext4_mb_init+0x1a3/0x374
     [<c00b7d7d>] ext4_fill_super+0x1385/0x16b4
     [<c0070891>] get_sb_bdev+0xa9/0xe4
     [<c00b574b>] ext4_get_sb+0xf/0x14
     [<c006fd3f>] vfs_kern_mount+0x33/0x64
     [<c006fda5>] do_kern_mount+0x25/0x8c
     [<c007e11f>] do_mount+0x47f/0x4c4
     [<c007e1b5>] sys_mount+0x51/0x80
     [<c0027c01>] ret_fast_syscall+0x1/0x40
     [<ffffffff>] 0xffffffff

So, ext4_mb_add_groupinfo() is overriding the pointers stored in
sbi->s_group_info[] by the ext4_mb_init_backend() function without
freeing them first.

Maybe the ext4 people could clarify what is happening here as I'm not
familiar with the code.

-- 
Catalin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ext4 memory leak (was Re: [PATCH] x86: _edata should include all .data.* sections on X86_64)
  2009-07-14 12:26                 ` ext4 memory leak (was Re: [PATCH] x86: _edata should include all .data.* sections on X86_64) Catalin Marinas
@ 2009-07-15  8:03                   ` Aneesh Kumar K.V
  2009-07-15  8:54                     ` Alexey Fisher
  2009-07-15 10:33                     ` Catalin Marinas
  0 siblings, 2 replies; 8+ messages in thread
From: Aneesh Kumar K.V @ 2009-07-15  8:03 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Alexey Fisher, Pekka Enberg, Kernel Testers List,
	linux-kernel@vger.kernel.org, Sam Ravnborg, Ingo Molnar,
	linux-ext4

On Tue, Jul 14, 2009 at 01:26:30PM +0100, Catalin Marinas wrote:
> (I cc'ed linux-ext4@vger.kernel.org as well)
> 
> On Tue, 2009-07-14 at 12:37 +0200, Alexey Fisher wrote:
> > this is complete trace from debug/kmemleak .
> [...]
> > i will compile now latest linux-arm.org/linux-2.6.git
> > unreferenced object 0xffff880132c48890 (size 1024):
> >    comm "exe", pid 1612, jiffies 4294894130
> >    backtrace:
> >      [<ffffffff810fbaca>] create_object+0x13a/0x2c0
> >      [<ffffffff810fbd75>] kmemleak_alloc+0x25/0x60
> >      [<ffffffff810f596b>] __kmalloc+0x11b/0x210
> >      [<ffffffff811ae061>] ext4_mb_init+0x1b1/0x5c0
> >      [<ffffffff8119f1e9>] ext4_fill_super+0x1e29/0x2720
> >      [<ffffffff8110111f>] get_sb_bdev+0x16f/0x1b0
> >      [<ffffffff81195413>] ext4_get_sb+0x13/0x20
> >      [<ffffffff81100bf6>] vfs_kern_mount+0x76/0x180
> >      [<ffffffff81100d6d>] do_kern_mount+0x4d/0x120
> >      [<ffffffff81118ee7>] do_mount+0x307/0x8b0
> >      [<ffffffff8111951f>] sys_mount+0x8f/0xe0
> >      [<ffffffff8100b66b>] system_call_fastpath+0x16/0x1b
> >      [<ffffffffffffffff>] 0xffffffffffffffff
> 
> After some investigation, this looks to me like a real leak.
> 
> I managed to reproduce something similar (though the size may differ, I
> think depending on filesystem size - only tried with a 64MB loop
> device):
> 
> unreferenced object 0xde468300 (size 32):
>   comm "mount", pid 1445, jiffies 4294950074
>   backtrace:
>     [<c006d473>] __save_stack_trace+0x17/0x1c
>     [<c006d545>] create_object+0xcd/0x188
>     [<c01efe43>] kmemleak_alloc+0x1b/0x3c
>     [<c006c013>] __kmalloc+0xd7/0xe4
>     [<c00c1029>] ext4_mb_init+0x14d/0x374
>     [<c00b7d7d>] ext4_fill_super+0x1385/0x16b4
>     [<c0070891>] get_sb_bdev+0xa9/0xe4
>     [<c00b574b>] ext4_get_sb+0xf/0x14
>     [<c006fd3f>] vfs_kern_mount+0x33/0x64
>     [<c006fda5>] do_kern_mount+0x25/0x8c
>     [<c007e11f>] do_mount+0x47f/0x4c4
>     [<c007e1b5>] sys_mount+0x51/0x80
>     [<c0027c01>] ret_fast_syscall+0x1/0x40
>     [<ffffffff>] 0xffffffff
> 
> The above block is the meta_group_info allocated in
> ext4_mb_init_backend() and stored in sbi->s_group_info[i] (i = 0 in my
> case). Adding printk's and and inspecting the memory at
> sbi->s_group_info[] shows different value stored, not the pointer
> reported as leak.
> 
> About the new pointer at sbi->s_group_info[0], kmemleak has this
> information (via the dump= option in my branch; it isn't a leak report):
> 
> kmemleak: Object 0xdfebfa80 (size 128):
> kmemleak:   comm "mount", pid 1445, jiffies 4294950075
> kmemleak:   min_count = 1
> kmemleak:   count = 1
> kmemleak:   flags = 0x1
> kmemleak:   backtrace:
>      [<c006d473>] __save_stack_trace+0x17/0x1c
>      [<c006d545>] create_object+0xcd/0x188
>      [<c01efe43>] kmemleak_alloc+0x1b/0x3c
>      [<c006c013>] __kmalloc+0xd7/0xe4
>      [<c00c0df1>] ext4_mb_add_groupinfo+0x29/0x114
>      [<c00c107f>] ext4_mb_init+0x1a3/0x374
>      [<c00b7d7d>] ext4_fill_super+0x1385/0x16b4
>      [<c0070891>] get_sb_bdev+0xa9/0xe4
>      [<c00b574b>] ext4_get_sb+0xf/0x14
>      [<c006fd3f>] vfs_kern_mount+0x33/0x64
>      [<c006fda5>] do_kern_mount+0x25/0x8c
>      [<c007e11f>] do_mount+0x47f/0x4c4
>      [<c007e1b5>] sys_mount+0x51/0x80
>      [<c0027c01>] ret_fast_syscall+0x1/0x40
>      [<ffffffff>] 0xffffffff
> 
> So, ext4_mb_add_groupinfo() is overriding the pointers stored in
> sbi->s_group_info[] by the ext4_mb_init_backend() function without
> freeing them first.
> 
> Maybe the ext4 people could clarify what is happening here as I'm not
> familiar with the code.
> 

Can you try this patch ?

commit 4cc505d4c16c86f8f590ce4b288c920572bf2be9
Author: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Date:   Wed Jul 15 13:20:37 2009 +0530

    ext4: Memory leak fix ext4_group_info allocation.
    
    commit 5f21b0e642d7bf6fe4434c9ba12bc9cb96b17cf7  was done to
    reallocate groupinfo struct during resize properly. That goal
    was to allocate new groupinfo struct when we are adding new block
    groups during resize. Calling ext4_mb_add_group_info in the
    mballoc initialization code path resulted in we reallocating
    the group info struct . Fix this by not separately allocating
    group info in the mballoc init path and always depend on
    ext4_mb_add_group_info to allocate group info struct.
    
    The earlier code also had a bug that we allocated less number of
    group info struct for the last meta group. But on resize we
    expected that we had EXT4_DESC_PER_BLOCK group info struct for
    each meta group.
    
    Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 519a0a6..96ed1d8 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2615,22 +2615,6 @@ static int ext4_mb_init_backend(struct super_block *sb)
 		goto err_freesgi;
 	}
 	EXT4_I(sbi->s_buddy_cache)->i_disksize = 0;
-
-	metalen = sizeof(*meta_group_info) << EXT4_DESC_PER_BLOCK_BITS(sb);
-	for (i = 0; i < num_meta_group_infos; i++) {
-		if ((i + 1) == num_meta_group_infos)
-			metalen = sizeof(*meta_group_info) *
-				(ngroups -
-					(i << EXT4_DESC_PER_BLOCK_BITS(sb)));
-		meta_group_info = kmalloc(metalen, GFP_KERNEL);
-		if (meta_group_info == NULL) {
-			printk(KERN_ERR "EXT4-fs: can't allocate mem for a "
-			       "buddy group\n");
-			goto err_freemeta;
-		}
-		sbi->s_group_info[i] = meta_group_info;
-	}

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: ext4 memory leak (was Re: [PATCH] x86: _edata should include all .data.* sections on X86_64)
  2009-07-15  8:03                   ` Aneesh Kumar K.V
@ 2009-07-15  8:54                     ` Alexey Fisher
       [not found]                       ` <4A5D9939.3000500-M18mAb7Tlt0yCq4wW13eYl6hYfS7NtTn@public.gmane.org>
  2009-07-15 10:33                     ` Catalin Marinas
  1 sibling, 1 reply; 8+ messages in thread
From: Alexey Fisher @ 2009-07-15  8:54 UTC (permalink / raw)
  To: Aneesh Kumar K.V
  Cc: Catalin Marinas, Pekka Enberg, Kernel Testers List,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Sam Ravnborg, Ingo Molnar, linux-ext4-u79uwXL29TY76Z2rM5mHXA

This patch work for me.

Aneesh Kumar K.V schrieb:
> On Tue, Jul 14, 2009 at 01:26:30PM +0100, Catalin Marinas wrote:
>> (I cc'ed linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org as well)
>>
>> On Tue, 2009-07-14 at 12:37 +0200, Alexey Fisher wrote:
>>> this is complete trace from debug/kmemleak .
>> [...]
>>> i will compile now latest linux-arm.org/linux-2.6.git
>>> unreferenced object 0xffff880132c48890 (size 1024):
>>>    comm "exe", pid 1612, jiffies 4294894130
>>>    backtrace:
>>>      [<ffffffff810fbaca>] create_object+0x13a/0x2c0
>>>      [<ffffffff810fbd75>] kmemleak_alloc+0x25/0x60
>>>      [<ffffffff810f596b>] __kmalloc+0x11b/0x210
>>>      [<ffffffff811ae061>] ext4_mb_init+0x1b1/0x5c0
>>>      [<ffffffff8119f1e9>] ext4_fill_super+0x1e29/0x2720
>>>      [<ffffffff8110111f>] get_sb_bdev+0x16f/0x1b0
>>>      [<ffffffff81195413>] ext4_get_sb+0x13/0x20
>>>      [<ffffffff81100bf6>] vfs_kern_mount+0x76/0x180
>>>      [<ffffffff81100d6d>] do_kern_mount+0x4d/0x120
>>>      [<ffffffff81118ee7>] do_mount+0x307/0x8b0
>>>      [<ffffffff8111951f>] sys_mount+0x8f/0xe0
>>>      [<ffffffff8100b66b>] system_call_fastpath+0x16/0x1b
>>>      [<ffffffffffffffff>] 0xffffffffffffffff
>> After some investigation, this looks to me like a real leak.
>>
>> I managed to reproduce something similar (though the size may differ, I
>> think depending on filesystem size - only tried with a 64MB loop
>> device):
>>
>> unreferenced object 0xde468300 (size 32):
>>   comm "mount", pid 1445, jiffies 4294950074
>>   backtrace:
>>     [<c006d473>] __save_stack_trace+0x17/0x1c
>>     [<c006d545>] create_object+0xcd/0x188
>>     [<c01efe43>] kmemleak_alloc+0x1b/0x3c
>>     [<c006c013>] __kmalloc+0xd7/0xe4
>>     [<c00c1029>] ext4_mb_init+0x14d/0x374
>>     [<c00b7d7d>] ext4_fill_super+0x1385/0x16b4
>>     [<c0070891>] get_sb_bdev+0xa9/0xe4
>>     [<c00b574b>] ext4_get_sb+0xf/0x14
>>     [<c006fd3f>] vfs_kern_mount+0x33/0x64
>>     [<c006fda5>] do_kern_mount+0x25/0x8c
>>     [<c007e11f>] do_mount+0x47f/0x4c4
>>     [<c007e1b5>] sys_mount+0x51/0x80
>>     [<c0027c01>] ret_fast_syscall+0x1/0x40
>>     [<ffffffff>] 0xffffffff
>>
>> The above block is the meta_group_info allocated in
>> ext4_mb_init_backend() and stored in sbi->s_group_info[i] (i = 0 in my
>> case). Adding printk's and and inspecting the memory at
>> sbi->s_group_info[] shows different value stored, not the pointer
>> reported as leak.
>>
>> About the new pointer at sbi->s_group_info[0], kmemleak has this
>> information (via the dump= option in my branch; it isn't a leak report):
>>
>> kmemleak: Object 0xdfebfa80 (size 128):
>> kmemleak:   comm "mount", pid 1445, jiffies 4294950075
>> kmemleak:   min_count = 1
>> kmemleak:   count = 1
>> kmemleak:   flags = 0x1
>> kmemleak:   backtrace:
>>      [<c006d473>] __save_stack_trace+0x17/0x1c
>>      [<c006d545>] create_object+0xcd/0x188
>>      [<c01efe43>] kmemleak_alloc+0x1b/0x3c
>>      [<c006c013>] __kmalloc+0xd7/0xe4
>>      [<c00c0df1>] ext4_mb_add_groupinfo+0x29/0x114
>>      [<c00c107f>] ext4_mb_init+0x1a3/0x374
>>      [<c00b7d7d>] ext4_fill_super+0x1385/0x16b4
>>      [<c0070891>] get_sb_bdev+0xa9/0xe4
>>      [<c00b574b>] ext4_get_sb+0xf/0x14
>>      [<c006fd3f>] vfs_kern_mount+0x33/0x64
>>      [<c006fda5>] do_kern_mount+0x25/0x8c
>>      [<c007e11f>] do_mount+0x47f/0x4c4
>>      [<c007e1b5>] sys_mount+0x51/0x80
>>      [<c0027c01>] ret_fast_syscall+0x1/0x40
>>      [<ffffffff>] 0xffffffff
>>
>> So, ext4_mb_add_groupinfo() is overriding the pointers stored in
>> sbi->s_group_info[] by the ext4_mb_init_backend() function without
>> freeing them first.
>>
>> Maybe the ext4 people could clarify what is happening here as I'm not
>> familiar with the code.
>>
> 
> Can you try this patch ?
> 
> commit 4cc505d4c16c86f8f590ce4b288c920572bf2be9
> Author: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Date:   Wed Jul 15 13:20:37 2009 +0530
> 
>     ext4: Memory leak fix ext4_group_info allocation.
>     
>     commit 5f21b0e642d7bf6fe4434c9ba12bc9cb96b17cf7  was done to
>     reallocate groupinfo struct during resize properly. That goal
>     was to allocate new groupinfo struct when we are adding new block
>     groups during resize. Calling ext4_mb_add_group_info in the
>     mballoc initialization code path resulted in we reallocating
>     the group info struct . Fix this by not separately allocating
>     group info in the mballoc init path and always depend on
>     ext4_mb_add_group_info to allocate group info struct.
>     
>     The earlier code also had a bug that we allocated less number of
>     group info struct for the last meta group. But on resize we
>     expected that we had EXT4_DESC_PER_BLOCK group info struct for
>     each meta group.
>     
>     Signed-off-by: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> 
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 519a0a6..96ed1d8 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2615,22 +2615,6 @@ static int ext4_mb_init_backend(struct super_block *sb)
>  		goto err_freesgi;
>  	}
>  	EXT4_I(sbi->s_buddy_cache)->i_disksize = 0;
> -
> -	metalen = sizeof(*meta_group_info) << EXT4_DESC_PER_BLOCK_BITS(sb);
> -	for (i = 0; i < num_meta_group_infos; i++) {
> -		if ((i + 1) == num_meta_group_infos)
> -			metalen = sizeof(*meta_group_info) *
> -				(ngroups -
> -					(i << EXT4_DESC_PER_BLOCK_BITS(sb)));
> -		meta_group_info = kmalloc(metalen, GFP_KERNEL);
> -		if (meta_group_info == NULL) {
> -			printk(KERN_ERR "EXT4-fs: can't allocate mem for a "
> -			       "buddy group\n");
> -			goto err_freemeta;
> -		}
> -		sbi->s_group_info[i] = meta_group_info;
> -	}
> -
>  	for (i = 0; i < ngroups; i++) {
>  		desc = ext4_get_group_desc(sb, i, NULL);
>  		if (desc == NULL) {
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-testers" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ext4 memory leak (was Re: [PATCH] x86: _edata should include all .data.* sections on X86_64)
  2009-07-15  8:03                   ` Aneesh Kumar K.V
  2009-07-15  8:54                     ` Alexey Fisher
@ 2009-07-15 10:33                     ` Catalin Marinas
  1 sibling, 0 replies; 8+ messages in thread
From: Catalin Marinas @ 2009-07-15 10:33 UTC (permalink / raw)
  To: Aneesh Kumar K.V
  Cc: Alexey Fisher, Pekka Enberg, Kernel Testers List,
	linux-kernel@vger.kernel.org, Sam Ravnborg, Ingo Molnar,
	linux-ext4

On Wed, 2009-07-15 at 13:33 +0530, Aneesh Kumar K.V wrote:
> Can you try this patch ?
[...]
>     ext4: Memory leak fix ext4_group_info allocation.
>     
>     commit 5f21b0e642d7bf6fe4434c9ba12bc9cb96b17cf7  was done to
>     reallocate groupinfo struct during resize properly. That goal
>     was to allocate new groupinfo struct when we are adding new block
>     groups during resize. Calling ext4_mb_add_group_info in the
>     mballoc initialization code path resulted in we reallocating
>     the group info struct . Fix this by not separately allocating
>     group info in the mballoc init path and always depend on
>     ext4_mb_add_group_info to allocate group info struct.
>     
>     The earlier code also had a bug that we allocated less number of
>     group info struct for the last meta group. But on resize we
>     expected that we had EXT4_DESC_PER_BLOCK group info struct for
>     each meta group.
>     
>     Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

The kmemleak report disappeared.

Tested-by: Catalin Marinas <catalin.marinas@arm.com>

BTW, there are a few compiler warnings about unused variables with this
patch.

-- 
Catalin


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ext4 memory leak (was Re: [PATCH] x86: _edata should include all .data.* sections on X86_64)
       [not found]                       ` <4A5D9939.3000500-M18mAb7Tlt0yCq4wW13eYl6hYfS7NtTn@public.gmane.org>
@ 2009-07-18 11:55                         ` Ingo Molnar
       [not found]                           ` <20090718115556.GA31007-X9Un+BFzKDI@public.gmane.org>
  2009-07-18 22:33                           ` Catalin Marinas
  0 siblings, 2 replies; 8+ messages in thread
From: Ingo Molnar @ 2009-07-18 11:55 UTC (permalink / raw)
  To: Alexey Fisher
  Cc: Aneesh Kumar K.V, Catalin Marinas, Pekka Enberg,
	Kernel Testers List,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Sam Ravnborg, linux-ext4-u79uwXL29TY76Z2rM5mHXA


* Alexey Fisher <bug-track-M18mAb7Tlt0yCq4wW13eYl6hYfS7NtTn@public.gmane.org> wrote:

> This patch work for me.

nice. Any leftovers that might be false positives and need 
annotation?

We learned this with lockdep: the moment a typical x86 distro bootup 
is 'warnings free', utility of the debugging facility increases 
dramatically: people can standardize on 'kmemleak should never 
produce warnings' workflows and distros can also start feeding 
kmemleak reports into kerneloops.org or so.

So the general direction kmemleak is moving into is really 
encouraging.

	Ingo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ext4 memory leak (was Re: [PATCH] x86: _edata should include all .data.* sections on X86_64)
       [not found]                           ` <20090718115556.GA31007-X9Un+BFzKDI@public.gmane.org>
@ 2009-07-18 13:30                             ` Alexey Fisher
       [not found]                               ` <4A61CE59.3030905-M18mAb7Tlt0yCq4wW13eYl6hYfS7NtTn@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Alexey Fisher @ 2009-07-18 13:30 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Aneesh Kumar K.V, Catalin Marinas, Pekka Enberg,
	Kernel Testers List,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Sam Ravnborg, linux-ext4-u79uwXL29TY76Z2rM5mHXA

Ingo Molnar schrieb:
> * Alexey Fisher <bug-track-M18mAb7Tlt0yCq4wW13eYl6hYfS7NtTn@public.gmane.org> wrote:
> 
>> This patch work for me.
> 
> nice. Any leftovers that might be false positives and need 
> annotation?
> 
> We learned this with lockdep: the moment a typical x86 distro bootup 
> is 'warnings free', utility of the debugging facility increases 
> dramatically: people can standardize on 'kmemleak should never 
> produce warnings' workflows and distros can also start feeding 
> kmemleak reports into kerneloops.org or so.
> 
> So the general direction kmemleak is moving into is really 
> encouraging.
> 
> 	Ingo

suddenly my kernel is not warning free... i have still warning about 
acpi_init, cpufreg, intel_gem and inoitfy on my PC and about firmware 
loader on my laptop. So i think there is still some job to do. I will 
report this warnings soon.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ext4 memory leak (was Re: [PATCH] x86: _edata should include all .data.* sections on X86_64)
  2009-07-18 11:55                         ` Ingo Molnar
       [not found]                           ` <20090718115556.GA31007-X9Un+BFzKDI@public.gmane.org>
@ 2009-07-18 22:33                           ` Catalin Marinas
  1 sibling, 0 replies; 8+ messages in thread
From: Catalin Marinas @ 2009-07-18 22:33 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Alexey Fisher, Aneesh Kumar K.V, Pekka Enberg,
	Kernel Testers List, linux-kernel@vger.kernel.org, Sam Ravnborg,
	linux-ext4

On Sat, 2009-07-18 at 13:55 +0200, Ingo Molnar wrote:
> * Alexey Fisher <bug-track@fisher-privat.net> wrote:
> 
> > This patch work for me.
> 
> nice. Any leftovers that might be false positives and need 
> annotation?

With the latest mainline all the reports I get look like real leaks but
some of them are pretty difficult to debug. I have a kmemleak
development tree as well which, among other things like more
cond_resched() calls, scans all the task stacks (currently using
for_each_process) but it doesn't reduce the number of reports.

> We learned this with lockdep: the moment a typical x86 distro bootup 
> is 'warnings free', utility of the debugging facility increases 
> dramatically: people can standardize on 'kmemleak should never 
> produce warnings' workflows and distros can also start feeding 
> kmemleak reports into kerneloops.org or so.

Yes. It's also easy to identify recent commits causing leaks but
currently it looks like some of the have been around for some time
(though probably not so serious leaks).

-- 
Catalin


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ext4 memory leak (was Re: [PATCH] x86: _edata should include all .data.* sections on X86_64)
       [not found]                               ` <4A61CE59.3030905-M18mAb7Tlt0yCq4wW13eYl6hYfS7NtTn@public.gmane.org>
@ 2009-07-18 22:44                                 ` Catalin Marinas
  0 siblings, 0 replies; 8+ messages in thread
From: Catalin Marinas @ 2009-07-18 22:44 UTC (permalink / raw)
  To: Alexey Fisher
  Cc: Ingo Molnar, Aneesh Kumar K.V, Pekka Enberg, Kernel Testers List,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Sam Ravnborg, linux-ext4-u79uwXL29TY76Z2rM5mHXA

On Sat, 2009-07-18 at 15:30 +0200, Alexey Fisher wrote:
> suddenly my kernel is not warning free... i have still warning about 
> acpi_init, cpufreg, intel_gem and inoitfy on my PC and about firmware 
> loader on my laptop. So i think there is still some job to do. I will 
> report this warnings soon.

Mine is not warning free either but they look like real leaks. There are
also a few more leaks reported by Jaswinder.

The inotify one was fixed but not in mainline yet (a fix was included in
my kmemleak-fixes branch).

What I get consistently:

unreferenced object 0xf72a9a80 (size 64):
  comm "swapper", pid 1, jiffies 4294892557
  hex dump (first 32 bytes):
    00 00 00 00 38 00 00 00 00 00 00 00 00 00 00 00  ....8...........
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<c056aa8d>] kmemleak_alloc+0x3d/0x70
    [<c01e234d>] __kmalloc+0x10d/0x210
    [<c0351122>] kzalloc+0xb/0xd
    [<c0351798>] acpi_add_single_object+0x609/0xe65
    [<c03521c6>] acpi_bus_scan+0xfd/0x174
    [<c07c1629>] acpi_scan_init+0xb5/0xd5
    [<c07c140b>] acpi_init+0x21e/0x262
    [<c010112b>] do_one_initcall+0x2b/0x160
    [<c0799355>] kernel_init+0x150/0x1aa
    [<c0103e57>] kernel_thread_helper+0x7/0x10
    [<ffffffff>] 0xffffffff

Every time I kill the X server (reported here -
http://lkml.org/lkml/2009/7/8/422)

unreferenced object 0xcb5a6600 (size 44):
  comm "gdm", pid 5246, jiffies 4960
  hex dump (first 32 bytes):
    02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 e4 b5 2d ca 90 4e 15 c0 83 14 00 00  ......-..N......
  backtrace:
    [<c056aa8d>] kmemleak_alloc+0x3d/0x70
    [<c01e0be6>] kmem_cache_alloc+0x156/0x1a0
    [<c01552f9>] alloc_pid+0x19/0x350
    [<c013e6f0>] copy_process+0x800/0x1230
    [<c013f18f>] do_fork+0x6f/0x370
    [<c0101986>] sys_clone+0x36/0x40
    [<c010319c>] sysenter_do_call+0x12/0x38
    [<ffffffff>] 0xffffffff

And quite a lot of these (also reported here -
http://lkml.org/lkml/2009/7/9/110 - but the first one was fixed by
Jaswinder and included in my kmemleak-fixes branch):

unreferenced object 0xcb0166c0 (size 148):
  comm "Xorg", pid 5251, jiffies 5784
  hex dump (first 32 bytes):
    ff ff ff ff 30 7f 98 c3 00 80 18 cb 90 80 18 cb  ....0...........
    20 81 18 cb b0 81 18 cb 40 82 18 cb d0 82 18 cb   .......@.......
  backtrace:
    [<c056aa8d>] kmemleak_alloc+0x3d/0x70
    [<c01e0be6>] kmem_cache_alloc+0x156/0x1a0
    [<c0317bf0>] idr_pre_get+0x50/0x70
    [<fa448ff4>] drm_gem_handle_create+0x24/0x90 [drm]
    [<fa8b34ad>] i915_gem_create_ioctl+0x5d/0xb0 [i915]
    [<fa4477c2>] drm_ioctl+0x192/0x3a0 [drm]
    [<c01f8339>] vfs_ioctl+0x79/0x90
    [<c01f849a>] do_vfs_ioctl+0x6a/0x5e0
    [<c01f8a73>] sys_ioctl+0x63/0x70
    [<c010319c>] sysenter_do_call+0x12/0x38
    [<ffffffff>] 0xffffffff

-- 
Catalin

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-07-18 22:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <4A5C20E5.6010203@fisher-privat.net>
     [not found] ` <84144f020907140019g511723dctb541f6333d1a082b@mail.gmail.com>
     [not found]   ` <4A5C41C8.7040904@fisher-privat.net>
     [not found]     ` <1247564356.28240.30.camel@pc1117.cambridge.arm.com>
     [not found]       ` <1247565175.28240.37.camel@pc1117.cambridge.arm.com>
     [not found]         ` <4A5C5A59.5080304@fisher-privat.net>
     [not found]           ` <1247567499.28240.48.camel@pc1117.cambridge.arm.com>
     [not found]             ` <4A5C5FD0.3020204@fisher-privat.net>
     [not found]               ` <4A5C5FD0.3020204-M18mAb7Tlt0yCq4wW13eYl6hYfS7NtTn@public.gmane.org>
2009-07-14 12:26                 ` ext4 memory leak (was Re: [PATCH] x86: _edata should include all .data.* sections on X86_64) Catalin Marinas
2009-07-15  8:03                   ` Aneesh Kumar K.V
2009-07-15  8:54                     ` Alexey Fisher
     [not found]                       ` <4A5D9939.3000500-M18mAb7Tlt0yCq4wW13eYl6hYfS7NtTn@public.gmane.org>
2009-07-18 11:55                         ` Ingo Molnar
     [not found]                           ` <20090718115556.GA31007-X9Un+BFzKDI@public.gmane.org>
2009-07-18 13:30                             ` Alexey Fisher
     [not found]                               ` <4A61CE59.3030905-M18mAb7Tlt0yCq4wW13eYl6hYfS7NtTn@public.gmane.org>
2009-07-18 22:44                                 ` Catalin Marinas
2009-07-18 22:33                           ` Catalin Marinas
2009-07-15 10:33                     ` Catalin Marinas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox