[PATCH 2/5] f2fs: add cur_victim_sec for BG_GC to avoid skipping BG_GC victim

From: Yunlong Song
Date: Mon Jul 23 2018 - 10:10:52 EST


If f2fs aborts BG_GC, then the section bit of victim_secmap will be set,
which will cause the section skipped in the future get_victim of BG_GC.
In a worst case that each section in the victim_secmap is set and there
are enough free sections (so FG_GC can not be triggered), then BG_GC
will skip all the sections and cannot find any victims, causing BG_GC
failed each time. Besides, SSR also uses BG_GC to get ssr segment, if
many sections in the victim_secmap are set, then SSR cannot get a proper
ssr segment to allocate blocks, which makes SSR inefficiently. To fix
this problem, we can add cur_victim_sec for BG_GC similar like that in
FG_GC to avoid selecting the same section repeatedly.

Signed-off-by: Yunlong Song <yunlong.song@xxxxxxxxxx>
---
fs/f2fs/f2fs.h | 3 ++-
fs/f2fs/gc.c | 15 +++++++++------
fs/f2fs/segment.h | 3 ++-
fs/f2fs/super.c | 3 ++-
include/trace/events/f2fs.h | 18 ++++++++++++------
5 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 57a8851..f8a7b42 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1217,7 +1217,8 @@ struct f2fs_sb_info {
/* for cleaning operations */
struct mutex gc_mutex; /* mutex for GC */
struct f2fs_gc_kthread *gc_thread; /* GC thread */
- unsigned int cur_victim_sec; /* current victim section num */
+ unsigned int cur_fg_victim_sec; /* current FG_GC victim section num */
+ unsigned int cur_bg_victim_sec; /* current BG_GC victim section num */
unsigned int gc_mode; /* current GC state */
/* for skip statistic */
unsigned long long skipped_atomic_files[2]; /* FG_GC and BG_GC */
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 2ba470d..705d419 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -367,8 +367,6 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,

if (sec_usage_check(sbi, secno))
goto next;
- if (gc_type == BG_GC && test_bit(secno, dirty_i->victim_secmap))
- goto next;

cost = get_gc_cost(sbi, segno, &p);

@@ -391,14 +389,17 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
if (p.alloc_mode == LFS) {
secno = GET_SEC_FROM_SEG(sbi, p.min_segno);
if (gc_type == FG_GC)
- sbi->cur_victim_sec = secno;
- else
+ sbi->cur_fg_victim_sec = secno;
+ else {
set_bit(secno, dirty_i->victim_secmap);
+ sbi->cur_bg_victim_sec = secno;
+ }
}
*result = (p.min_segno / p.ofs_unit) * p.ofs_unit;

trace_f2fs_get_victim(sbi->sb, type, gc_type, &p,
- sbi->cur_victim_sec,
+ sbi->cur_fg_victim_sec,
+ sbi->cur_bg_victim_sec,
prefree_segments(sbi), free_segments(sbi));
}
out:
@@ -1098,7 +1099,9 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
}

if (gc_type == FG_GC)
- sbi->cur_victim_sec = NULL_SEGNO;
+ sbi->cur_fg_victim_sec = NULL_SEGNO;
+ else
+ sbi->cur_bg_victim_sec = NULL_SEGNO;

if (!sync) {
if (has_not_enough_free_secs(sbi, sec_freed, 0)) {
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 5049551..b21bb96 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -787,7 +787,8 @@ static inline block_t sum_blk_addr(struct f2fs_sb_info *sbi, int base, int type)

static inline bool sec_usage_check(struct f2fs_sb_info *sbi, unsigned int secno)
{
- if (IS_CURSEC(sbi, secno) || (sbi->cur_victim_sec == secno))
+ if (IS_CURSEC(sbi, secno) || (sbi->cur_fg_victim_sec == secno) ||
+ (sbi->cur_bg_victim_sec == secno))
return true;
return false;
}
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 7187885..ef69ebf 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2386,7 +2386,8 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
sbi->root_ino_num = le32_to_cpu(raw_super->root_ino);
sbi->node_ino_num = le32_to_cpu(raw_super->node_ino);
sbi->meta_ino_num = le32_to_cpu(raw_super->meta_ino);
- sbi->cur_victim_sec = NULL_SECNO;
+ sbi->cur_fg_victim_sec = NULL_SECNO;
+ sbi->cur_bg_victim_sec = NULL_SECNO;
sbi->max_victim_search = DEF_MAX_VICTIM_SEARCH;

sbi->dir_level = DEF_DIR_LEVEL;
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 7956989..0f01f82 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -693,10 +693,12 @@
TRACE_EVENT(f2fs_get_victim,

TP_PROTO(struct super_block *sb, int type, int gc_type,
- struct victim_sel_policy *p, unsigned int pre_victim,
+ struct victim_sel_policy *p, unsigned int pre_fg_victim,
+ unsigned int pre_bg_victim,
unsigned int prefree, unsigned int free),

- TP_ARGS(sb, type, gc_type, p, pre_victim, prefree, free),
+ TP_ARGS(sb, type, gc_type, p, pre_fg_victim, pre_bg_victim,
+ prefree, free),

TP_STRUCT__entry(
__field(dev_t, dev)
@@ -707,7 +709,8 @@
__field(unsigned int, victim)
__field(unsigned int, cost)
__field(unsigned int, ofs_unit)
- __field(unsigned int, pre_victim)
+ __field(unsigned int, pre_fg_victim)
+ __field(unsigned int, pre_bg_victim)
__field(unsigned int, prefree)
__field(unsigned int, free)
),
@@ -721,14 +724,16 @@
__entry->victim = p->min_segno;
__entry->cost = p->min_cost;
__entry->ofs_unit = p->ofs_unit;
- __entry->pre_victim = pre_victim;
+ __entry->pre_fg_victim = pre_fg_victim;
+ __entry->pre_bg_victim = pre_bg_victim;
__entry->prefree = prefree;
__entry->free = free;
),

TP_printk("dev = (%d,%d), type = %s, policy = (%s, %s, %s), "
"victim = %u, cost = %u, ofs_unit = %u, "
- "pre_victim_secno = %d, prefree = %u, free = %u",
+ "pre_fg_victim_secno = %d, pre_bg_victim_secno = %d, "
+ "prefree = %u, free = %u",
show_dev(__entry->dev),
show_data_type(__entry->type),
show_gc_type(__entry->gc_type),
@@ -737,7 +742,8 @@
__entry->victim,
__entry->cost,
__entry->ofs_unit,
- (int)__entry->pre_victim,
+ (int)__entry->pre_fg_victim,
+ (int)__entry->pre_bg_victim,
__entry->prefree,
__entry->free)
);
--
1.8.5.2