From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by mu-out-0910.google.com with SMTP id g7so632920muf for ; Thu, 02 Aug 2007 15:55:30 -0700 (PDT) From: Jesper Juhl Subject: Re: [PATCH] Fix two potential mem leaks in MPT Fusion (mpt_attach()) Date: Fri, 3 Aug 2007 00:53:44 +0200 References: <200708020155.33690.jesper.juhl@gmail.com> <20070801172653.1fd44e99.akpm@linux-foundation.org> <9a8748490708020120w4bbfe6d1n6f6986aec507316@mail.gmail.com> In-Reply-To: <9a8748490708020120w4bbfe6d1n6f6986aec507316@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200708030053.45297.jesper.juhl@gmail.com> Sender: owner-linux-mm@kvack.org Return-Path: To: Andrew Morton Cc: Linux Kernel Mailing List , James Bottomley , Christoph Lameter , Pekka Enberg , linux-mm@kvack.org, Ingo Molnar , Matt Mackall List-ID: On Thursday 02 August 2007 10:20:47 Jesper Juhl wrote: > On 02/08/07, Andrew Morton wrote: [snip] > > y'know, we could have a debug option which will spit warnings if someone > > does a !__GFP_WAIT allocation while !in_atomic() (only works if > > CONFIG_PREEMPT). > > > > But please, make it depend on !CONFIG_AKPM. I shudder to think about all > > the stuff it would pick up. > > > > I can try to cook up something like that tonight... > Ok, so I did a quick hack and I'm drowning in dmesg WARN_ON() traces with my usual config. This is what I added : diff --git a/mm/slub.c b/mm/slub.c index 6c6d74f..e60dd9e 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -20,6 +20,7 @@ #include #include #include +#include /* * Lock order: @@ -1568,6 +1569,10 @@ static void __always_inline *slab_alloc(struct kmem_cache *s, void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags) { +#ifdef CONFIG_PREEMPT + WARN_ON( !in_atomic() && !(gfpflags & __GFP_WAIT) ); +#endif + return slab_alloc(s, gfpflags, -1, __builtin_return_address(0)); } EXPORT_SYMBOL(kmem_cache_alloc); @@ -2370,6 +2375,10 @@ void *__kmalloc(size_t size, gfp_t flags) { struct kmem_cache *s = get_slab(size, flags); +#ifdef CONFIG_PREEMPT + WARN_ON( !in_atomic() && !(flags & __GFP_WAIT) ); +#endif + if (ZERO_OR_NULL_PTR(s)) return s; And this is what I'm getting heaps of : ... [ 165.128607] ======================= [ 165.128609] WARNING: at mm/slub.c:1573 kmem_cache_alloc() [ 165.128611] [] show_trace_log_lvl+0x1a/0x30 [ 165.128614] [] show_trace+0x12/0x20 [ 165.128616] [] dump_stack+0x16/0x20 [ 165.128619] [] kmem_cache_alloc+0xe3/0x110 [ 165.128622] [] mempool_alloc_slab+0xe/0x10 [ 165.128625] [] mempool_alloc+0x31/0xf0 [ 165.128628] [] bio_alloc_bioset+0x73/0x140 [ 165.128631] [] bio_alloc+0xe/0x20 [ 165.128634] [] bio_map_kern+0x31/0x100 [ 165.128637] [] blk_rq_map_kern+0x52/0x90 [ 165.128640] [] scsi_execute+0x4b/0xe0 [ 165.128643] [] sr_do_ioctl+0xa8/0x230 [ 165.128646] [] sr_read_tochdr+0x76/0xb0 [ 165.128649] [] sr_disk_status+0x1b/0xa0 [ 165.128652] [] sr_cd_check+0x9b/0x1b0 [ 165.128655] [] sr_media_change+0x7d/0x250 [ 165.128659] [] media_changed+0x5e/0xa0 [ 165.128662] [] cdrom_media_changed+0x31/0x40 [ 165.128665] [] sr_block_media_changed+0xe/0x10 [ 165.128668] [] check_disk_change+0x20/0x80 [ 165.128671] [] cdrom_open+0x173/0xa10 [ 165.128674] [] sr_block_open+0x5e/0xa0 [ 165.128677] [] do_open+0x85/0x2c0 [ 165.128680] [] blkdev_open+0x33/0x80 [ 165.128683] [] __dentry_open+0xe4/0x200 [ 165.128686] [] nameidata_to_filp+0x35/0x40 [ 165.128689] [] do_filp_open+0x49/0x60 [ 165.128692] [] do_sys_open+0x49/0xe0 [ 165.128695] [] sys_open+0x1c/0x20 [ 165.128697] [] syscall_call+0x7/0xb ... [ 165.134957] WARNING: at mm/slub.c:1573 kmem_cache_alloc() [ 165.134959] [] show_trace_log_lvl+0x1a/0x30 [ 165.134962] [] show_trace+0x12/0x20 [ 165.134965] [] dump_stack+0x16/0x20 [ 165.134969] [] kmem_cache_alloc+0xe3/0x110 [ 165.134971] [] mempool_alloc_slab+0xe/0x10 [ 165.134974] [] mempool_alloc+0x31/0xf0 [ 165.134977] [] get_request+0xac/0x260 [ 165.134981] [] get_request_wait+0x1c/0x100 [ 165.134983] [] blk_get_request+0x32/0x70 [ 165.134986] [] scsi_execute+0x22/0xe0 [ 165.134989] [] scsi_execute_req+0x6c/0xd0 [ 165.134991] [] ioctl_internal_command+0x40/0x100 [ 165.134996] [] scsi_set_medium_removal+0x5c/0x90 [ 165.134999] [] sr_lock_door+0x16/0x20 [ 165.135002] [] cdrom_release+0x104/0x250 [ 165.135005] [] sr_block_release+0x24/0x40 [ 165.135008] [] __blkdev_put+0x146/0x150 [ 165.135012] [] blkdev_put+0xa/0x10 [ 165.135015] [] blkdev_close+0x32/0x40 [ 165.135018] [] __fput+0xb6/0x180 [ 165.135021] [] fput+0x19/0x20 [ 165.135024] [] filp_close+0x47/0x80 [ 165.135027] [] sys_close+0x66/0xc0 [ 165.135030] [] syscall_call+0x7/0xb [ 165.135032] ======================= [ 166.564998] WARNING: at mm/slub.c:1573 kmem_cache_alloc() [ 166.565006] [] show_trace_log_lvl+0x1a/0x30 [ 166.565013] [] show_trace+0x12/0x20 [ 166.565016] [] dump_stack+0x16/0x20 [ 166.565020] [] kmem_cache_alloc+0xe3/0x110 [ 166.565030] [] mempool_alloc_slab+0xe/0x10 [ 166.565039] [] mempool_alloc+0x31/0xf0 [ 166.565047] [] bio_alloc_bioset+0x1f/0x140 [ 166.565057] [] bio_alloc+0xe/0x20 [ 166.565066] [] submit_bh+0x63/0x100 [ 166.565075] [] journal_do_submit_data+0x28/0x40 [ 166.565085] [] journal_commit_transaction+0x658/0x1290 [ 166.565095] [] kjournald+0xb2/0x1e0 [ 166.565103] [] kthread+0x42/0x70 [ 166.565112] [] kernel_thread_helper+0x7/0x18 [ 166.565121] ======================= ... etc... So, where do we go from here? Obviously my patch above is nothing but a quick hack. Should I turn that into a proper debug config option? Do we even want to clean up this stuff? Am I even looking at the right thing? I'm more than willing to try and create a proper debug option patch as well as clean up some of these allocations if wanted... What say "the powers that be" ? Kind regards, Jesper Juhl PS. Please keep me on Cc when replying. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org