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.

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

Reply via email to