Thanks,
*From:*Gao Xiang
*To:*guoweichao,
*Cc:*linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx,heyunlei,
*Date:*2018-01-20 11:49:22
*Subject:*Re: [PATCH RFC] f2fs: add PRE2 to mark segments free to one
checkpoint but obsolete to the other
Hi Weichao,
On 2018/1/19 23:47, guoweichao wrote:
> A critical case is using the free segments as data segments which
> are previously node segments to the old checkpoint. With fault injecting
> of the newer CP pack, fsck can find errors when checking the sanity
of nid.
Sorry to interrupt because I'm just curious about this scenario and the
detail.
As far as I know, if the whole blocks in a segment become invalid,
the segment will become PREFREE, and then if a checkpoint is followed,
we can reuse this segment or
discard the whole segment safely after this checkpoint was done
(I think It makes sure that this segment is certainly FREE and not
reused in this checkpoint).
If the segment in the old checkpoint is a node segment, and node blocks
in the segment are all invalid until the new checkpoint.
It seems no danger to reuse the FREE node segment as a data segment in
the next checkpoint?
or something related to SPOR? In my mind f2fs-tools ignores POR node
chain...
Thanks,
> On 2018/1/20 7:29, Weichao Guo wrote:
>> Currently, we set prefree segments as free ones after writing
>> a checkpoint, then believe the segments could be used safely.
>> However, if a sudden power-off coming and the newer checkpoint
>> corrupted due to hardware issues at the same time, we will try
>> to use the old checkpoint and may face an inconsistent file
>> system status.
>>
>> How about add an PRE2 status for prefree segments, and make
>> sure the segments could be used safely to both checkpoints?
>> Or any better solutions? Or this is not a problem?
>>
>> Look forward to your comments!
>>
>> Signed-off-by: Weichao Guo <guoweichao@xxxxxxxxxx>
>> ---
>> fs/f2fs/gc.c | 11 +++++++++--
>> fs/f2fs/segment.c | 21 ++++++++++++++++++---
>> fs/f2fs/segment.h | 6 ++++++
>> 3 files changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index 33e7969..153e3ea 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -1030,7 +1030,12 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>> * threshold, we can make them free by checkpoint.
Then, we
>> * secure free segments which doesn't need fggc any more.
>> */
>> - if (prefree_segments(sbi)) {
>> + if (prefree_segments(sbi) || prefree2_segments(sbi)) {
>> + ret = write_checkpoint(sbi, &cpc);
>> + if (ret)
>> + goto stop;
>> + }
>> + if (has_not_enough_free_secs(sbi, 0, 0) &&
prefree2_segments(sbi)) {
>> ret = write_checkpoint(sbi, &cpc);
>> if (ret)
>> goto stop;
>> @@ -1063,8 +1068,10 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>> goto gc_more;
>> }
>>
>> - if (gc_type == FG_GC)
>> + if (gc_type == FG_GC) {
>> + ret = write_checkpoint(sbi, &cpc);
>> ret = write_checkpoint(sbi, &cpc);
>> + }
>> }
>> stop:
>> SIT_I(sbi)->last_victim[ALLOC_NEXT] = 0;
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 2e8e054d..9dec445 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -1606,7 +1606,7 @@ static void
set_prefree_as_free_segments(struct f2fs_sb_info *sbi)
>> unsigned int segno;
>>
>> mutex_lock(&dirty_i->seglist_lock);
>> - for_each_set_bit(segno, dirty_i->dirty_segmap[PRE],
MAIN_SEGS(sbi))
>> + for_each_set_bit(segno, dirty_i->dirty_segmap[PRE2],
MAIN_SEGS(sbi))
>> __set_test_and_free(sbi, segno);
>> mutex_unlock(&dirty_i->seglist_lock);
>> }
>> @@ -1617,13 +1617,17 @@ void clear_prefree_segments(struct
f2fs_sb_info *sbi, struct cp_control *cpc)
>> struct list_head *head = &dcc->entry_list;
>> struct discard_entry *entry, *this;
>> struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
>> - unsigned long *prefree_map = dirty_i->dirty_segmap[PRE];
>> + unsigned long *prefree_map;
>> unsigned int start = 0, end = -1;
>> unsigned int secno, start_segno;
>> bool force = (cpc->reason & CP_DISCARD);
>> + int phase = 0;
>> + enum dirty_type dirty_type = PRE2;
>>
>> mutex_lock(&dirty_i->seglist_lock);
>>
>> +next_step:
>> + prefree_map = dirty_i->dirty_segmap[dirty_type];
>> while (1) {
>> int i;
>> start = find_next_bit(prefree_map, MAIN_SEGS(sbi),
end + 1);
>> @@ -1635,7 +1639,7 @@ void clear_prefree_segments(struct
f2fs_sb_info *sbi, struct cp_control *cpc)
>> for (i = start; i < end; i++)
>> clear_bit(i, prefree_map);
>>
>> - dirty_i->nr_dirty[PRE] -= end - start;
>> + dirty_i->nr_dirty[dirty_type] -= end - start;
>>
>> if (!test_opt(sbi, DISCARD))
>> continue;
>> @@ -1663,6 +1667,16 @@ void clear_prefree_segments(struct
f2fs_sb_info *sbi, struct cp_control *cpc)
>> else
>> end = start - 1;
>> }
>> + if (phase == 0) {
>> + /* status change: PRE -> PRE2 */
>> + for_each_set_bit(segno, dirty_i->dirty_segmap[PRE],
MAIN_SEGS(sbi))
>> + if (!test_and_set_bit(segno, prefree_map))
>> + dirty_i->nr_dirty[PRE2]++;
>> +
>> + phase = 1;
>> + dirty_type = PRE;
>> + goto next_step;
>> + }
>> mutex_unlock(&dirty_i->seglist_lock);
>>
>> /* send small discards */
>> @@ -2245,6 +2259,7 @@ static void change_curseg(struct f2fs_sb_info
*sbi, int type)
>>
>> mutex_lock(&dirty_i->seglist_lock);
>> __remove_dirty_segment(sbi, new_segno, PRE);
>> + __remove_dirty_segment(sbi, new_segno, PRE2);
>> __remove_dirty_segment(sbi, new_segno, DIRTY);
>> mutex_unlock(&dirty_i->seglist_lock);
>>
>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>> index 71a2aaa..f702726 100644
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -263,6 +263,7 @@ enum dirty_type {
>> DIRTY_COLD_NODE, /* dirty segments assigned as cold
node logs */
>> DIRTY, /* to count # of dirty segments */
>> PRE, /* to count # of entirely obsolete
segments */
>> + PRE2, /* to count # of the segments free to
one checkpoint but obsolete to the other checkpoint */
>> NR_DIRTY_TYPE
>> };
>>
>> @@ -477,6 +478,11 @@ static inline unsigned int
prefree_segments(struct f2fs_sb_info *sbi)
>> return DIRTY_I(sbi)->nr_dirty[PRE];
>> }
>>
>> +static inline unsigned int prefree2_segments(struct f2fs_sb_info *sbi)
>> +{
>> + return DIRTY_I(sbi)->nr_dirty[PRE2];
>> +}
>> +
>> static inline unsigned int dirty_segments(struct f2fs_sb_info *sbi)
>> {
>> return DIRTY_I(sbi)->nr_dirty[DIRTY_HOT_DATA] +
>>
