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