On 02/25, Chao Yu wrote:
> On 2017/2/25 10:03, Jaegeuk Kim wrote:
> > On 02/25, Chao Yu wrote:
> >> On 2017/2/25 1:45, Jaegeuk Kim wrote:
> >>> On 02/24, Chao Yu wrote:
> >>>> On 2017/2/23 17:18, Hou Pengyang wrote:
> >>>>> proc A: proc B:
> >>>>> - writeback_sb_inodes
> >>>>> - __writeback_single_inode
> >>>>> - do_writepages
> >>>>> - f2fs_write_node_pages
> >>>>> - f2fs_balance_fs_bg - write_checkpoint
> >>>>> - build_free_nids - flush_nat_entries
> >>>>> - __build_free_nids - __flush_nat_entry_set
> >>>>> - ra_meta_pages - get_next_nat_page
> >>>>> - current_nat_addr - set_to_next_nat
> >>>>> [do nat_bitmap checking] - f2fs_change_bit
> >>>>
> >>>> Both flows were protected by nat_tree_lock, so we don't need to worry
> >>>> about such
> >>>> case?
> >>>
> >>> The nat_tree_lock doesn't cover ra_meta_pages in proc A.
> >>
> >> Can we cover ra_meta_pages in __build_free_nid with nat_tree_lock?
> >
> > I don't think we need to do this only for the consistency check.
>
> For improving efficiency of NAT block readahead, otherwise we will wast IO for
> wrong NAT block in race condition.
That should be an another topic about the hit ratio of all the ra_meta_pages()
not limited nat pages here.
Thanks,
>
> Thanks,
>
> >
> > Thanks,
> >
> >>
> >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> >> index 819032961218..17ae737a958d 100644
> >> --- a/fs/f2fs/node.c
> >> +++ b/fs/f2fs/node.c
> >> @@ -1997,12 +1997,12 @@ static void __build_free_nids(struct f2fs_sb_info
> >> *sbi,
> >> bool sync, bool mount)
> >> nid = idx * NAT_ENTRY_PER_BLOCK;
> >> }
> >>
> >> + down_read(&nm_i->nat_tree_lock);
> >> +
> >> /* readahead nat pages to be scanned */
> >> ra_meta_pages(sbi, NAT_BLOCK_OFFSET(nid), FREE_NID_PAGES,
> >> META_NAT, true);
> >>
> >> - down_read(&nm_i->nat_tree_lock);
> >> -
> >> while (1) {
> >> struct page *page = get_current_nat_page(sbi, nid);
> >>
> >> @@ -2033,10 +2033,10 @@ static void __build_free_nids(struct f2fs_sb_info
> >> *sbi,
> >> bool sync, bool mount)
> >> remove_free_nid(sbi, nid);
> >> }
> >> up_read(&curseg->journal_rwsem);
> >> - up_read(&nm_i->nat_tree_lock);
> >>
> >> ra_meta_pages(sbi, NAT_BLOCK_OFFSET(nm_i->next_scan_nid),
> >> nm_i->ra_nid_pages, META_NAT,
> >> false);
> >> + up_read(&nm_i->nat_tree_lock);
> >> }
> >>
> >> void build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool mount)
> >>
> >>
> >>>
> >>> Thanks,
> >>>
> >>>>
> >>>> Thanks,
> >>>>
> >>>>>
> >>>>> For proc A, nat_bitmap and nat_bitmap_mir would be compared without
> >>>>> lock_op and
> >>>>> nm_i->nat_tree_lock, while proc B is changing nat_bitmap/nat_bitmap_ver
> >>>>> in cp.
> >>>>>
> >>>>> So it is normal for nat_bitmap/nat_bitmap diffrence under such scenario.
> >>>>>
> >>>>> This patch fix this by removing the monitoring point.
> >>>>>
> >>>>> [Fix: 599a09b f2fs: check in-memory nat version bitmap]
> >>>>> Signed-off-by: Hou Pengyang <houpengy...@huawei.com>
> >>>>> ---
> >>>>> fs/f2fs/node.h | 6 ------
> >>>>> 1 file changed, 6 deletions(-)
> >>>>>
> >>>>> diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
> >>>>> index d3d2893..3fc9c4b 100644
> >>>>> --- a/fs/f2fs/node.h
> >>>>> +++ b/fs/f2fs/node.h
> >>>>> @@ -209,12 +209,6 @@ static inline pgoff_t current_nat_addr(struct
> >>>>> f2fs_sb_info *sbi, nid_t start)
> >>>>> (seg_off << sbi->log_blocks_per_seg << 1) +
> >>>>> (block_off & (sbi->blocks_per_seg - 1)));
> >>>>>
> >>>>> -#ifdef CONFIG_F2FS_CHECK_FS
> >>>>> - if (f2fs_test_bit(block_off, nm_i->nat_bitmap) !=
> >>>>> - f2fs_test_bit(block_off, nm_i->nat_bitmap_mir))
> >>>>> - f2fs_bug_on(sbi, 1);
> >>>>> -#endif
> >>>>> -
> >>>>> if (f2fs_test_bit(block_off, nm_i->nat_bitmap))
> >>>>> block_addr += sbi->blocks_per_seg;
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>> ------------------------------------------------------------------------------
> >>>> 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
> >>>> Linux-f2fs-devel@lists.sourceforge.net
> >>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> >>>
> >>> .
> >>>
> >
> > .
> >
>
>
> ------------------------------------------------------------------------------
> 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
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
------------------------------------------------------------------------------
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
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
