Hi Jaegeuk, On 2018/1/31 10:18, Jaegeuk Kim wrote: > On 01/26, Gaoxiang (OS) wrote: >> Hi Jaegeuk and Chao, >> >> On 2018/1/26 9:36, Chao Yu wrote: >>> On 2018/1/26 6:06, Jaegeuk Kim wrote: >>>> >>>> Then, we don't need to wait for this as well as wait_on_all_pages_writeback() >>>> in the early stage in do_checkpoint()? >>>> >>>> So, it seems like we can modify like below: >>>> >>>> --- >>>> 1. while (get_pages()) >>>> sync_meta_pages() >>>> 2. if (enabled_nat_bits()) >>>> while (get_pages()) >>>> sync_meta_pages() >>>> >>>> 3. wait_on_all_pages_writeback() >>>> -> remove >>> >>> Would meta area across two devices? if it would, we need to wait all meta >>> be persisted in second device before f2fs_flush_device_cache? >>> >>>> >>>> 4. f2fs_flush_device_cache() > > -> remove > >>>> >>>> 5. update_meta_page() <- for first cp_block >>>> >>>> 6. update_meta_page()... <- payload >>>> >>>> 7. orphan writes >>>> >>>> 8. node_summary writes >>>> >>>> 9. update_meta_page() <- for last cp_block >>>> -> remove >>> >> >> - /* writeout checkpoint block */ >> - update_meta_page(sbi, ckpt, start_blk); >> - >> - /* wait for previous submitted node/meta pages writeback */ >> - wait_on_all_pages_writeback(sbi); >> - >> - if (unlikely(f2fs_cp_error(sbi))) >> - return -EIO; >> - >> Could also be removed, too? >> >> filemap_fdatawait_range(NODE_MAPPING(sbi), 0, LLONG_MAX); >> filemap_fdatawait_range(META_MAPPING(sbi), 0, LLONG_MAX); > > -> remove > > Hmm, think so. > >> >> >>> 9.1 sync_meta_pages(META) to make sure all meta IOs are issued. >>> >> >> If I understand correctly, I have the same questions with Chao. >> It seems that META doesn't have another flush mechanism (eg. flush >> thread) other than sync_meta_pages? > > 9.2 f2fs_flush_device_cache(), if we have multiple devices. > >> >>>> >>>> 10. wait_on_all_pages_writeback() > > 10.1. (f2fs_cp_error()) > return -EIO; > >>>> >>>> ---- >>>> Add) 11. commit_checkpoint() >>>> - update_meta_page() <- for last cp_block >>>> - sync_meta_pages(META_FLUSH) >>>> >>>> We don't need to wait for page_writeback any more. >>>> >> >> >> Apart from that, I think we should "wait_on_all_pages_writeback(sbi);" >> after META_FLUSH in case for pulluting the next checkpoint when the last >> cp block is failed to write with FUA? > > Next cp block won't be written by 10.1. > I think that 10.1 ensures the cp pack before the last cp_block was done. However, what if the last cp block writes fail later without FUA? Should we need to ensure the last cp block going into device medium rather than device cache before switching to go into the next checkpoint (I mean we need to ensure writing to medium and then unblock_operation and quit write_checkpoint and go on fs operations)? Other parts seems OK for me :), I will sort out and resent a new patch. Thanks, >> >> >> Thanks all, >> >>>>> >>>>> @@ -1313,10 +1337,15 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) >>>>> sbi->last_valid_block_count = sbi->total_valid_block_count; >>>>> percpu_counter_set(&sbi->alloc_valid_block_count, 0); >>>>> >>>>> - /* Here, we only have one bio having CP pack */ >>>>> - sync_meta_pages(sbi, META_FLUSH, LONG_MAX, FS_CP_META_IO); >>>>> + /* Here, we have one bio having CP pack except cp pack 2 page */ >>>>> + sync_meta_pages(sbi, META, LONG_MAX, FS_CP_META_IO); >>>>> >>>>> /* wait for previous submitted meta pages writeback */ >>>>> + if (!test_opt(sbi, NOBARRIER)) >>>> >>>> The above has nothing to do with this patch. >>> >>> We only need to use wait_on_all_pages_writeback to keep writeback order of >>> previous metadata and last cp pack metadata if barrier is on? >>> >>> Thanks, >>> >>>> >>>>> + wait_on_all_pages_writeback(sbi); >>>>> + >>>>> + /* barrier and flush checkpoint cp pack 2 page */ >>>>> + commit_checkpoint(sbi, ckpt, start_blk); >>>>> wait_on_all_pages_writeback(sbi); >>>>> >>>>> release_ino_entry(sbi, false); >>>>> -- >>>>> 2.1.4 >>>> >>>> . >>>> >>>
