On 2017/11/17 8:58, Yunlong Song wrote: > Is there any problem if just deleting the judgement condition in this patch? IIRC, dirty node comes from data segment GC can be writebacked & flushed during atomic commit, but related data will still be in inner bio cache, after later SPOR, data would be inconsistent. Thanks, > > On 2017/11/8 17:28, Chao Yu wrote: >> On 2017/11/8 10:34, Yunlong Song wrote: >>> If some files are opened with atomic flag and have not commited yet, at >>> the same time, if all the target victim segments have at least one page >>> of these atomic files, then f2fs gc will fail to do gc and hangs in the >>> process of go to gc_more, since gc_date_segment will not move any data >>> and get_valid_blocks will never be 0, then do_garbage_collect will >>> always return 0. >> Oh, I added this judgment condition to avoid ruining atomic write by data >> GC, could we find another way to solve this issue? BTW, if there is direct >> IO, we will also skip data segment GC. >> >> Thanks, >> >>> Signed-off-by: Yunlong Song <yunlong.song@xxxxxxxxxx> >>> --- >>> fs/f2fs/gc.c | 6 ------ >>> 1 file changed, 6 deletions(-) >>> >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >>> index 5d5bba4..3fdcd04 100644 >>> --- a/fs/f2fs/gc.c >>> +++ b/fs/f2fs/gc.c >>> @@ -621,9 +621,6 @@ static void move_data_block(struct inode *inode, block_t bidx, >>> if (!check_valid_map(F2FS_I_SB(inode), segno, off)) >>> goto out; >>> >>> - if (f2fs_is_atomic_file(inode)) >>> - goto out; >>> - >>> set_new_dnode(&dn, inode, NULL, NULL, 0); >>> err = get_dnode_of_data(&dn, bidx, LOOKUP_NODE); >>> if (err) >>> @@ -718,9 +715,6 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type, >>> if (!check_valid_map(F2FS_I_SB(inode), segno, off)) >>> goto out; >>> >>> - if (f2fs_is_atomic_file(inode)) >>> - goto out; >>> - >>> if (gc_type == BG_GC) { >>> if (PageWriteback(page)) >>> goto out; >>> >> >> . >> >