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 <[email protected]>
>>> ---
>>> 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
>> [email protected]
>> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
