Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751929AbbGMX0E (ORCPT ); Mon, 13 Jul 2015 19:26:04 -0400 Received: from mail.kernel.org ([198.145.29.136]:49090 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751556AbbGMX0C (ORCPT ); Mon, 13 Jul 2015 19:26:02 -0400 Date: Mon, 13 Jul 2015 16:25:57 -0700 From: Jaegeuk Kim To: Chao Yu Cc: 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 Message-ID: <20150713232437.GA64479@jaegeuk-mac02.mot.com> References: <00f301d0ba30$ebe95b80$c3bc1280$@samsung.com> <20150711001715.GA61190@jaegeuk-mac02> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2056 Lines: 64 Hi Chao, On Sat, Jul 11, 2015 at 10:02:51AM +0800, Chao Yu wrote: > 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? Indeed. I cannot think about other clean way for now. Instead, how about adding this description in the patch and some comments in the codes? Thanks, -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/