From: Alexey Fisher Subject: Re: ext4 memory leak (was Re: [PATCH] x86: _edata should include all .data.* sections on X86_64) Date: Wed, 15 Jul 2009 10:54:17 +0200 Message-ID: <4A5D9939.3000500@fisher-privat.net> References: <4A5C20E5.6010203@fisher-privat.net> <84144f020907140019g511723dctb541f6333d1a082b@mail.gmail.com> <4A5C41C8.7040904@fisher-privat.net> <1247564356.28240.30.camel@pc1117.cambridge.arm.com> <1247565175.28240.37.camel@pc1117.cambridge.arm.com> <4A5C5A59.5080304@fisher-privat.net> <1247567499.28240.48.camel@pc1117.cambridge.arm.com> <4A5C5FD0.3020204@fisher-privat.net> <1247574390.28240.67.camel@pc1117.cambridge.arm.com> <20090715080336.GA4823@skywalker> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Catalin Marinas , Pekka Enberg , Kernel Testers List , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Sam Ravnborg , Ingo Molnar , linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: "Aneesh Kumar K.V" Return-path: In-Reply-To: <20090715080336.GA4823@skywalker> Sender: kernel-testers-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-ext4.vger.kernel.org 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: >>> [] create_object+0x13a/0x2c0 >>> [] kmemleak_alloc+0x25/0x60 >>> [] __kmalloc+0x11b/0x210 >>> [] ext4_mb_init+0x1b1/0x5c0 >>> [] ext4_fill_super+0x1e29/0x2720 >>> [] get_sb_bdev+0x16f/0x1b0 >>> [] ext4_get_sb+0x13/0x20 >>> [] vfs_kern_mount+0x76/0x180 >>> [] do_kern_mount+0x4d/0x120 >>> [] do_mount+0x307/0x8b0 >>> [] sys_mount+0x8f/0xe0 >>> [] system_call_fastpath+0x16/0x1b >>> [] 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: >> [] __save_stack_trace+0x17/0x1c >> [] create_object+0xcd/0x188 >> [] kmemleak_alloc+0x1b/0x3c >> [] __kmalloc+0xd7/0xe4 >> [] ext4_mb_init+0x14d/0x374 >> [] ext4_fill_super+0x1385/0x16b4 >> [] get_sb_bdev+0xa9/0xe4 >> [] ext4_get_sb+0xf/0x14 >> [] vfs_kern_mount+0x33/0x64 >> [] do_kern_mount+0x25/0x8c >> [] do_mount+0x47f/0x4c4 >> [] sys_mount+0x51/0x80 >> [] ret_fast_syscall+0x1/0x40 >> [] 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: >> [] __save_stack_trace+0x17/0x1c >> [] create_object+0xcd/0x188 >> [] kmemleak_alloc+0x1b/0x3c >> [] __kmalloc+0xd7/0xe4 >> [] ext4_mb_add_groupinfo+0x29/0x114 >> [] ext4_mb_init+0x1a3/0x374 >> [] ext4_fill_super+0x1385/0x16b4 >> [] get_sb_bdev+0xa9/0xe4 >> [] ext4_get_sb+0xf/0x14 >> [] vfs_kern_mount+0x33/0x64 >> [] do_kern_mount+0x25/0x8c >> [] do_mount+0x47f/0x4c4 >> [] sys_mount+0x51/0x80 >> [] ret_fast_syscall+0x1/0x40 >> [] 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 > 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 > > 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