On 2018/1/31 10:43, Gaoxiang (OS) wrote: > Hi Jaegeuk, > > On 2018/1/31 10:39, Jaegeuk Kim wrote: >> On 01/31, Gaoxiang (OS) wrote: >>> 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? >> >> Without FUA? The last cp_block is written by FUA, no? > > quote " > 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? > " > > what I meant is that the last cp_block should be written by FUA. > we need to use META_FLUSH to write last cp_block, right? :) > we need to ensure FUA writing to medium (using "wait_on_all_pages_writeback(sbi)") and then unblock_operation ,quit write_checkpoint, go on fs operations 11. commit_checkpoint() - update_meta_page() <- for last cp_block - sync_meta_pages(META_FLUSH) 12. wait_on_all_pages_writeback() Thanks, > Thanks, > >> >>> 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 >>>>>>> >>>>>>> . >>>>>>> >>>>>>
