On 2016/11/2 15:19, Chao Yu wrote: > On 2016/10/28 6:34, Jaegeuk Kim wrote: >> On Wed, Oct 26, 2016 at 10:14:04AM +0800, heyunlei wrote: >>> >>> >>> On 2016/10/26 9:09, Jaegeuk Kim wrote: >>> Hi Kim, >>>> Hi Yunlei, >>>> >>>> On Mon, Oct 24, 2016 at 11:37:28AM +0800, Yunlei He wrote: >>>>> If one block has been to written to a new place, just return >>>>> in move data process. >>>>> >>>>> Signed-off-by: Yunlei He <[email protected]> >>>>> --- >>>>> fs/f2fs/gc.c | 17 ++++++++++++----- >>>>> 1 file changed, 12 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >>>>> index f35fca5..fc78f3e 100644 >>>>> --- a/fs/f2fs/gc.c >>>>> +++ b/fs/f2fs/gc.c >>>>> @@ -545,7 +545,8 @@ static bool is_alive(struct f2fs_sb_info *sbi, struct >>>>> f2fs_summary *sum, >>>>> return true; >>>>> } >>>>> >>>>> -static void move_encrypted_block(struct inode *inode, block_t bidx) >>>>> +static void move_encrypted_block(struct inode *inode, block_t bidx, >>>>> + unsigned int segno, int off) >>>>> { >>>>> struct f2fs_io_info fio = { >>>>> .sbi = F2FS_I_SB(inode), >>>>> @@ -580,6 +581,9 @@ static void move_encrypted_block(struct inode *inode, >>>>> block_t bidx) >>>>> * don't cache encrypted data into meta inode until previous dirty >>>>> * data were writebacked to avoid racing between GC and flush. >>>>> */ >>>>> + if (!check_valid_map(F2FS_I_SB(inode), segno, off)) >>>>> + goto out; >>>> >>>> This is done by gc_data_segment() before calling move function. >>>> Why do we need this again? >>> >>> gc_data_segment() check this without lock the data page. If this block >>> is written to a new place between check in gc_data_segment() and locked >>> the page, it will need to wait this page write back, and write it again, >>> it's unnecessary. >>> >>>> >>>>> + >>>>> f2fs_wait_on_page_writeback(page, DATA, true); >>>>> >>>>> get_node_info(fio.sbi, dn.nid, &ni); >>>>> @@ -646,7 +650,8 @@ static void move_encrypted_block(struct inode *inode, >>>>> block_t bidx) >>>>> f2fs_put_page(page, 1); >>>>> } >>>>> >>>>> -static void move_data_page(struct inode *inode, block_t bidx, int >>>>> gc_type) >>>>> +static void move_data_page(struct inode *inode, block_t bidx, int >>>>> gc_type, >>>>> + unsigned int segno, int >>>>> off) >>>>> { >>>>> struct page *page; >>>>> >>>>> @@ -655,8 +660,10 @@ static void move_data_page(struct inode *inode, >>>>> block_t bidx, int gc_type) >>>>> return; >>>>> >>>>> if (gc_type == BG_GC) { >>>>> - if (PageWriteback(page)) >>>> >>>> Seems there is no reason to remove the existing writeback case. >>>> This is a BG_GC case and we don't need to proceed GC in this case. >>> Here, PageWriteback(page) means two cases: >>> 1. write to this victim segment >>> 2. write to a new place >>> >>> In the first case, if we return directly, this victim bg_gc will fail >>> for some blocks are still valid. >> >> It should be fine, and its main reason would be to avoid >> wait_on_page_writeback. >> Next bg_gc will do better. > > Agreed. > > Yunlei, I think it's better add this judgment condition after we grabbed > locked > data page for covering both background and foreground GC cases, instead of > changing previous process logic. > > Thanks,
Ok, I have sent a new version. Thanks, > >> >> Thanks, >> >> ------------------------------------------------------------------------------ >> The Command Line: Reinvented for Modern Developers >> Did the resurgence of CLI tooling catch you by surprise? >> Reconnect with the command line and become more productive. >> Learn the new .NET and ASP.NET CLI. Get your free copy! >> http://sdm.link/telerik >> _______________________________________________ >> Linux-f2fs-devel mailing list >> [email protected] >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel >> >> . >> > > > . > ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
