Thread A Thread B Thread C - f2fs_remount - stop_gc_thread - f2fs_sbi_store - issue_discard_thread sbi->gc_thread = NULL; sbi->gc_thread->gc_wake = 1 access sbi->gc_thread->gc_urgent
Previously, we allocate memory for sbi->gc_thread based on background gc thread mount option, the memory can be released if we turn off that mount option, but still there are several places access gc_thread pointer without considering race condition, result in NULL point dereference. In order to fix this issue, introduce gc_rwsem to exclude those operations. Signed-off-by: Chao Yu <yuch...@huawei.com> --- v4: - use introduced sbi.gc_rwsem lock instead of sb.s_umount. fs/f2fs/f2fs.h | 1 + fs/f2fs/gc.c | 4 ++++ fs/f2fs/segment.c | 11 ++++++++++- fs/f2fs/super.c | 19 ++++++++++++++----- fs/f2fs/sysfs.c | 14 +++++++++++--- 5 files changed, 40 insertions(+), 9 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 80490a7991a7..e238d0ea0be7 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -1191,6 +1191,7 @@ struct f2fs_sb_info { /* for cleaning operations */ struct mutex gc_mutex; /* mutex for GC */ + struct rw_semaphore gc_rwsem; /* rw semaphore for gc_thread */ struct f2fs_gc_kthread *gc_thread; /* GC thread */ unsigned int cur_victim_sec; /* current victim section num */ diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index 9bb2ddbbed1e..b74714be7be7 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -187,17 +187,21 @@ static void select_policy(struct f2fs_sb_info *sbi, int gc_type, p->max_search = dirty_i->nr_dirty[type]; p->ofs_unit = 1; } else { + down_read(&sbi->gc_rwsem); p->gc_mode = select_gc_type(sbi->gc_thread, gc_type); + up_read(&sbi->gc_rwsem); p->dirty_segmap = dirty_i->dirty_segmap[DIRTY]; p->max_search = dirty_i->nr_dirty[DIRTY]; p->ofs_unit = sbi->segs_per_sec; } /* we need to check every dirty segments in the FG_GC case */ + down_read(&sbi->gc_rwsem); if (gc_type != FG_GC && (sbi->gc_thread && !sbi->gc_thread->gc_urgent) && p->max_search > sbi->max_victim_search) p->max_search = sbi->max_victim_search; + up_read(&sbi->gc_rwsem); /* let's select beginning hot/small space first in no_heap mode*/ if (test_opt(sbi, NOHEAP) && diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 320cc1c57246..33d146939048 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -174,11 +174,18 @@ bool need_SSR(struct f2fs_sb_info *sbi) int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES); int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS); int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA); + bool gc_urgent = false; if (test_opt(sbi, LFS)) return false; + + down_read(&sbi->gc_rwsem); if (sbi->gc_thread && sbi->gc_thread->gc_urgent) - return true; + gc_urgent = true; + up_read(&sbi->gc_rwsem); + + if (gc_urgent) + return false; return free_sections(sbi) <= (node_secs + 2 * dent_secs + imeta_secs + SM_I(sbi)->min_ssr_sections + reserved_sections(sbi)); @@ -1421,8 +1428,10 @@ static int issue_discard_thread(void *data) if (dcc->discard_wake) dcc->discard_wake = 0; + down_read(&sbi->gc_rwsem); if (sbi->gc_thread && sbi->gc_thread->gc_urgent) init_discard_policy(&dpolicy, DPOLICY_FORCE, 1); + up_read(&sbi->gc_rwsem); sb_start_intwrite(sbi->sb); diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index c8e5fe5d71fe..294be9e92aee 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -1476,15 +1476,23 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) * option. Also sync the filesystem. */ if ((*flags & SB_RDONLY) || !test_opt(sbi, BG_GC)) { + down_write(&sbi->gc_rwsem); if (sbi->gc_thread) { stop_gc_thread(sbi); need_restart_gc = true; } - } else if (!sbi->gc_thread) { - err = start_gc_thread(sbi); - if (err) - goto restore_opts; - need_stop_gc = true; + up_write(&sbi->gc_rwsem); + } else { + down_write(&sbi->gc_rwsem); + if (!sbi->gc_thread) { + err = start_gc_thread(sbi); + if (err) { + up_write(&sbi->gc_rwsem); + goto restore_opts; + } + need_stop_gc = true; + } + up_write(&sbi->gc_rwsem); } if (*flags & SB_RDONLY || @@ -2802,6 +2810,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) } init_rwsem(&sbi->cp_rwsem); + init_rwsem(&sbi->gc_rwsem); init_waitqueue_head(&sbi->cp_wait); init_sb_info(sbi); diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c index 6d8d8f41e517..34c542dbc459 100644 --- a/fs/f2fs/sysfs.c +++ b/fs/f2fs/sysfs.c @@ -173,6 +173,7 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a, unsigned long t; unsigned int *ui; ssize_t ret; + bool gc_entry = (a->struct_type == GC_THREAD); ptr = __struct_ptr(sbi, a->struct_type); if (!ptr) @@ -248,16 +249,23 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a, if (!strcmp(a->attr.name, "trim_sections")) return -EINVAL; - *ui = t; - - if (!strcmp(a->attr.name, "iostat_enable") && *ui == 0) + if (!strcmp(a->attr.name, "iostat_enable") && t == 0) f2fs_reset_iostat(sbi); + + if (gc_entry) + down_read(&sbi->gc_rwsem); + if (!strcmp(a->attr.name, "gc_urgent") && t == 1 && sbi->gc_thread) { sbi->gc_thread->gc_wake = 1; wake_up_interruptible_all(&sbi->gc_thread->gc_wait_queue_head); wake_up_discard_thread(sbi, true); } + *ui = t; + + if (gc_entry) + up_read(&sbi->gc_rwsem); + return count; } -- 2.17.0.391.g1f1cddd558b5