From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751937AbbGKGG6 (ORCPT ); Sat, 11 Jul 2015 02:06:58 -0400 Received: from col004-omc1s2.hotmail.com ([65.55.34.12]:55001 "EHLO COL004-OMC1S2.hotmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751685AbbGKGG4 (ORCPT ); Sat, 11 Jul 2015 02:06:56 -0400 X-TMN: [gVVB28qey4VbmzJVSSDe1o88HkeAsB0I] X-Originating-Email: [yuchaochina@hotmail.com] Message-ID: From: Chao Yu To: "'Jaegeuk Kim'" CC: , References: <00f301d0ba30$ebe95b80$c3bc1280$@samsung.com> <20150711001715.GA61190@jaegeuk-mac02> In-Reply-To: <20150711001715.GA61190@jaegeuk-mac02> Subject: RE: [f2fs-dev] [PATCH] f2fs: fix to release inode page in get_new_data_page Date: Sat, 11 Jul 2015 10:02:51 +0800 MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 15.0 Thread-index: AQABAgMEHcB9UHgY2Se2FM+FfopETgCNtsvpoXCHIBA= Content-Language: zh-cn X-OriginalArrivalTime: 11 Jul 2015 02:03:04.0018 (UTC) FILETIME=[B8D77B20:01D0BB7D] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Jaegeuk, > -----Original Message----- > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org] > Sent: Saturday, July 11, 2015 8:17 AM > To: Chao Yu; Chao Yu > Cc: linux-kernel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net; > linux-kernel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH] f2fs: fix to release inode page in get_new_data_page > > Hi Chao, > > On Thu, Jul 09, 2015 at 06:20:08PM +0800, Chao Yu wrote: > > get_new_data_page should release inode page when we encounter any > > error in its procedure, but there is one error path didn't cover > > this, fix it. > > Nice catch. > But, I think we should fix its caller: > > in init_inode_metadata(), > err = make_empty_dir(); > if (err) > goto put_error; > --------------- Previously, I fixed in the same way, but I got an oops when I test the patch with xfstest suit, it shows we will meet an error in this call path IIRC: ->f2fs_mkdir ->__f2fs_add_link ->init_inode_metadata ->make_empty_dir ->get_new_data_page ->f2fs_reserve_block ->reserve_new_block ->inc_valid_block_count return -ENOSPC; ->f2fs_put_dnode ->f2fs_put_page(ipage, 1) put_error: ->f2fs_put_page(ipage, 1); f2fs_bug_on(F2FS_P_SB(page), !PageLocked(page)); And I don't think we should change error handling method of f2fs_put_dnode for just fixing this issue. How do you think? Thanks, > Thanks, > > > > > Signed-off-by: Chao Yu > > --- > > fs/f2fs/data.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > > index 08dfdc6..ea8898b 100644 > > --- a/fs/f2fs/data.c > > +++ b/fs/f2fs/data.c > > @@ -397,8 +397,10 @@ struct page *get_new_data_page(struct inode *inode, > > int err; > > repeat: > > page = grab_cache_page(mapping, index); > > - if (!page) > > + if (!page) { > > + f2fs_put_page(ipage, 1); > > return ERR_PTR(-ENOMEM); > > + } > > > > set_new_dnode(&dn, inode, ipage, NULL, 0); > > err = f2fs_reserve_block(&dn, index); > > -- > > 2.4.2 > > ------------------------------------------------------------------------------ > Don't Limit Your Business. Reach for the Cloud. > GigeNET's Cloud Solutions provide you with the tools and support that > you need to offload your IT needs and focus on growing your business. > Configured For All Businesses. Start Your Cloud Today. > https://www.gigenetcloud.com/ > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel