diff mbox

f2fs: update dirty status for CURSEG as well

Message ID 1507728318-113117-1-git-send-email-yunlong.song@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yunlong Song Oct. 11, 2017, 1:25 p.m. UTC
Without this patch, it will cause all the free segments using up in some
corner case. For example, there are 100 segments, and 20 of them are
reserved for ovp. If 79 segments are full of data, segment 80 becomes
CURSEG segment, write 512 blocks and then delete 511 blocks. Since it is
CURSEG segment, the __locate_dirty_segment will not update its dirty
status. Then the dirty_segments(sbi) is 0, f2fs_gc will fail to
get_victim, and f2fs_balance_fs will fail to trigger gc action. After
f2fs_balance_fs returns, f2fs can continue to write data to segment 81.
Again, segment 81 becomes CURSEG segment, write 512 blocks and delete
511 blocks, the dirty_segments(sbi) is 0 and f2fs_gc fail again. This
can finally use up all the free segments and cause panic.

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
 fs/f2fs/segment.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Chao Yu Oct. 13, 2017, 11:08 a.m. UTC | #1
Hi Yunlong,

On 2017/10/11 21:25, Yunlong Song wrote:
> Without this patch, it will cause all the free segments using up in some
> corner case. For example, there are 100 segments, and 20 of them are
> reserved for ovp. If 79 segments are full of data, segment 80 becomes
> CURSEG segment, write 512 blocks and then delete 511 blocks. Since it is
> CURSEG segment, the __locate_dirty_segment will not update its dirty
> status. Then the dirty_segments(sbi) is 0, f2fs_gc will fail to
> get_victim, and f2fs_balance_fs will fail to trigger gc action. After
> f2fs_balance_fs returns, f2fs can continue to write data to segment 81.
> Again, segment 81 becomes CURSEG segment, write 512 blocks and delete
> 511 blocks, the dirty_segments(sbi) is 0 and f2fs_gc fail again. This
> can finally use up all the free segments and cause panic.

Should we also remove the check in locate_dirty_segment?

Thanks,

> 
> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
> ---
>  fs/f2fs/segment.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index bfbcff8..0ff52d5 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -686,10 +686,6 @@ static void __locate_dirty_segment(struct f2fs_sb_info *sbi, unsigned int segno,
>  {
>  	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
>  
> -	/* need not be added */
> -	if (IS_CURSEG(sbi, segno))
> -		return;
> -
>  	if (!test_and_set_bit(segno, dirty_i->dirty_segmap[dirty_type]))
>  		dirty_i->nr_dirty[dirty_type]++;
>  
>
Yunlong Song Oct. 13, 2017, 11:15 a.m. UTC | #2
Correct, I will update the patch.

On 2017/10/13 19:08, Chao Yu wrote:
> Hi Yunlong,
>
> On 2017/10/11 21:25, Yunlong Song wrote:
>> Without this patch, it will cause all the free segments using up in some
>> corner case. For example, there are 100 segments, and 20 of them are
>> reserved for ovp. If 79 segments are full of data, segment 80 becomes
>> CURSEG segment, write 512 blocks and then delete 511 blocks. Since it is
>> CURSEG segment, the __locate_dirty_segment will not update its dirty
>> status. Then the dirty_segments(sbi) is 0, f2fs_gc will fail to
>> get_victim, and f2fs_balance_fs will fail to trigger gc action. After
>> f2fs_balance_fs returns, f2fs can continue to write data to segment 81.
>> Again, segment 81 becomes CURSEG segment, write 512 blocks and delete
>> 511 blocks, the dirty_segments(sbi) is 0 and f2fs_gc fail again. This
>> can finally use up all the free segments and cause panic.
> Should we also remove the check in locate_dirty_segment?
>
> Thanks,
>
>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>> ---
>>   fs/f2fs/segment.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index bfbcff8..0ff52d5 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -686,10 +686,6 @@ static void __locate_dirty_segment(struct f2fs_sb_info *sbi, unsigned int segno,
>>   {
>>   	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
>>   
>> -	/* need not be added */
>> -	if (IS_CURSEG(sbi, segno))
>> -		return;
>> -
>>   	if (!test_and_set_bit(segno, dirty_i->dirty_segmap[dirty_type]))
>>   		dirty_i->nr_dirty[dirty_type]++;
>>   
>>
> .
>
diff mbox

Patch

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index bfbcff8..0ff52d5 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -686,10 +686,6 @@  static void __locate_dirty_segment(struct f2fs_sb_info *sbi, unsigned int segno,
 {
 	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
 
-	/* need not be added */
-	if (IS_CURSEG(sbi, segno))
-		return;
-
 	if (!test_and_set_bit(segno, dirty_i->dirty_segmap[dirty_type]))
 		dirty_i->nr_dirty[dirty_type]++;