[PATCH v2] f2fs: use cur_map instead of ckpt_map

[PATCH v2] f2fs: use cur_map instead of ckpt_map

[Yunlei He]

In flush_sit_entries::add_discard_addrs path, cur_map and ckpt_map
are the same, but in exist_trim_candidates::add_discard_addrs,
cur_map is updated one, and newer than ckpt_map, so we need to use
cur_map to check whether there are discard candidates.

v1->v2: one problem of check discard candidates reported by Chao,
besides, update commit message.

Signed-off-by: Yunlei He <heyunlei@huawei.com>
---
 fs/f2fs/segment.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 5854cc4..f5d0499 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1559,7 +1559,7 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
 
 	/* SIT_VBLOCK_MAP_SIZE should be multiple of sizeof(unsigned long) */
 	for (i = 0; i < entries; i++)
-		dmap[i] = force ? ~ckpt_map[i] & ~discard_map[i] :
+		dmap[i] = force ? ~cur_map[i] & ~discard_map[i] :
 				(cur_map[i] ^ ckpt_map[i]) & ckpt_map[i];
 
 	while (force || SM_I(sbi)->dcc_info->nr_discards <=
-- 
1.9.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map

[Chao Yu]

On 2018/4/10 10:50, Yunlei He wrote:
> In flush_sit_entries::add_discard_addrs path, cur_map and ckpt_map
> are the same, but in exist_trim_candidates::add_discard_addrs,
> cur_map is updated one, and newer than ckpt_map, so we need to use
> cur_map to check whether there are discard candidates.
> 
> v1->v2: one problem of check discard candidates reported by Chao,
> besides, update commit message.
> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map

[Jaegeuk Kim]

On 04/10, Yunlei He wrote:
> In flush_sit_entries::add_discard_addrs path, cur_map and ckpt_map
> are the same, but in exist_trim_candidates::add_discard_addrs,
> cur_map is updated one, and newer than ckpt_map, so we need to use
> cur_map to check whether there are discard candidates.
> 
> v1->v2: one problem of check discard candidates reported by Chao,
> besides, update commit message.
> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
>  fs/f2fs/segment.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 5854cc4..f5d0499 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1559,7 +1559,7 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>  
>  	/* SIT_VBLOCK_MAP_SIZE should be multiple of sizeof(unsigned long) */
>  	for (i = 0; i < entries; i++)
> -		dmap[i] = force ? ~ckpt_map[i] & ~discard_map[i] :
> +		dmap[i] = force ? ~cur_map[i] & ~discard_map[i] :

NAK. We're able to loose data for roll-forward recovery.

>  				(cur_map[i] ^ ckpt_map[i]) & ckpt_map[i];
>  
>  	while (force || SM_I(sbi)->dcc_info->nr_discards <=
> -- 
> 1.9.1

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map

[Chao Yu]

On 2018/4/13 6:37, Jaegeuk Kim wrote:
> On 04/10, Yunlei He wrote:
>> In flush_sit_entries::add_discard_addrs path, cur_map and ckpt_map
>> are the same, but in exist_trim_candidates::add_discard_addrs,
>> cur_map is updated one, and newer than ckpt_map, so we need to use
>> cur_map to check whether there are discard candidates.
>>
>> v1->v2: one problem of check discard candidates reported by Chao,
>> besides, update commit message.
>>
>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>> ---
>>  fs/f2fs/segment.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 5854cc4..f5d0499 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -1559,7 +1559,7 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>>  
>>  	/* SIT_VBLOCK_MAP_SIZE should be multiple of sizeof(unsigned long) */
>>  	for (i = 0; i < entries; i++)
>> -		dmap[i] = force ? ~ckpt_map[i] & ~discard_map[i] :
>> +		dmap[i] = force ? ~cur_map[i] & ~discard_map[i] :
> 
> NAK. We're able to loose data for roll-forward recovery.

Actually, in fstrim flow (force == 1), cur_map is complete the same as ckpt_map.
Do you suffer data corruption during testing this patch?

Thanks,

> 
>>  				(cur_map[i] ^ ckpt_map[i]) & ckpt_map[i];
>>  
>>  	while (force || SM_I(sbi)->dcc_info->nr_discards <=
>> -- 
>> 1.9.1
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map

[heyunlei]

>-----Original Message-----
>From: Yuchao (T)
>Sent: Friday, April 13, 2018 9:12 AM
>To: Jaegeuk Kim; heyunlei
>Cc: linux-f2fs-devel@lists.sourceforge.net; Wangbintian; Zhangdianfang (Euler)
>Subject: Re: [f2fs-dev][PATCH v2] f2fs: use cur_map instead of ckpt_map
>
>On 2018/4/13 6:37, Jaegeuk Kim wrote:
>> On 04/10, Yunlei He wrote:
>>> In flush_sit_entries::add_discard_addrs path, cur_map and ckpt_map
>>> are the same, but in exist_trim_candidates::add_discard_addrs,
>>> cur_map is updated one, and newer than ckpt_map, so we need to use
>>> cur_map to check whether there are discard candidates.
>>>
>>> v1->v2: one problem of check discard candidates reported by Chao,
>>> besides, update commit message.
>>>
>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>> ---
>>>  fs/f2fs/segment.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 5854cc4..f5d0499 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -1559,7 +1559,7 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>>>
>>>  	/* SIT_VBLOCK_MAP_SIZE should be multiple of sizeof(unsigned long) */
>>>  	for (i = 0; i < entries; i++)
>>> -		dmap[i] = force ? ~ckpt_map[i] & ~discard_map[i] :
>>> +		dmap[i] = force ? ~cur_map[i] & ~discard_map[i] :
>>
>> NAK. We're able to loose data for roll-forward recovery.
Ping

Thanks.
>
>Actually, in fstrim flow (force == 1), cur_map is complete the same as ckpt_map.
>Do you suffer data corruption during testing this patch?
>
>Thanks,
>
>>
>>>  				(cur_map[i] ^ ckpt_map[i]) & ckpt_map[i];
>>>
>>>  	while (force || SM_I(sbi)->dcc_info->nr_discards <=
>>> --
>>> 1.9.1
>>
>> .
>>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map

[Jaegeuk Kim]

On 04/20, heyunlei wrote:
> 
> 
> >-----Original Message-----
> >From: Yuchao (T)
> >Sent: Friday, April 13, 2018 9:12 AM
> >To: Jaegeuk Kim; heyunlei
> >Cc: linux-f2fs-devel@lists.sourceforge.net; Wangbintian; Zhangdianfang (Euler)
> >Subject: Re: [f2fs-dev][PATCH v2] f2fs: use cur_map instead of ckpt_map
> >
> >On 2018/4/13 6:37, Jaegeuk Kim wrote:
> >> On 04/10, Yunlei He wrote:
> >>> In flush_sit_entries::add_discard_addrs path, cur_map and ckpt_map
> >>> are the same, but in exist_trim_candidates::add_discard_addrs,
> >>> cur_map is updated one, and newer than ckpt_map, so we need to use
> >>> cur_map to check whether there are discard candidates.
> >>>
> >>> v1->v2: one problem of check discard candidates reported by Chao,
> >>> besides, update commit message.
> >>>
> >>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> >>> ---
> >>>  fs/f2fs/segment.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >>> index 5854cc4..f5d0499 100644
> >>> --- a/fs/f2fs/segment.c
> >>> +++ b/fs/f2fs/segment.c
> >>> @@ -1559,7 +1559,7 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
> >>>
> >>>  	/* SIT_VBLOCK_MAP_SIZE should be multiple of sizeof(unsigned long) */
> >>>  	for (i = 0; i < entries; i++)
> >>> -		dmap[i] = force ? ~ckpt_map[i] & ~discard_map[i] :
> >>> +		dmap[i] = force ? ~cur_map[i] & ~discard_map[i] :
> >>
> >> NAK. We're able to loose data for roll-forward recovery.
> Ping
> 
> Thanks.
> >
> >Actually, in fstrim flow (force == 1), cur_map is complete the same as ckpt_map.
> >Do you suffer data corruption during testing this patch?

So, we don't need this patch, IIUC.

> >
> >Thanks,
> >
> >>
> >>>  				(cur_map[i] ^ ckpt_map[i]) & ckpt_map[i];
> >>>
> >>>  	while (force || SM_I(sbi)->dcc_info->nr_discards <=
> >>> --
> >>> 1.9.1
> >>
> >> .
> >>

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map

[Chao Yu]

On 2018/4/20 11:26, Jaegeuk Kim wrote:
> On 04/20, heyunlei wrote:
>>
>>
>>> -----Original Message-----
>>> From: Yuchao (T)
>>> Sent: Friday, April 13, 2018 9:12 AM
>>> To: Jaegeuk Kim; heyunlei
>>> Cc: linux-f2fs-devel@lists.sourceforge.net; Wangbintian; Zhangdianfang (Euler)
>>> Subject: Re: [f2fs-dev][PATCH v2] f2fs: use cur_map instead of ckpt_map
>>>
>>> On 2018/4/13 6:37, Jaegeuk Kim wrote:
>>>> On 04/10, Yunlei He wrote:
>>>>> In flush_sit_entries::add_discard_addrs path, cur_map and ckpt_map
>>>>> are the same, but in exist_trim_candidates::add_discard_addrs,
>>>>> cur_map is updated one, and newer than ckpt_map, so we need to use
>>>>> cur_map to check whether there are discard candidates.
>>>>>
>>>>> v1->v2: one problem of check discard candidates reported by Chao,
>>>>> besides, update commit message.
>>>>>
>>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>>> ---
>>>>>  fs/f2fs/segment.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>>> index 5854cc4..f5d0499 100644
>>>>> --- a/fs/f2fs/segment.c
>>>>> +++ b/fs/f2fs/segment.c
>>>>> @@ -1559,7 +1559,7 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>>>>>
>>>>>  	/* SIT_VBLOCK_MAP_SIZE should be multiple of sizeof(unsigned long) */
>>>>>  	for (i = 0; i < entries; i++)
>>>>> -		dmap[i] = force ? ~ckpt_map[i] & ~discard_map[i] :
>>>>> +		dmap[i] = force ? ~cur_map[i] & ~discard_map[i] :
>>>>
>>>> NAK. We're able to loose data for roll-forward recovery.
>> Ping
>>
>> Thanks.
>>>
>>> Actually, in fstrim flow (force == 1), cur_map is complete the same as ckpt_map.
>>> Do you suffer data corruption during testing this patch?
> 
> So, we don't need this patch, IIUC.

No, in precheck flow, we haven't flush cur_map to ckpt_map, so they are different.

- fstrim
 - exist_trim_candidates
  - add_discard_addrs

> 
>>>
>>> Thanks,
>>>
>>>>
>>>>>  				(cur_map[i] ^ ckpt_map[i]) & ckpt_map[i];
>>>>>
>>>>>  	while (force || SM_I(sbi)->dcc_info->nr_discards <=
>>>>> --
>>>>> 1.9.1
>>>>
>>>> .
>>>>
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map

[Jaegeuk Kim]

On 04/20, Chao Yu wrote:
> On 2018/4/20 11:26, Jaegeuk Kim wrote:
> > On 04/20, heyunlei wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Yuchao (T)
> >>> Sent: Friday, April 13, 2018 9:12 AM
> >>> To: Jaegeuk Kim; heyunlei
> >>> Cc: linux-f2fs-devel@lists.sourceforge.net; Wangbintian; Zhangdianfang (Euler)
> >>> Subject: Re: [f2fs-dev][PATCH v2] f2fs: use cur_map instead of ckpt_map
> >>>
> >>> On 2018/4/13 6:37, Jaegeuk Kim wrote:
> >>>> On 04/10, Yunlei He wrote:
> >>>>> In flush_sit_entries::add_discard_addrs path, cur_map and ckpt_map
> >>>>> are the same, but in exist_trim_candidates::add_discard_addrs,
> >>>>> cur_map is updated one, and newer than ckpt_map, so we need to use
> >>>>> cur_map to check whether there are discard candidates.
> >>>>>
> >>>>> v1->v2: one problem of check discard candidates reported by Chao,
> >>>>> besides, update commit message.
> >>>>>
> >>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> >>>>> ---
> >>>>>  fs/f2fs/segment.c | 2 +-
> >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >>>>> index 5854cc4..f5d0499 100644
> >>>>> --- a/fs/f2fs/segment.c
> >>>>> +++ b/fs/f2fs/segment.c
> >>>>> @@ -1559,7 +1559,7 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
> >>>>>
> >>>>>  	/* SIT_VBLOCK_MAP_SIZE should be multiple of sizeof(unsigned long) */
> >>>>>  	for (i = 0; i < entries; i++)
> >>>>> -		dmap[i] = force ? ~ckpt_map[i] & ~discard_map[i] :
> >>>>> +		dmap[i] = force ? ~cur_map[i] & ~discard_map[i] :
> >>>>
> >>>> NAK. We're able to loose data for roll-forward recovery.
> >> Ping
> >>
> >> Thanks.
> >>>
> >>> Actually, in fstrim flow (force == 1), cur_map is complete the same as ckpt_map.
> >>> Do you suffer data corruption during testing this patch?
> > 
> > So, we don't need this patch, IIUC.
> 
> No, in precheck flow, we haven't flush cur_map to ckpt_map, so they are different.

Then, what if power-cut happens between exist_trim_candidates() and
do_checkpoint()?

> 
> - fstrim
>  - exist_trim_candidates
>   - add_discard_addrs
> 
> > 
> >>>
> >>> Thanks,
> >>>
> >>>>
> >>>>>  				(cur_map[i] ^ ckpt_map[i]) & ckpt_map[i];
> >>>>>
> >>>>>  	while (force || SM_I(sbi)->dcc_info->nr_discards <=
> >>>>> --
> >>>>> 1.9.1
> >>>>
> >>>> .
> >>>>
> > 
> > .
> > 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map

[heyunlei]

>-----Original Message-----
>From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
>Sent: Friday, April 20, 2018 11:41 AM
>To: Yuchao (T)
>Cc: heyunlei; linux-f2fs-devel@lists.sourceforge.net; Wangbintian; Zhangdianfang (Euler)
>Subject: Re: [f2fs-dev][PATCH v2] f2fs: use cur_map instead of ckpt_map
>
>On 04/20, Chao Yu wrote:
>> On 2018/4/20 11:26, Jaegeuk Kim wrote:
>> > On 04/20, heyunlei wrote:
>> >>
>> >>
>> >>> -----Original Message-----
>> >>> From: Yuchao (T)
>> >>> Sent: Friday, April 13, 2018 9:12 AM
>> >>> To: Jaegeuk Kim; heyunlei
>> >>> Cc: linux-f2fs-devel@lists.sourceforge.net; Wangbintian; Zhangdianfang (Euler)
>> >>> Subject: Re: [f2fs-dev][PATCH v2] f2fs: use cur_map instead of ckpt_map
>> >>>
>> >>> On 2018/4/13 6:37, Jaegeuk Kim wrote:
>> >>>> On 04/10, Yunlei He wrote:
>> >>>>> In flush_sit_entries::add_discard_addrs path, cur_map and ckpt_map
>> >>>>> are the same, but in exist_trim_candidates::add_discard_addrs,
>> >>>>> cur_map is updated one, and newer than ckpt_map, so we need to use
>> >>>>> cur_map to check whether there are discard candidates.
>> >>>>>
>> >>>>> v1->v2: one problem of check discard candidates reported by Chao,
>> >>>>> besides, update commit message.
>> >>>>>
>> >>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>> >>>>> ---
>> >>>>>  fs/f2fs/segment.c | 2 +-
>> >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>>>>
>> >>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> >>>>> index 5854cc4..f5d0499 100644
>> >>>>> --- a/fs/f2fs/segment.c
>> >>>>> +++ b/fs/f2fs/segment.c
>> >>>>> @@ -1559,7 +1559,7 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>> >>>>>
>> >>>>>  	/* SIT_VBLOCK_MAP_SIZE should be multiple of sizeof(unsigned long) */
>> >>>>>  	for (i = 0; i < entries; i++)
>> >>>>> -		dmap[i] = force ? ~ckpt_map[i] & ~discard_map[i] :
>> >>>>> +		dmap[i] = force ? ~cur_map[i] & ~discard_map[i] :
>> >>>>
>> >>>> NAK. We're able to loose data for roll-forward recovery.
>> >> Ping
>> >>
>> >> Thanks.
>> >>>
>> >>> Actually, in fstrim flow (force == 1), cur_map is complete the same as ckpt_map.
>> >>> Do you suffer data corruption during testing this patch?
>> >
>> > So, we don't need this patch, IIUC.
>>
>> No, in precheck flow, we haven't flush cur_map to ckpt_map, so they are different.
>
>Then, what if power-cut happens between exist_trim_candidates() and
>do_checkpoint()?

Discards issued after do_checkpoint:
i.  new cp write ok, discard will not affect recovery
ii.  new cp write failed(including power-cut), discards will be released, will not affet recovery too?

Anything I missing?

Thanks.
>
>>
>> - fstrim
>>  - exist_trim_candidates
>>   - add_discard_addrs
>>
>> >
>> >>>
>> >>> Thanks,
>> >>>
>> >>>>
>> >>>>>  				(cur_map[i] ^ ckpt_map[i]) & ckpt_map[i];
>> >>>>>
>> >>>>>  	while (force || SM_I(sbi)->dcc_info->nr_discards <=
>> >>>>> --
>> >>>>> 1.9.1
>> >>>>
>> >>>> .
>> >>>>
>> >
>> > .
>> >

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map

[Chao Yu]

On 2018/4/20 11:40, Jaegeuk Kim wrote:
> On 04/20, Chao Yu wrote:
>> On 2018/4/20 11:26, Jaegeuk Kim wrote:
>>> On 04/20, heyunlei wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Yuchao (T)
>>>>> Sent: Friday, April 13, 2018 9:12 AM
>>>>> To: Jaegeuk Kim; heyunlei
>>>>> Cc: linux-f2fs-devel@lists.sourceforge.net; Wangbintian; Zhangdianfang (Euler)
>>>>> Subject: Re: [f2fs-dev][PATCH v2] f2fs: use cur_map instead of ckpt_map
>>>>>
>>>>> On 2018/4/13 6:37, Jaegeuk Kim wrote:
>>>>>> On 04/10, Yunlei He wrote:
>>>>>>> In flush_sit_entries::add_discard_addrs path, cur_map and ckpt_map
>>>>>>> are the same, but in exist_trim_candidates::add_discard_addrs,
>>>>>>> cur_map is updated one, and newer than ckpt_map, so we need to use
>>>>>>> cur_map to check whether there are discard candidates.
>>>>>>>
>>>>>>> v1->v2: one problem of check discard candidates reported by Chao,
>>>>>>> besides, update commit message.
>>>>>>>
>>>>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>>>>> ---
>>>>>>>  fs/f2fs/segment.c | 2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>>>>> index 5854cc4..f5d0499 100644
>>>>>>> --- a/fs/f2fs/segment.c
>>>>>>> +++ b/fs/f2fs/segment.c
>>>>>>> @@ -1559,7 +1559,7 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>>>>>>>
>>>>>>>  	/* SIT_VBLOCK_MAP_SIZE should be multiple of sizeof(unsigned long) */
>>>>>>>  	for (i = 0; i < entries; i++)
>>>>>>> -		dmap[i] = force ? ~ckpt_map[i] & ~discard_map[i] :
>>>>>>> +		dmap[i] = force ? ~cur_map[i] & ~discard_map[i] :
>>>>>>
>>>>>> NAK. We're able to loose data for roll-forward recovery.
>>>> Ping
>>>>
>>>> Thanks.
>>>>>
>>>>> Actually, in fstrim flow (force == 1), cur_map is complete the same as ckpt_map.
>>>>> Do you suffer data corruption during testing this patch?
>>>
>>> So, we don't need this patch, IIUC.
>>
>> No, in precheck flow, we haven't flush cur_map to ckpt_map, so they are different.
> 
> Then, what if power-cut happens between exist_trim_candidates() and
> do_checkpoint()?

For example:
1. write data block to block address #N
2. write checkpoint				cur_map:1, ckpt_map:1
3. punch data block in block address #N		cur_map:0, ckpt_map:1
4. call fstrim with range [0, X], X > N
5. exist_trim_candidates should check cur_map to decide whether we need a
checkpoint and further discard? otherwise we will fail to trim suitable candidates?

I have one other question:

Discard should be issued after a checkpoint, right? except below codes:

		if (NM_I(sbi)->dirty_nat_cnt == 0 &&
				SIT_I(sbi)->dirty_sentries == 0 &&
				prefree_segments(sbi) == 0) {
			flush_sit_entries(sbi, cpc);
			clear_prefree_segments(sbi, cpc);
			unblock_operations(sbi);
			goto out;
		}

Why can we call flush_sit_entries & clear_prefree_segments without checkpoint?

Thanks,

> 
>>
>> - fstrim
>>  - exist_trim_candidates
>>   - add_discard_addrs
>>
>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>>>
>>>>>>>  				(cur_map[i] ^ ckpt_map[i]) & ckpt_map[i];
>>>>>>>
>>>>>>>  	while (force || SM_I(sbi)->dcc_info->nr_discards <=
>>>>>>> --
>>>>>>> 1.9.1
>>>>>>
>>>>>> .
>>>>>>
>>>
>>> .
>>>
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot