* [PATCH v2] f2fs: use cur_map instead of ckpt_map
@ 2018-04-10 2:50 Yunlei He
2018-04-10 6:37 ` Chao Yu
2018-04-12 22:37 ` Jaegeuk Kim
0 siblings, 2 replies; 10+ messages in thread
From: Yunlei He @ 2018-04-10 2:50 UTC (permalink / raw)
To: jaegeuk, yuchao0, linux-f2fs-devel; +Cc: zhangdianfang
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map
2018-04-10 2:50 [PATCH v2] f2fs: use cur_map instead of ckpt_map Yunlei He
@ 2018-04-10 6:37 ` Chao Yu
2018-04-12 22:37 ` Jaegeuk Kim
1 sibling, 0 replies; 10+ messages in thread
From: Chao Yu @ 2018-04-10 6:37 UTC (permalink / raw)
To: Yunlei He, jaegeuk, linux-f2fs-devel; +Cc: zhangdianfang
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map
2018-04-10 2:50 [PATCH v2] f2fs: use cur_map instead of ckpt_map Yunlei He
2018-04-10 6:37 ` Chao Yu
@ 2018-04-12 22:37 ` Jaegeuk Kim
2018-04-13 1:12 ` Chao Yu
1 sibling, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2018-04-12 22:37 UTC (permalink / raw)
To: Yunlei He; +Cc: zhangdianfang, linux-f2fs-devel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map
2018-04-12 22:37 ` Jaegeuk Kim
@ 2018-04-13 1:12 ` Chao Yu
2018-04-20 1:41 ` heyunlei
0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2018-04-13 1:12 UTC (permalink / raw)
To: Jaegeuk Kim, Yunlei He; +Cc: zhangdianfang, linux-f2fs-devel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map
2018-04-13 1:12 ` Chao Yu
@ 2018-04-20 1:41 ` heyunlei
2018-04-20 3:26 ` Jaegeuk Kim
0 siblings, 1 reply; 10+ messages in thread
From: heyunlei @ 2018-04-20 1:41 UTC (permalink / raw)
To: Yuchao (T), Jaegeuk Kim; +Cc: Zhangdianfang (Euler), linux-f2fs-devel
>-----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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map
2018-04-20 1:41 ` heyunlei
@ 2018-04-20 3:26 ` Jaegeuk Kim
2018-04-20 3:31 ` Chao Yu
0 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2018-04-20 3:26 UTC (permalink / raw)
To: heyunlei; +Cc: Zhangdianfang (Euler), linux-f2fs-devel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map
2018-04-20 3:26 ` Jaegeuk Kim
@ 2018-04-20 3:31 ` Chao Yu
2018-04-20 3:40 ` Jaegeuk Kim
0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2018-04-20 3:31 UTC (permalink / raw)
To: Jaegeuk Kim, heyunlei; +Cc: Zhangdianfang (Euler), linux-f2fs-devel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map
2018-04-20 3:31 ` Chao Yu
@ 2018-04-20 3:40 ` Jaegeuk Kim
2018-04-20 3:57 ` heyunlei
2018-04-20 7:36 ` Chao Yu
0 siblings, 2 replies; 10+ messages in thread
From: Jaegeuk Kim @ 2018-04-20 3:40 UTC (permalink / raw)
To: Chao Yu; +Cc: Zhangdianfang (Euler), linux-f2fs-devel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map
2018-04-20 3:40 ` Jaegeuk Kim
@ 2018-04-20 3:57 ` heyunlei
2018-04-20 7:36 ` Chao Yu
1 sibling, 0 replies; 10+ messages in thread
From: heyunlei @ 2018-04-20 3:57 UTC (permalink / raw)
To: Jaegeuk Kim, Yuchao (T); +Cc: Zhangdianfang (Euler), linux-f2fs-devel
>-----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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: use cur_map instead of ckpt_map
2018-04-20 3:40 ` Jaegeuk Kim
2018-04-20 3:57 ` heyunlei
@ 2018-04-20 7:36 ` Chao Yu
1 sibling, 0 replies; 10+ messages in thread
From: Chao Yu @ 2018-04-20 7:36 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: Zhangdianfang (Euler), linux-f2fs-devel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-04-20 7:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-10 2:50 [PATCH v2] f2fs: use cur_map instead of ckpt_map Yunlei He
2018-04-10 6:37 ` Chao Yu
2018-04-12 22:37 ` Jaegeuk Kim
2018-04-13 1:12 ` Chao Yu
2018-04-20 1:41 ` heyunlei
2018-04-20 3:26 ` Jaegeuk Kim
2018-04-20 3:31 ` Chao Yu
2018-04-20 3:40 ` Jaegeuk Kim
2018-04-20 3:57 ` heyunlei
2018-04-20 7:36 ` Chao Yu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.