From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755313AbcEQDCA (ORCPT ); Mon, 16 May 2016 23:02:00 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:38388 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755057AbcEQDB7 (ORCPT ); Mon, 16 May 2016 23:01:59 -0400 Message-ID: <573A8965.9060900@huawei.com> Date: Tue, 17 May 2016 11:00:53 +0800 From: Hou Pengyang User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/20130509 Thunderbird/17.0.6 MIME-Version: 1.0 To: Chao Yu CC: , , , wangbintian 00221568 Subject: Re: [f2fs-dev] [RFC] f2fs: fix a race condition between evict & gc References: <1463395221-109622-1-git-send-email-houpengyang@huawei.com> <7ab55f84-5ad7-8986-12a3-0e9c8c96e54e@kernel.org> In-Reply-To: <7ab55f84-5ad7-8986-12a3-0e9c8c96e54e@kernel.org> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.111.95.59] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090202.573A8973.01A5,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: a0b2be685f5711f966f28d56e55a3d33 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016/5/16 23:10, Chao Yu wrote: Hi chao, > Hi Pengyang, > > On 2016/5/16 18:40, Hou Pengyang wrote: >> When collecting data segment(gc_data_segment), there is a race condition >> between evict and phases of gc: >> 0) ra_node_page(dnode) >> 1) ra_node_page(inode) >> <--- evict the inode >> 2) f2fs_iget get the inode and add it to gc_list >> 3) move_data_page >> >> In step 2), f2fs_iget does NOT find the inode and allocs a new inode as result, > > If inode was unlinked and then be evicted, f2fs_iget should fail when reading > inode's page as blkaddr of this node is null. agree, after do_read_inode fail, the newly created inode would be freed as a bad inode and f2fs_iget fails. But this may lead to create file fail: gc:iget_locked <---- touch/mkdir(reuse the evicted ino) gc:free the bad inode during the bad inode allocated and freed in gc, the inode is reserved in the global inode_hash, while the ino is a free nid in free_nid tree. touch/mkdir may reuse the ino, during the touch/mkdir path, the global inode_hash would be checked if the ino file exists. Under this scenario, because of the gc bad inode in inode_hash, touch/mkdir would fail. ilookup seems better, as no need to alloc and free a bad inode. if ilookup fails, that exactly means inode has been evicted and no need to gc; if ilookup success, before phase 3, is_alive to deal with the ino reuse scenario; Do I miss anything else? thanks > If inode still have non-zero nlink value and then be evicted, we should allow gc > thread to reference this inode for moving its data pages. > > Thanks, > >> which is not resonable. >> >> This patch changes f2fs_iget to ilookup. when no inode is found, no new inode is >> created. >> >> Signed-off-by: Hou Pengyang >> --- >> fs/f2fs/gc.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >> index 38d56f6..6e73193 100644 >> --- a/fs/f2fs/gc.c >> +++ b/fs/f2fs/gc.c >> @@ -717,8 +717,8 @@ next_step: >> ofs_in_node = le16_to_cpu(entry->ofs_in_node); >> >> if (phase == 2) { >> - inode = f2fs_iget(sb, dni.ino); >> - if (IS_ERR(inode) || is_bad_inode(inode)) >> + inode = ilookup(sb, dni.ino); >> + if (!inode || IS_ERR(inode) || is_bad_inode(inode)) >> continue; >> >> /* if encrypted inode, let's go phase 3 */ >> > > . >