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