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. > > > 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 > >> > >> . > >> > >