On 2018/11/6 12:08, Yunlei He wrote: > Now, discard thread will issue discards in parallel with fstrim, > which will cause problem as below: > i. maybe too many discards commands at some point > ii. disturb the order of block address, which is continuous in fstrim
I'm not strongly against this patch, but one potential issue I can image is: If fstrim is called from userspace periodically with large granularity, it can cause kernel discard thread always skip issuing small granularity discard, result in bad performance due to too many invalid mapping in device. Not sure, it may create a chance for hacker to use this to trigger a Dos attack on f2fs. Thanks, > > Signed-off-by: Yunlei He <[email protected]> > --- > fs/f2fs/f2fs.h | 1 + > fs/f2fs/segment.c | 7 ++++++- > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 56204a8..fec596d2 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -330,6 +330,7 @@ struct discard_cmd_control { > atomic_t discard_cmd_cnt; /* # of cached cmd count */ > struct rb_root_cached root; /* root of discard rb-tree */ > bool rbtree_check; /* config for consistence check > */ > + atomic_t in_fstrim_cnt; /* # of fstrim in progress */ > }; > > /* for the list of fsync inodes, used only during recovery */ > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 6edcf83..cea1d27 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -1655,7 +1655,8 @@ static int issue_discard_thread(void *data) > continue; > if (kthread_should_stop()) > return 0; > - if (is_sbi_flag_set(sbi, SBI_NEED_FSCK)) { > + if (is_sbi_flag_set(sbi, SBI_NEED_FSCK) > + || atomic_read(&dcc->in_fstrim_cnt)) { > wait_ms = dpolicy.max_interval; > continue; > } > @@ -1996,6 +1997,7 @@ static int create_discard_cmd_control(struct > f2fs_sb_info *sbi) > atomic_set(&dcc->issued_discard, 0); > atomic_set(&dcc->issing_discard, 0); > atomic_set(&dcc->discard_cmd_cnt, 0); > + atomic_set(&dcc->in_fstrim_cnt, 0); > dcc->nr_discards = 0; > dcc->max_discards = MAIN_SEGS(sbi) << sbi->log_blocks_per_seg; > dcc->undiscard_blks = 0; > @@ -2720,6 +2722,7 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct > fstrim_range *range) > block_t start_block, end_block; > struct cp_control cpc; > struct discard_policy dpolicy; > + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info; > unsigned long long trimmed = 0; > int err = 0; > bool need_align = test_opt(sbi, LFS) && sbi->segs_per_sec > 1; > @@ -2772,11 +2775,13 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct > fstrim_range *range) > end_block = START_BLOCK(sbi, end_segno + 1); > > __init_discard_policy(sbi, &dpolicy, DPOLICY_FSTRIM, cpc.trim_minlen); > + atomic_inc(&dcc->in_fstrim_cnt); > trimmed = __issue_discard_cmd_range(sbi, &dpolicy, > start_block, end_block); > > trimmed += __wait_discard_cmd_range(sbi, &dpolicy, > start_block, end_block); > + atomic_dec(&dcc->in_fstrim_cnt); > out: > if (!err) > range->len = F2FS_BLK_TO_BYTES(trimmed); > _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
