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?

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

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to