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