From: jing zhang Subject: Re: [PATCH] ext4: page_cache pages not released in ext4_mb_load_buddy() Date: Thu, 25 Mar 2010 22:02:44 +0800 Message-ID: References: <87eij9d5qs.fsf@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Cc: linux-ext4 , "Theodore Ts'o" , Andreas Dilger , Dave Kleikamp To: "Aneesh Kumar K. V" Return-path: Received: from mail-gx0-f217.google.com ([209.85.217.217]:44660 "EHLO mail-gx0-f217.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752773Ab0CYOCp (ORCPT ); Thu, 25 Mar 2010 10:02:45 -0400 Received: by gxk9 with SMTP id 9so2149827gxk.8 for ; Thu, 25 Mar 2010 07:02:44 -0700 (PDT) In-Reply-To: <87eij9d5qs.fsf@linux.vnet.ibm.com> Sender: linux-ext4-owner@vger.kernel.org List-ID: 2010/3/25, Aneesh Kumar K. V : > On Wed, 24 Mar 2010 22:21:01 +0800, jing zhang wrote: >> From: Jing Zhang >> >> Date: Wed Mar 24 20:38:48 2010 >> >> If ext4_mb_init_cache() returns error, there is no page_cache_release() >> issued. >> >> Cc: Theodore Ts'o >> Cc: Andreas Dilger >> Cc: Dave Kleikamp >> Signed-off-by: Jing Zhang >> >> --- >> >> --- linux-2.6.32/fs/ext4/mballoc.c 2009-12-03 11:51:22.000000000 +0800 >> +++ ext4_mm_leak/mballoc-9.c 2010-03-24 22:24:56.000000000 +0800 >> @@ -1132,8 +1132,6 @@ repeat_load_buddy: >> e4b->bd_buddy = page_address(page) + (poff * sb->s_blocksize); >> mark_page_accessed(page); >> >> - BUG_ON(e4b->bd_bitmap_page == NULL); >> - BUG_ON(e4b->bd_buddy_page == NULL); > > Why remove these ? Just before return successfully, both bitmap page and buddy page are assigned. If not either, EIO will branch. > >> >> return 0; >> >> @@ -1142,6 +1140,8 @@ err: >> page_cache_release(e4b->bd_bitmap_page); >> if (e4b->bd_buddy_page) >> page_cache_release(e4b->bd_buddy_page); >> + if (page) >> + page_cache_release(page); >> e4b->bd_buddy = NULL; >> e4b->bd_bitmap = NULL; > > Can you add a comment which says on error page will point to the page > cache page which haven't been assigned to neither bd_bitmap_page or > bd_buddy_page + if (page) + /* we have to release page here in case that + * ext4_mb_init_cache() returns error when preparing + * either bitmap_ or buddy_page. + */ + page_cache_release(page); > > But good catch !! > > -aneesh > Is that ok, Aneesh? - zj