Hi Chao, On Sat, Jul 11, 2015 at 10:02:51AM +0800, Chao Yu wrote: > Hi Jaegeuk, > > > -----Original Message----- > > From: Jaegeuk Kim [mailto:jaeg...@kernel.org] > > Sent: Saturday, July 11, 2015 8:17 AM > > To: Chao Yu; Chao Yu > > Cc: linux-kernel@vger.kernel.org; linux-f2fs-de...@lists.sourceforge.net; > > linux-kernel@vger.kernel.org; linux-f2fs-de...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
