Re: [f2fs-dev] [PATCH v3] f2fs: flush cp pack except cp pack 2 page at first

From: Jaegeuk Kim <jaegeuk@kernel.org>
To: "Gaoxiang (OS)" <gaoxiang25@huawei.com>
Cc: "Yuchao (T)" <yuchao0@huawei.com>, Chao Yu <chao@kernel.org>,
	"linux-f2fs-devel@lists.sourceforge.net"
	<linux-f2fs-devel@lists.sourceforge.net>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	heyunlei <heyunlei@huawei.com>, hutj <hutj@huawei.com>,
	"Duwei (Device OS)" <weidu.du@huawei.com>
Subject: Re: [f2fs-dev] [PATCH v3] f2fs: flush cp pack except cp pack 2 page at first
Date: Tue, 30 Jan 2018 18:18:21 -0800	[thread overview]
Message-ID: <20180131021821.GC86468@jaegeuk-macbookpro.roam.corp.google.com> (raw)
In-Reply-To: <9047C53C18267742AB12E43B65C7F9F70BCD37C5@dggemi505-mbs.china.huawei.com>

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