From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751731AbdJCUQ7 (ORCPT ); Tue, 3 Oct 2017 16:16:59 -0400 Received: from mail.kernel.org ([198.145.29.99]:37016 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751101AbdJCUQ6 (ORCPT ); Tue, 3 Oct 2017 16:16:58 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org ED88420BED Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=jaegeuk@kernel.org Date: Tue, 3 Oct 2017 13:16:57 -0700 From: Jaegeuk Kim To: Chao Yu Cc: linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, Chao Yu Subject: Re: [PATCH 1/6] f2fs: support issuing/waiting discard in range Message-ID: <20171003201657.GA35961@jaegeuk-macbookpro.roam.corp.google.com> References: <20170919153016.4116-1-chao@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170919153016.4116-1-chao@kernel.org> User-Agent: Mutt/1.8.2 (2017-04-18) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/19, Chao Yu wrote: > From: Chao Yu > > Fstrim intends to trim invalid blocks of filesystem only with specified > range and granularity, but actually, it will issue all previous cached > discard commands which may be out-of-range and be with unmatched > granularity, it's unneeded. > > In order to fix above issues, this patch introduces new helps to support > to issue and wait discard in range and adds a new fstrim_list for tracking > in-flight discard from ->fstrim. > > Signed-off-by: Chao Yu > --- > fs/f2fs/f2fs.h | 1 + > fs/f2fs/segment.c | 125 +++++++++++++++++++++++++++++++++++++++++++++--------- > 2 files changed, 106 insertions(+), 20 deletions(-) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 89b4927dcd79..cb13c7df6971 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -249,6 +249,7 @@ struct discard_cmd_control { > struct list_head pend_list[MAX_PLIST_NUM];/* store pending entries */ > unsigned char pend_list_tag[MAX_PLIST_NUM];/* tag for pending entries */ > struct list_head wait_list; /* store on-flushing entries */ > + struct list_head fstrim_list; /* in-flight discard from fstrim */ > wait_queue_head_t discard_wait_queue; /* waiting queue for wake-up */ > unsigned int discard_wake; /* to wake up discard thread */ > struct mutex cmd_lock; > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index dedf0209d820..088936c61905 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -845,9 +845,11 @@ void __check_sit_bitmap(struct f2fs_sb_info *sbi, > > /* this function is copied from blkdev_issue_discard from block/blk-lib.c */ > static void __submit_discard_cmd(struct f2fs_sb_info *sbi, > - struct discard_cmd *dc) > + struct discard_cmd *dc, bool fstrim) > { > struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info; > + struct list_head *wait_list = fstrim ? &(dcc->fstrim_list) : > + &(dcc->wait_list); > struct bio *bio = NULL; > > if (dc->state != D_PREP) > @@ -869,7 +871,7 @@ static void __submit_discard_cmd(struct f2fs_sb_info *sbi, > bio->bi_end_io = f2fs_submit_discard_endio; > bio->bi_opf |= REQ_SYNC; > submit_bio(bio); > - list_move_tail(&dc->list, &dcc->wait_list); > + list_move_tail(&dc->list, wait_list); > __check_sit_bitmap(sbi, dc->start, dc->start + dc->len); > > f2fs_update_iostat(sbi, FS_DISCARD, 1); > @@ -1054,6 +1056,68 @@ static int __queue_discard_cmd(struct f2fs_sb_info *sbi, > return 0; > } > > +static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi, > + unsigned int start, unsigned int end, > + unsigned int granularity) > +{ > + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info; > + struct discard_cmd *prev_dc = NULL, *next_dc = NULL; > + struct rb_node **insert_p = NULL, *insert_parent = NULL; > + struct discard_cmd *dc; > + struct blk_plug plug; > + int issued; > + > +next: > + issued = 0; > + > + mutex_lock(&dcc->cmd_lock); > + f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, &dcc->root)); > + > + dc = (struct discard_cmd *)__lookup_rb_tree_ret(&dcc->root, > + NULL, start, > + (struct rb_entry **)&prev_dc, > + (struct rb_entry **)&next_dc, > + &insert_p, &insert_parent, true); > + if (!dc) > + dc = next_dc; > + > + blk_start_plug(&plug); > + > + while (dc && dc->lstart <= end) { > + struct rb_node *node; > + > + if (dc->len < granularity) > + goto skip; > + > + if (dc->state != D_PREP) { > + list_move_tail(&dc->list, &dcc->fstrim_list); > + goto skip; > + } > + > + __submit_discard_cmd(sbi, dc, true); > + > + if (++issued >= DISCARD_ISSUE_RATE) { > + start = dc->lstart + dc->len; > + > + blk_finish_plug(&plug); > + mutex_unlock(&dcc->cmd_lock); > + > + schedule(); > + > + goto next; > + } > +skip: > + node = rb_next(&dc->rb_node); > + dc = rb_entry_safe(node, struct discard_cmd, rb_node); > + > + if (fatal_signal_pending(current)) > + break; > + } > + > + blk_finish_plug(&plug); > + mutex_unlock(&dcc->cmd_lock); > +} > + > static int __issue_discard_cmd(struct f2fs_sb_info *sbi, bool issue_cond) > { > struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info; > @@ -1076,22 +1140,19 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi, bool issue_cond) > > /* Hurry up to finish fstrim */ > if (dcc->pend_list_tag[i] & P_TRIM) { > - __submit_discard_cmd(sbi, dc); > + __submit_discard_cmd(sbi, dc, false); > issued++; > - > - if (fatal_signal_pending(current)) > - break; > continue; > } > > if (!issue_cond) { > - __submit_discard_cmd(sbi, dc); > + __submit_discard_cmd(sbi, dc, false); > issued++; > continue; > } > > if (is_idle(sbi)) { > - __submit_discard_cmd(sbi, dc); > + __submit_discard_cmd(sbi, dc, false); > issued++; > } else { > io_interrupted = true; > @@ -1145,10 +1206,14 @@ static void __wait_one_discard_bio(struct f2fs_sb_info *sbi, > mutex_unlock(&dcc->cmd_lock); > } > > -static void __wait_discard_cmd(struct f2fs_sb_info *sbi, bool wait_cond) > +static void __wait_discard_cmd_range(struct f2fs_sb_info *sbi, bool wait_cond, > + block_t start, block_t end, > + unsigned int granularity, > + bool fstrim) > { > struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info; > - struct list_head *wait_list = &(dcc->wait_list); > + struct list_head *wait_list = fstrim ? &(dcc->fstrim_list) : > + &(dcc->wait_list); > struct discard_cmd *dc, *tmp; > bool need_wait; > > @@ -1157,6 +1222,10 @@ static void __wait_discard_cmd(struct f2fs_sb_info *sbi, bool wait_cond) > > mutex_lock(&dcc->cmd_lock); > list_for_each_entry_safe(dc, tmp, wait_list, list) { > + if (dc->lstart + dc->len <= start || end <= dc->lstart) > + continue; > + if (dc->len < granularity) > + continue; > if (!wait_cond || (dc->state == D_DONE && !dc->ref)) { > wait_for_completion_io(&dc->wait); > __remove_discard_cmd(sbi, dc); So, we need to add this on top of the patch that fixes the bug reported by Juhyung, right? if (fstrim) need_wait = true; Thanks, > @@ -1174,6 +1243,11 @@ static void __wait_discard_cmd(struct f2fs_sb_info *sbi, bool wait_cond) > } > } > > +static void __wait_all_discard_cmd(struct f2fs_sb_info *sbi, bool wait_cond) > +{ > + __wait_discard_cmd_range(sbi, wait_cond, 0, UINT_MAX, 1, false); > +} > + > /* This should be covered by global mutex, &sit_i->sentry_lock */ > void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr) > { > @@ -1209,12 +1283,12 @@ void stop_discard_thread(struct f2fs_sb_info *sbi) > } > } > > -/* This comes from f2fs_put_super and f2fs_trim_fs */ > +/* This comes from f2fs_put_super */ > void f2fs_wait_discard_bios(struct f2fs_sb_info *sbi) > { > __issue_discard_cmd(sbi, false); > __drop_discard_cmd(sbi); > - __wait_discard_cmd(sbi, false); > + __wait_all_discard_cmd(sbi, false); > } > > static void mark_discard_range_all(struct f2fs_sb_info *sbi) > @@ -1258,7 +1332,7 @@ static int issue_discard_thread(void *data) > > issued = __issue_discard_cmd(sbi, true); > if (issued) { > - __wait_discard_cmd(sbi, true); > + __wait_all_discard_cmd(sbi, true); > wait_ms = DEF_MIN_DISCARD_ISSUE_TIME; > } else { > wait_ms = DEF_MAX_DISCARD_ISSUE_TIME; > @@ -1569,6 +1643,7 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi) > dcc->pend_list_tag[i] |= P_ACTIVE; > } > INIT_LIST_HEAD(&dcc->wait_list); > + INIT_LIST_HEAD(&dcc->fstrim_list); > mutex_init(&dcc->cmd_lock); > atomic_set(&dcc->issued_discard, 0); > atomic_set(&dcc->issing_discard, 0); > @@ -2196,7 +2271,8 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range) > { > __u64 start = F2FS_BYTES_TO_BLK(range->start); > __u64 end = start + F2FS_BYTES_TO_BLK(range->len) - 1; > - unsigned int start_segno, end_segno; > + unsigned int start_segno, end_segno, cur_segno; > + block_t start_block, end_block; > struct cp_control cpc; > int err = 0; > > @@ -2217,12 +2293,17 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range) > start_segno = (start <= MAIN_BLKADDR(sbi)) ? 0 : GET_SEGNO(sbi, start); > end_segno = (end >= MAX_BLKADDR(sbi)) ? MAIN_SEGS(sbi) - 1 : > GET_SEGNO(sbi, end); > + > + start_block = START_BLOCK(sbi, start_segno); > + end_block = START_BLOCK(sbi, end_segno + 1); > + > cpc.reason = CP_DISCARD; > cpc.trim_minlen = max_t(__u64, 1, F2FS_BYTES_TO_BLK(range->minlen)); > > /* do checkpoint to issue discard commands safely */ > - for (; start_segno <= end_segno; start_segno = cpc.trim_end + 1) { > - cpc.trim_start = start_segno; > + for (cur_segno = start_segno; cur_segno <= end_segno; > + cur_segno = cpc.trim_end + 1) { > + cpc.trim_start = cur_segno; > > if (sbi->discard_blks == 0) > break; > @@ -2230,7 +2311,7 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range) > cpc.trim_end = end_segno; > else > cpc.trim_end = min_t(unsigned int, > - rounddown(start_segno + > + rounddown(cur_segno + > BATCHED_TRIM_SEGMENTS(sbi), > sbi->segs_per_sec) - 1, end_segno); > > @@ -2242,9 +2323,13 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range) > > schedule(); > } > - /* It's time to issue all the filed discards */ > - mark_discard_range_all(sbi); > - f2fs_wait_discard_bios(sbi); > + > + start_block = START_BLOCK(sbi, start_segno); > + end_block = START_BLOCK(sbi, min(cur_segno, end_segno) + 1); > + > + __issue_discard_cmd_range(sbi, start_block, end_block, cpc.trim_minlen); > + __wait_discard_cmd_range(sbi, false, start_block, end_block, > + cpc.trim_minlen, true); > out: > range->len = F2FS_BLK_TO_BYTES(cpc.trimmed); > return err; > -- > 2.14.1.145.gb3622a4ee