[PATCH 1/6] f2fs: support issuing/waiting discard in range

[PATCH 1/6] f2fs: support issuing/waiting discard in range

[Chao Yu]

From: Chao Yu <yuchao0@huawei.com>

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 <yuchao0@huawei.com>
---
 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);
@@ -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

[PATCH 2/6] f2fs: wrap discard policy

[Chao Yu]

From: Chao Yu <yuchao0@huawei.com>

This patch wraps scattered optional parameters into discard policy as
below, later, with it we expect that we can adjust these parameters with
proper strategy in different scenario.

struct discard_policy {
	unsigned int min_interval;	/* used for candidates exist */
	unsigned int max_interval;	/* used for candidates not exist */
	unsigned int max_requests;	/* # of discards issued per round */
	unsigned int io_aware_gran;	/* minimum granularity discard not be aware of I/O */
	bool io_aware;			/* issue discard in idle time */
	bool sync;			/* submit discard with REQ_SYNC flag */
};

This patch doesn't change any logic of codes.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/f2fs.h    | 12 +++++++++++-
 fs/f2fs/segment.c | 38 +++++++++++++++++++++++++++++---------
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index cb13c7df6971..b119104e75bd 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -149,7 +149,7 @@ enum {
 #define BATCHED_TRIM_BLOCKS(sbi)	\
 		(BATCHED_TRIM_SEGMENTS(sbi) << (sbi)->log_blocks_per_seg)
 #define MAX_DISCARD_BLOCKS(sbi)		BLKS_PER_SEC(sbi)
-#define DISCARD_ISSUE_RATE		8
+#define DEF_MAX_DISCARD_REQUEST		8	/* issue 8 discards per round */
 #define DEF_MIN_DISCARD_ISSUE_TIME	50	/* 50 ms, if exists */
 #define DEF_MAX_DISCARD_ISSUE_TIME	60000	/* 60 s, if no candidates */
 #define DEF_CP_INTERVAL			60	/* 60 secs */
@@ -243,6 +243,15 @@ struct discard_cmd {
 	int error;			/* bio error */
 };
 
+struct discard_policy {
+	unsigned int min_interval;	/* used for candidates exist */
+	unsigned int max_interval;	/* used for candidates not exist */
+	unsigned int max_requests;	/* # of discards issued per round */
+	unsigned int io_aware_gran;	/* minimum granularity discard not be aware of I/O */
+	bool io_aware;			/* issue discard in idle time */
+	bool sync;			/* submit discard with REQ_SYNC flag */
+};
+
 struct discard_cmd_control {
 	struct task_struct *f2fs_issue_discard;	/* discard thread */
 	struct list_head entry_list;		/* 4KB discard entry list */
@@ -261,6 +270,7 @@ struct discard_cmd_control {
 	atomic_t issing_discard;		/* # of issing discard */
 	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
 	struct rb_root root;			/* root of discard rb-tree */
+	struct discard_policy dpolicy;		/* current discard policy */
 };
 
 /* for the list of fsync inodes, used only during recovery */
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 088936c61905..b3787ba1108f 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -851,6 +851,7 @@ static void __submit_discard_cmd(struct f2fs_sb_info *sbi,
 	struct list_head *wait_list = fstrim ? &(dcc->fstrim_list) :
 							&(dcc->wait_list);
 	struct bio *bio = NULL;
+	int flag = dcc->dpolicy.sync ? REQ_SYNC : 0;
 
 	if (dc->state != D_PREP)
 		return;
@@ -869,7 +870,7 @@ static void __submit_discard_cmd(struct f2fs_sb_info *sbi,
 		if (bio) {
 			bio->bi_private = dc;
 			bio->bi_end_io = f2fs_submit_discard_endio;
-			bio->bi_opf |= REQ_SYNC;
+			bio->bi_opf |= flag;
 			submit_bio(bio);
 			list_move_tail(&dc->list, wait_list);
 			__check_sit_bitmap(sbi, dc->start, dc->start + dc->len);
@@ -1064,6 +1065,7 @@ static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
 	struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
 	struct rb_node **insert_p = NULL, *insert_parent = NULL;
 	struct discard_cmd *dc;
+	struct discard_policy *dpolicy = &dcc->dpolicy;
 	struct blk_plug plug;
 	int issued;
 
@@ -1096,7 +1098,7 @@ static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
 
 		__submit_discard_cmd(sbi, dc, true);
 
-		if (++issued >= DISCARD_ISSUE_RATE) {
+		if (++issued >= dpolicy->max_requests) {
 			start = dc->lstart + dc->len;
 
 			blk_finish_plug(&plug);
@@ -1124,6 +1126,7 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi, bool issue_cond)
 	struct list_head *pend_list;
 	struct discard_cmd *dc, *tmp;
 	struct blk_plug plug;
+	struct discard_policy *dpolicy = &dcc->dpolicy;
 	int iter = 0, issued = 0;
 	int i;
 	bool io_interrupted = false;
@@ -1151,14 +1154,16 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi, bool issue_cond)
 				continue;
 			}
 
-			if (is_idle(sbi)) {
-				__submit_discard_cmd(sbi, dc, false);
-				issued++;
-			} else {
+			if (dpolicy->io_aware && i < dpolicy->io_aware_gran &&
+								!is_idle(sbi)) {
 				io_interrupted = true;
+				goto skip;
 			}
 
-			if (++iter >= DISCARD_ISSUE_RATE)
+			__submit_discard_cmd(sbi, dc, false);
+			issued++;
+skip:
+			if (++iter >= dpolicy->max_requests)
 				goto out;
 		}
 		if (list_empty(pend_list) && dcc->pend_list_tag[i] & P_TRIM)
@@ -1307,6 +1312,7 @@ static int issue_discard_thread(void *data)
 	struct f2fs_sb_info *sbi = data;
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
 	wait_queue_head_t *q = &dcc->discard_wait_queue;
+	struct discard_policy *dpolicy = &dcc->dpolicy;
 	unsigned int wait_ms = DEF_MIN_DISCARD_ISSUE_TIME;
 	int issued;
 
@@ -1333,9 +1339,9 @@ static int issue_discard_thread(void *data)
 		issued = __issue_discard_cmd(sbi, true);
 		if (issued) {
 			__wait_all_discard_cmd(sbi, true);
-			wait_ms = DEF_MIN_DISCARD_ISSUE_TIME;
+			wait_ms = dpolicy->min_interval;
 		} else {
-			wait_ms = DEF_MAX_DISCARD_ISSUE_TIME;
+			wait_ms = dpolicy->max_interval;
 		}
 
 		sb_end_intwrite(sbi->sb);
@@ -1620,6 +1626,18 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 	wake_up_discard_thread(sbi, false);
 }
 
+static void inline init_discard_policy(struct discard_cmd_control *dcc)
+{
+	struct discard_policy *dpolicy = &dcc->dpolicy;
+
+	dpolicy->min_interval = DEF_MIN_DISCARD_ISSUE_TIME;
+	dpolicy->max_interval = DEF_MAX_DISCARD_ISSUE_TIME;
+	dpolicy->max_requests = DEF_MAX_DISCARD_REQUEST;
+	dpolicy->io_aware_gran = MAX_PLIST_NUM;
+	dpolicy->io_aware = true;
+	dpolicy->sync = true;
+}
+
 static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
 {
 	dev_t dev = sbi->sb->s_bdev->bd_dev;
@@ -1653,6 +1671,8 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
 	dcc->undiscard_blks = 0;
 	dcc->root = RB_ROOT;
 
+	init_discard_policy(dcc);
+
 	init_waitqueue_head(&dcc->discard_wait_queue);
 	SM_I(sbi)->dcc_info = dcc;
 init_thread:
-- 
2.14.1.145.gb3622a4ee

Re: [PATCH 2/6] f2fs: wrap discard policy

[Chao Yu]

Hi Jaegeuk,

Any comments on other patches in this patchset?

Thanks,

On 2017/9/19 23:30, Chao Yu wrote:
> From: Chao Yu <yuchao0@huawei.com>
> 
> This patch wraps scattered optional parameters into discard policy as
> below, later, with it we expect that we can adjust these parameters with
> proper strategy in different scenario.
> 
> struct discard_policy {
> 	unsigned int min_interval;	/* used for candidates exist */
> 	unsigned int max_interval;	/* used for candidates not exist */
> 	unsigned int max_requests;	/* # of discards issued per round */
> 	unsigned int io_aware_gran;	/* minimum granularity discard not be aware of I/O */
> 	bool io_aware;			/* issue discard in idle time */
> 	bool sync;			/* submit discard with REQ_SYNC flag */
> };
> 
> This patch doesn't change any logic of codes.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/f2fs.h    | 12 +++++++++++-
>  fs/f2fs/segment.c | 38 +++++++++++++++++++++++++++++---------
>  2 files changed, 40 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index cb13c7df6971..b119104e75bd 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -149,7 +149,7 @@ enum {
>  #define BATCHED_TRIM_BLOCKS(sbi)	\
>  		(BATCHED_TRIM_SEGMENTS(sbi) << (sbi)->log_blocks_per_seg)
>  #define MAX_DISCARD_BLOCKS(sbi)		BLKS_PER_SEC(sbi)
> -#define DISCARD_ISSUE_RATE		8
> +#define DEF_MAX_DISCARD_REQUEST		8	/* issue 8 discards per round */
>  #define DEF_MIN_DISCARD_ISSUE_TIME	50	/* 50 ms, if exists */
>  #define DEF_MAX_DISCARD_ISSUE_TIME	60000	/* 60 s, if no candidates */
>  #define DEF_CP_INTERVAL			60	/* 60 secs */
> @@ -243,6 +243,15 @@ struct discard_cmd {
>  	int error;			/* bio error */
>  };
>  
> +struct discard_policy {
> +	unsigned int min_interval;	/* used for candidates exist */
> +	unsigned int max_interval;	/* used for candidates not exist */
> +	unsigned int max_requests;	/* # of discards issued per round */
> +	unsigned int io_aware_gran;	/* minimum granularity discard not be aware of I/O */
> +	bool io_aware;			/* issue discard in idle time */
> +	bool sync;			/* submit discard with REQ_SYNC flag */
> +};
> +
>  struct discard_cmd_control {
>  	struct task_struct *f2fs_issue_discard;	/* discard thread */
>  	struct list_head entry_list;		/* 4KB discard entry list */
> @@ -261,6 +270,7 @@ struct discard_cmd_control {
>  	atomic_t issing_discard;		/* # of issing discard */
>  	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
>  	struct rb_root root;			/* root of discard rb-tree */
> +	struct discard_policy dpolicy;		/* current discard policy */
>  };
>  
>  /* for the list of fsync inodes, used only during recovery */
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 088936c61905..b3787ba1108f 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -851,6 +851,7 @@ static void __submit_discard_cmd(struct f2fs_sb_info *sbi,
>  	struct list_head *wait_list = fstrim ? &(dcc->fstrim_list) :
>  							&(dcc->wait_list);
>  	struct bio *bio = NULL;
> +	int flag = dcc->dpolicy.sync ? REQ_SYNC : 0;
>  
>  	if (dc->state != D_PREP)
>  		return;
> @@ -869,7 +870,7 @@ static void __submit_discard_cmd(struct f2fs_sb_info *sbi,
>  		if (bio) {
>  			bio->bi_private = dc;
>  			bio->bi_end_io = f2fs_submit_discard_endio;
> -			bio->bi_opf |= REQ_SYNC;
> +			bio->bi_opf |= flag;
>  			submit_bio(bio);
>  			list_move_tail(&dc->list, wait_list);
>  			__check_sit_bitmap(sbi, dc->start, dc->start + dc->len);
> @@ -1064,6 +1065,7 @@ static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
>  	struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
>  	struct rb_node **insert_p = NULL, *insert_parent = NULL;
>  	struct discard_cmd *dc;
> +	struct discard_policy *dpolicy = &dcc->dpolicy;
>  	struct blk_plug plug;
>  	int issued;
>  
> @@ -1096,7 +1098,7 @@ static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
>  
>  		__submit_discard_cmd(sbi, dc, true);
>  
> -		if (++issued >= DISCARD_ISSUE_RATE) {
> +		if (++issued >= dpolicy->max_requests) {
>  			start = dc->lstart + dc->len;
>  
>  			blk_finish_plug(&plug);
> @@ -1124,6 +1126,7 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi, bool issue_cond)
>  	struct list_head *pend_list;
>  	struct discard_cmd *dc, *tmp;
>  	struct blk_plug plug;
> +	struct discard_policy *dpolicy = &dcc->dpolicy;
>  	int iter = 0, issued = 0;
>  	int i;
>  	bool io_interrupted = false;
> @@ -1151,14 +1154,16 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi, bool issue_cond)
>  				continue;
>  			}
>  
> -			if (is_idle(sbi)) {
> -				__submit_discard_cmd(sbi, dc, false);
> -				issued++;
> -			} else {
> +			if (dpolicy->io_aware && i < dpolicy->io_aware_gran &&
> +								!is_idle(sbi)) {
>  				io_interrupted = true;
> +				goto skip;
>  			}
>  
> -			if (++iter >= DISCARD_ISSUE_RATE)
> +			__submit_discard_cmd(sbi, dc, false);
> +			issued++;
> +skip:
> +			if (++iter >= dpolicy->max_requests)
>  				goto out;
>  		}
>  		if (list_empty(pend_list) && dcc->pend_list_tag[i] & P_TRIM)
> @@ -1307,6 +1312,7 @@ static int issue_discard_thread(void *data)
>  	struct f2fs_sb_info *sbi = data;
>  	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
>  	wait_queue_head_t *q = &dcc->discard_wait_queue;
> +	struct discard_policy *dpolicy = &dcc->dpolicy;
>  	unsigned int wait_ms = DEF_MIN_DISCARD_ISSUE_TIME;
>  	int issued;
>  
> @@ -1333,9 +1339,9 @@ static int issue_discard_thread(void *data)
>  		issued = __issue_discard_cmd(sbi, true);
>  		if (issued) {
>  			__wait_all_discard_cmd(sbi, true);
> -			wait_ms = DEF_MIN_DISCARD_ISSUE_TIME;
> +			wait_ms = dpolicy->min_interval;
>  		} else {
> -			wait_ms = DEF_MAX_DISCARD_ISSUE_TIME;
> +			wait_ms = dpolicy->max_interval;
>  		}
>  
>  		sb_end_intwrite(sbi->sb);
> @@ -1620,6 +1626,18 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  	wake_up_discard_thread(sbi, false);
>  }
>  
> +static void inline init_discard_policy(struct discard_cmd_control *dcc)
> +{
> +	struct discard_policy *dpolicy = &dcc->dpolicy;
> +
> +	dpolicy->min_interval = DEF_MIN_DISCARD_ISSUE_TIME;
> +	dpolicy->max_interval = DEF_MAX_DISCARD_ISSUE_TIME;
> +	dpolicy->max_requests = DEF_MAX_DISCARD_REQUEST;
> +	dpolicy->io_aware_gran = MAX_PLIST_NUM;
> +	dpolicy->io_aware = true;
> +	dpolicy->sync = true;
> +}
> +
>  static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
>  {
>  	dev_t dev = sbi->sb->s_bdev->bd_dev;
> @@ -1653,6 +1671,8 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
>  	dcc->undiscard_blks = 0;
>  	dcc->root = RB_ROOT;
>  
> +	init_discard_policy(dcc);
> +
>  	init_waitqueue_head(&dcc->discard_wait_queue);
>  	SM_I(sbi)->dcc_info = dcc;
>  init_thread:
> 

Re: [PATCH 2/6] f2fs: wrap discard policy

[Chao Yu]

Hi Jaegeuk,

Any comments on other patches in this patchset?

Thanks,

On 2017/9/19 23:30, Chao Yu wrote:
> From: Chao Yu <yuchao0@huawei.com>
> 
> This patch wraps scattered optional parameters into discard policy as
> below, later, with it we expect that we can adjust these parameters with
> proper strategy in different scenario.
> 
> struct discard_policy {
> 	unsigned int min_interval;	/* used for candidates exist */
> 	unsigned int max_interval;	/* used for candidates not exist */
> 	unsigned int max_requests;	/* # of discards issued per round */
> 	unsigned int io_aware_gran;	/* minimum granularity discard not be aware of I/O */
> 	bool io_aware;			/* issue discard in idle time */
> 	bool sync;			/* submit discard with REQ_SYNC flag */
> };
> 
> This patch doesn't change any logic of codes.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/f2fs.h    | 12 +++++++++++-
>  fs/f2fs/segment.c | 38 +++++++++++++++++++++++++++++---------
>  2 files changed, 40 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index cb13c7df6971..b119104e75bd 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -149,7 +149,7 @@ enum {
>  #define BATCHED_TRIM_BLOCKS(sbi)	\
>  		(BATCHED_TRIM_SEGMENTS(sbi) << (sbi)->log_blocks_per_seg)
>  #define MAX_DISCARD_BLOCKS(sbi)		BLKS_PER_SEC(sbi)
> -#define DISCARD_ISSUE_RATE		8
> +#define DEF_MAX_DISCARD_REQUEST		8	/* issue 8 discards per round */
>  #define DEF_MIN_DISCARD_ISSUE_TIME	50	/* 50 ms, if exists */
>  #define DEF_MAX_DISCARD_ISSUE_TIME	60000	/* 60 s, if no candidates */
>  #define DEF_CP_INTERVAL			60	/* 60 secs */
> @@ -243,6 +243,15 @@ struct discard_cmd {
>  	int error;			/* bio error */
>  };
>  
> +struct discard_policy {
> +	unsigned int min_interval;	/* used for candidates exist */
> +	unsigned int max_interval;	/* used for candidates not exist */
> +	unsigned int max_requests;	/* # of discards issued per round */
> +	unsigned int io_aware_gran;	/* minimum granularity discard not be aware of I/O */
> +	bool io_aware;			/* issue discard in idle time */
> +	bool sync;			/* submit discard with REQ_SYNC flag */
> +};
> +
>  struct discard_cmd_control {
>  	struct task_struct *f2fs_issue_discard;	/* discard thread */
>  	struct list_head entry_list;		/* 4KB discard entry list */
> @@ -261,6 +270,7 @@ struct discard_cmd_control {
>  	atomic_t issing_discard;		/* # of issing discard */
>  	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
>  	struct rb_root root;			/* root of discard rb-tree */
> +	struct discard_policy dpolicy;		/* current discard policy */
>  };
>  
>  /* for the list of fsync inodes, used only during recovery */
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 088936c61905..b3787ba1108f 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -851,6 +851,7 @@ static void __submit_discard_cmd(struct f2fs_sb_info *sbi,
>  	struct list_head *wait_list = fstrim ? &(dcc->fstrim_list) :
>  							&(dcc->wait_list);
>  	struct bio *bio = NULL;
> +	int flag = dcc->dpolicy.sync ? REQ_SYNC : 0;
>  
>  	if (dc->state != D_PREP)
>  		return;
> @@ -869,7 +870,7 @@ static void __submit_discard_cmd(struct f2fs_sb_info *sbi,
>  		if (bio) {
>  			bio->bi_private = dc;
>  			bio->bi_end_io = f2fs_submit_discard_endio;
> -			bio->bi_opf |= REQ_SYNC;
> +			bio->bi_opf |= flag;
>  			submit_bio(bio);
>  			list_move_tail(&dc->list, wait_list);
>  			__check_sit_bitmap(sbi, dc->start, dc->start + dc->len);
> @@ -1064,6 +1065,7 @@ static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
>  	struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
>  	struct rb_node **insert_p = NULL, *insert_parent = NULL;
>  	struct discard_cmd *dc;
> +	struct discard_policy *dpolicy = &dcc->dpolicy;
>  	struct blk_plug plug;
>  	int issued;
>  
> @@ -1096,7 +1098,7 @@ static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
>  
>  		__submit_discard_cmd(sbi, dc, true);
>  
> -		if (++issued >= DISCARD_ISSUE_RATE) {
> +		if (++issued >= dpolicy->max_requests) {
>  			start = dc->lstart + dc->len;
>  
>  			blk_finish_plug(&plug);
> @@ -1124,6 +1126,7 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi, bool issue_cond)
>  	struct list_head *pend_list;
>  	struct discard_cmd *dc, *tmp;
>  	struct blk_plug plug;
> +	struct discard_policy *dpolicy = &dcc->dpolicy;
>  	int iter = 0, issued = 0;
>  	int i;
>  	bool io_interrupted = false;
> @@ -1151,14 +1154,16 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi, bool issue_cond)
>  				continue;
>  			}
>  
> -			if (is_idle(sbi)) {
> -				__submit_discard_cmd(sbi, dc, false);
> -				issued++;
> -			} else {
> +			if (dpolicy->io_aware && i < dpolicy->io_aware_gran &&
> +								!is_idle(sbi)) {
>  				io_interrupted = true;
> +				goto skip;
>  			}
>  
> -			if (++iter >= DISCARD_ISSUE_RATE)
> +			__submit_discard_cmd(sbi, dc, false);
> +			issued++;
> +skip:
> +			if (++iter >= dpolicy->max_requests)
>  				goto out;
>  		}
>  		if (list_empty(pend_list) && dcc->pend_list_tag[i] & P_TRIM)
> @@ -1307,6 +1312,7 @@ static int issue_discard_thread(void *data)
>  	struct f2fs_sb_info *sbi = data;
>  	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
>  	wait_queue_head_t *q = &dcc->discard_wait_queue;
> +	struct discard_policy *dpolicy = &dcc->dpolicy;
>  	unsigned int wait_ms = DEF_MIN_DISCARD_ISSUE_TIME;
>  	int issued;
>  
> @@ -1333,9 +1339,9 @@ static int issue_discard_thread(void *data)
>  		issued = __issue_discard_cmd(sbi, true);
>  		if (issued) {
>  			__wait_all_discard_cmd(sbi, true);
> -			wait_ms = DEF_MIN_DISCARD_ISSUE_TIME;
> +			wait_ms = dpolicy->min_interval;
>  		} else {
> -			wait_ms = DEF_MAX_DISCARD_ISSUE_TIME;
> +			wait_ms = dpolicy->max_interval;
>  		}
>  
>  		sb_end_intwrite(sbi->sb);
> @@ -1620,6 +1626,18 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  	wake_up_discard_thread(sbi, false);
>  }
>  
> +static void inline init_discard_policy(struct discard_cmd_control *dcc)
> +{
> +	struct discard_policy *dpolicy = &dcc->dpolicy;
> +
> +	dpolicy->min_interval = DEF_MIN_DISCARD_ISSUE_TIME;
> +	dpolicy->max_interval = DEF_MAX_DISCARD_ISSUE_TIME;
> +	dpolicy->max_requests = DEF_MAX_DISCARD_REQUEST;
> +	dpolicy->io_aware_gran = MAX_PLIST_NUM;
> +	dpolicy->io_aware = true;
> +	dpolicy->sync = true;
> +}
> +
>  static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
>  {
>  	dev_t dev = sbi->sb->s_bdev->bd_dev;
> @@ -1653,6 +1671,8 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
>  	dcc->undiscard_blks = 0;
>  	dcc->root = RB_ROOT;
>  
> +	init_discard_policy(dcc);
> +
>  	init_waitqueue_head(&dcc->discard_wait_queue);
>  	SM_I(sbi)->dcc_info = dcc;
>  init_thread:
> 

[PATCH 3/6] f2fs: split discard policy

[Chao Yu]

From: Chao Yu <yuchao0@huawei.com>

There are many different scenarios such as fstrim, umount, urgent or
background where we will issue discards, actually, they need use
different policy in aspect of io aware, discard granularity, delay
interval and so on. But now they just share one common discard policy,
so there will be race when changing policy in between these scenarios,
the interference of changing discard policy will be very serious.

This patch changes to split discard policy for different scenarios.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/f2fs.h    |  17 +++++--
 fs/f2fs/segment.c | 149 ++++++++++++++++++++++++++----------------------------
 fs/f2fs/segment.h |   5 +-
 fs/f2fs/sysfs.c   |  13 -----
 4 files changed, 88 insertions(+), 96 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index b119104e75bd..e6f88171cbc9 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -208,10 +208,6 @@ struct discard_entry {
 #define plist_idx(blk_num)	((blk_num) >= MAX_PLIST_NUM ?		\
 					(MAX_PLIST_NUM - 1) : (blk_num - 1))
 
-#define P_ACTIVE	0x01
-#define P_TRIM		0x02
-#define plist_issue(tag)	(((tag) & P_ACTIVE) || ((tag) & P_TRIM))
-
 enum {
 	D_PREP,
 	D_SUBMIT,
@@ -243,13 +239,23 @@ struct discard_cmd {
 	int error;			/* bio error */
 };
 
+enum {
+	DPOLICY_BG,
+	DPOLICY_FORCE,
+	DPOLICY_FSTRIM,
+	DPOLICY_UMOUNT,
+	MAX_DPOLICY,
+};
+
 struct discard_policy {
+	int type;			/* type of discard */
 	unsigned int min_interval;	/* used for candidates exist */
 	unsigned int max_interval;	/* used for candidates not exist */
 	unsigned int max_requests;	/* # of discards issued per round */
 	unsigned int io_aware_gran;	/* minimum granularity discard not be aware of I/O */
 	bool io_aware;			/* issue discard in idle time */
 	bool sync;			/* submit discard with REQ_SYNC flag */
+	unsigned int granularity;	/* discard granularity */
 };
 
 struct discard_cmd_control {
@@ -270,7 +276,6 @@ struct discard_cmd_control {
 	atomic_t issing_discard;		/* # of issing discard */
 	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
 	struct rb_root root;			/* root of discard rb-tree */
-	struct discard_policy dpolicy;		/* current discard policy */
 };
 
 /* for the list of fsync inodes, used only during recovery */
@@ -2554,6 +2559,8 @@ int create_flush_cmd_control(struct f2fs_sb_info *sbi);
 void destroy_flush_cmd_control(struct f2fs_sb_info *sbi, bool free);
 void invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr);
 bool is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr);
+void init_discard_policy(struct discard_policy *dpolicy, int discard_type,
+						unsigned int granularity);
 void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new);
 void stop_discard_thread(struct f2fs_sb_info *sbi);
 void f2fs_wait_discard_bios(struct f2fs_sb_info *sbi);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index b3787ba1108f..c3eb355b0171 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -845,13 +845,14 @@ 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, bool fstrim)
+						struct discard_policy *dpolicy,
+						struct discard_cmd *dc)
 {
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
-	struct list_head *wait_list = fstrim ? &(dcc->fstrim_list) :
-							&(dcc->wait_list);
+	struct list_head *wait_list = (dpolicy->type == DPOLICY_FSTRIM) ?
+					&(dcc->fstrim_list) : &(dcc->wait_list);
 	struct bio *bio = NULL;
-	int flag = dcc->dpolicy.sync ? REQ_SYNC : 0;
+	int flag = dpolicy->sync ? REQ_SYNC : 0;
 
 	if (dc->state != D_PREP)
 		return;
@@ -1058,14 +1059,13 @@ static int __queue_discard_cmd(struct f2fs_sb_info *sbi,
 }
 
 static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
-					unsigned int start, unsigned int end,
-					unsigned int granularity)
+					struct discard_policy *dpolicy,
+					unsigned int start, unsigned int end)
 {
 	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 discard_policy *dpolicy = &dcc->dpolicy;
 	struct blk_plug plug;
 	int issued;
 
@@ -1088,7 +1088,7 @@ static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
 	while (dc && dc->lstart <= end) {
 		struct rb_node *node;
 
-		if (dc->len < granularity)
+		if (dc->len < dpolicy->granularity)
 			goto skip;
 
 		if (dc->state != D_PREP) {
@@ -1096,7 +1096,7 @@ static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
 			goto skip;
 		}
 
-		__submit_discard_cmd(sbi, dc, true);
+		__submit_discard_cmd(sbi, dpolicy, dc);
 
 		if (++issued >= dpolicy->max_requests) {
 			start = dc->lstart + dc->len;
@@ -1120,54 +1120,39 @@ static void __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
 	mutex_unlock(&dcc->cmd_lock);
 }
 
-static int __issue_discard_cmd(struct f2fs_sb_info *sbi, bool issue_cond)
+static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
+					struct discard_policy *dpolicy)
 {
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
 	struct list_head *pend_list;
 	struct discard_cmd *dc, *tmp;
 	struct blk_plug plug;
-	struct discard_policy *dpolicy = &dcc->dpolicy;
-	int iter = 0, issued = 0;
-	int i;
+	int i, iter = 0, issued = 0;
 	bool io_interrupted = false;
 
 	mutex_lock(&dcc->cmd_lock);
 	f2fs_bug_on(sbi,
 		!__check_rb_tree_consistence(sbi, &dcc->root));
 	blk_start_plug(&plug);
-	for (i = MAX_PLIST_NUM - 1;
-			i >= 0 && plist_issue(dcc->pend_list_tag[i]); i--) {
+	for (i = MAX_PLIST_NUM - 1; i >= 0; i--) {
+		if (i + 1 < dpolicy->granularity)
+			break;
 		pend_list = &dcc->pend_list[i];
 		list_for_each_entry_safe(dc, tmp, pend_list, list) {
 			f2fs_bug_on(sbi, dc->state != D_PREP);
 
-			/* Hurry up to finish fstrim */
-			if (dcc->pend_list_tag[i] & P_TRIM) {
-				__submit_discard_cmd(sbi, dc, false);
-				issued++;
-				continue;
-			}
-
-			if (!issue_cond) {
-				__submit_discard_cmd(sbi, dc, false);
-				issued++;
-				continue;
-			}
-
 			if (dpolicy->io_aware && i < dpolicy->io_aware_gran &&
 								!is_idle(sbi)) {
 				io_interrupted = true;
 				goto skip;
 			}
 
-			__submit_discard_cmd(sbi, dc, false);
+			__submit_discard_cmd(sbi, dpolicy, dc);
 			issued++;
 skip:
 			if (++iter >= dpolicy->max_requests)
 				goto out;
 		}
-		if (list_empty(pend_list) && dcc->pend_list_tag[i] & P_TRIM)
-			dcc->pend_list_tag[i] &= (~P_TRIM);
 	}
 out:
 	blk_finish_plug(&plug);
@@ -1211,14 +1196,13 @@ static void __wait_one_discard_bio(struct f2fs_sb_info *sbi,
 	mutex_unlock(&dcc->cmd_lock);
 }
 
-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)
+static void __wait_discard_cmd_range(struct f2fs_sb_info *sbi,
+						struct discard_policy *dpolicy,
+						block_t start, block_t end)
 {
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
-	struct list_head *wait_list = fstrim ? &(dcc->fstrim_list) :
-							&(dcc->wait_list);
+	struct list_head *wait_list = (dpolicy->type == DPOLICY_FSTRIM) ?
+					&(dcc->fstrim_list) : &(dcc->wait_list);
 	struct discard_cmd *dc, *tmp;
 	bool need_wait;
 
@@ -1229,9 +1213,9 @@ static void __wait_discard_cmd_range(struct f2fs_sb_info *sbi, bool wait_cond,
 	list_for_each_entry_safe(dc, tmp, wait_list, list) {
 		if (dc->lstart + dc->len <= start || end <= dc->lstart)
 			continue;
-		if (dc->len < granularity)
+		if (dc->len < dpolicy->granularity)
 			continue;
-		if (!wait_cond || (dc->state == D_DONE && !dc->ref)) {
+		if (dc->state == D_DONE && !dc->ref) {
 			wait_for_completion_io(&dc->wait);
 			__remove_discard_cmd(sbi, dc);
 		} else {
@@ -1248,9 +1232,10 @@ static void __wait_discard_cmd_range(struct f2fs_sb_info *sbi, bool wait_cond,
 	}
 }
 
-static void __wait_all_discard_cmd(struct f2fs_sb_info *sbi, bool wait_cond)
+static void __wait_all_discard_cmd(struct f2fs_sb_info *sbi,
+						struct discard_policy *dpolicy)
 {
-	__wait_discard_cmd_range(sbi, wait_cond, 0, UINT_MAX, 1, false);
+	__wait_discard_cmd_range(sbi, dpolicy, 0, UINT_MAX);
 }
 
 /* This should be covered by global mutex, &sit_i->sentry_lock */
@@ -1290,21 +1275,14 @@ void stop_discard_thread(struct f2fs_sb_info *sbi)
 
 /* 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_all_discard_cmd(sbi, false);
-}
-
-static void mark_discard_range_all(struct f2fs_sb_info *sbi)
 {
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
-	int i;
+	struct discard_policy dpolicy;
 
-	mutex_lock(&dcc->cmd_lock);
-	for (i = 0; i < MAX_PLIST_NUM; i++)
-		dcc->pend_list_tag[i] |= P_TRIM;
-	mutex_unlock(&dcc->cmd_lock);
+	init_discard_policy(&dpolicy, DPOLICY_UMOUNT, dcc->discard_granularity);
+	__issue_discard_cmd(sbi, &dpolicy);
+	__drop_discard_cmd(sbi);
+	__wait_all_discard_cmd(sbi, &dpolicy);
 }
 
 static int issue_discard_thread(void *data)
@@ -1312,13 +1290,16 @@ static int issue_discard_thread(void *data)
 	struct f2fs_sb_info *sbi = data;
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
 	wait_queue_head_t *q = &dcc->discard_wait_queue;
-	struct discard_policy *dpolicy = &dcc->dpolicy;
+	struct discard_policy dpolicy;
 	unsigned int wait_ms = DEF_MIN_DISCARD_ISSUE_TIME;
 	int issued;
 
 	set_freezable();
 
 	do {
+		init_discard_policy(&dpolicy, DPOLICY_BG,
+					dcc->discard_granularity);
+
 		wait_event_interruptible_timeout(*q,
 				kthread_should_stop() || freezing(current) ||
 				dcc->discard_wake,
@@ -1331,17 +1312,18 @@ static int issue_discard_thread(void *data)
 		if (dcc->discard_wake) {
 			dcc->discard_wake = 0;
 			if (sbi->gc_thread && sbi->gc_thread->gc_urgent)
-				mark_discard_range_all(sbi);
+				init_discard_policy(&dpolicy,
+							DPOLICY_FORCE, 1);
 		}
 
 		sb_start_intwrite(sbi->sb);
 
-		issued = __issue_discard_cmd(sbi, true);
+		issued = __issue_discard_cmd(sbi, &dpolicy);
 		if (issued) {
-			__wait_all_discard_cmd(sbi, true);
-			wait_ms = dpolicy->min_interval;
+			__wait_all_discard_cmd(sbi, &dpolicy);
+			wait_ms = dpolicy.min_interval;
 		} else {
-			wait_ms = dpolicy->max_interval;
+			wait_ms = dpolicy.max_interval;
 		}
 
 		sb_end_intwrite(sbi->sb);
@@ -1626,16 +1608,35 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 	wake_up_discard_thread(sbi, false);
 }
 
-static void inline init_discard_policy(struct discard_cmd_control *dcc)
+void init_discard_policy(struct discard_policy *dpolicy,
+				int discard_type, unsigned int granularity)
 {
-	struct discard_policy *dpolicy = &dcc->dpolicy;
-
-	dpolicy->min_interval = DEF_MIN_DISCARD_ISSUE_TIME;
-	dpolicy->max_interval = DEF_MAX_DISCARD_ISSUE_TIME;
-	dpolicy->max_requests = DEF_MAX_DISCARD_REQUEST;
-	dpolicy->io_aware_gran = MAX_PLIST_NUM;
-	dpolicy->io_aware = true;
+	/* common policy */
+	dpolicy->type = discard_type;
 	dpolicy->sync = true;
+	dpolicy->granularity = granularity;
+
+	if (discard_type == DPOLICY_BG) {
+		dpolicy->min_interval = DEF_MIN_DISCARD_ISSUE_TIME;
+		dpolicy->max_interval = DEF_MAX_DISCARD_ISSUE_TIME;
+		dpolicy->max_requests = DEF_MAX_DISCARD_REQUEST;
+		dpolicy->io_aware_gran = MAX_PLIST_NUM;
+		dpolicy->io_aware = true;
+	} else if (discard_type == DPOLICY_FORCE) {
+		dpolicy->min_interval = DEF_MIN_DISCARD_ISSUE_TIME;
+		dpolicy->max_interval = DEF_MAX_DISCARD_ISSUE_TIME;
+		dpolicy->max_requests = DEF_MAX_DISCARD_REQUEST;
+		dpolicy->io_aware_gran = MAX_PLIST_NUM;
+		dpolicy->io_aware = false;
+	} else if (discard_type == DPOLICY_FSTRIM) {
+		dpolicy->max_requests = DEF_MAX_DISCARD_REQUEST;
+		dpolicy->io_aware_gran = MAX_PLIST_NUM;
+		dpolicy->io_aware = false;
+	} else if (discard_type == DPOLICY_UMOUNT) {
+		dpolicy->max_requests = DEF_MAX_DISCARD_REQUEST;
+		dpolicy->io_aware_gran = MAX_PLIST_NUM;
+		dpolicy->io_aware = false;
+	}
 }
 
 static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
@@ -1655,11 +1656,8 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
 
 	dcc->discard_granularity = DEFAULT_DISCARD_GRANULARITY;
 	INIT_LIST_HEAD(&dcc->entry_list);
-	for (i = 0; i < MAX_PLIST_NUM; i++) {
+	for (i = 0; i < MAX_PLIST_NUM; i++)
 		INIT_LIST_HEAD(&dcc->pend_list[i]);
-		if (i >= dcc->discard_granularity - 1)
-			dcc->pend_list_tag[i] |= P_ACTIVE;
-	}
 	INIT_LIST_HEAD(&dcc->wait_list);
 	INIT_LIST_HEAD(&dcc->fstrim_list);
 	mutex_init(&dcc->cmd_lock);
@@ -1671,8 +1669,6 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
 	dcc->undiscard_blks = 0;
 	dcc->root = RB_ROOT;
 
-	init_discard_policy(dcc);
-
 	init_waitqueue_head(&dcc->discard_wait_queue);
 	SM_I(sbi)->dcc_info = dcc;
 init_thread:
@@ -2294,6 +2290,7 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
 	unsigned int start_segno, end_segno, cur_segno;
 	block_t start_block, end_block;
 	struct cp_control cpc;
+	struct discard_policy dpolicy;
 	int err = 0;
 
 	if (start >= MAX_BLKADDR(sbi) || range->len < sbi->blocksize)
@@ -2347,9 +2344,9 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
 	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);
+	init_discard_policy(&dpolicy, DPOLICY_FSTRIM, cpc.trim_minlen);
+	__issue_discard_cmd_range(sbi, &dpolicy, start_block, end_block);
+	__wait_discard_cmd_range(sbi, &dpolicy, start_block, end_block);
 out:
 	range->len = F2FS_BLK_TO_BYTES(cpc.trimmed);
 	return err;
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index e0a6cc23ace3..5a1f7b9c8a72 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -796,8 +796,9 @@ static inline void wake_up_discard_thread(struct f2fs_sb_info *sbi, bool force)
 		goto wake_up;
 
 	mutex_lock(&dcc->cmd_lock);
-	for (i = MAX_PLIST_NUM - 1;
-			i >= 0 && plist_issue(dcc->pend_list_tag[i]); i--) {
+	for (i = MAX_PLIST_NUM - 1; i >= 0; i--) {
+		if (i + 1 < dcc->discard_granularity)
+			break;
 		if (!list_empty(&dcc->pend_list[i])) {
 			wakeup = true;
 			break;
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index b5a9e0a41c25..2beb929652a1 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -169,23 +169,10 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
 	}
 
 	if (!strcmp(a->attr.name, "discard_granularity")) {
-		struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
-		int i;
-
 		if (t == 0 || t > MAX_PLIST_NUM)
 			return -EINVAL;
 		if (t == *ui)
 			return count;
-
-		mutex_lock(&dcc->cmd_lock);
-		for (i = 0; i < MAX_PLIST_NUM; i++) {
-			if (i >= t - 1)
-				dcc->pend_list_tag[i] |= P_ACTIVE;
-			else
-				dcc->pend_list_tag[i] &= (~P_ACTIVE);
-		}
-		mutex_unlock(&dcc->cmd_lock);
-
 		*ui = t;
 		return count;
 	}
-- 
2.14.1.145.gb3622a4ee

[PATCH 4/6] f2fs: reduce cmd_lock coverage in __issue_discard_cmd

[Chao Yu]

From: Chao Yu <yuchao0@huawei.com>

__submit_discard_cmd may lead long latency due to exhaustion of I/O
request resource in block layer, so issuing all discard under cmd_lock
may lead to hangtask, in order to avoid that, let's reduce it's coverage.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/segment.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index c3eb355b0171..412ac3a68be3 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1130,14 +1130,14 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
 	int i, iter = 0, issued = 0;
 	bool io_interrupted = false;
 
-	mutex_lock(&dcc->cmd_lock);
-	f2fs_bug_on(sbi,
-		!__check_rb_tree_consistence(sbi, &dcc->root));
-	blk_start_plug(&plug);
 	for (i = MAX_PLIST_NUM - 1; i >= 0; i--) {
 		if (i + 1 < dpolicy->granularity)
 			break;
 		pend_list = &dcc->pend_list[i];
+
+		mutex_lock(&dcc->cmd_lock);
+		f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, &dcc->root));
+		blk_start_plug(&plug);
 		list_for_each_entry_safe(dc, tmp, pend_list, list) {
 			f2fs_bug_on(sbi, dc->state != D_PREP);
 
@@ -1151,12 +1151,14 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
 			issued++;
 skip:
 			if (++iter >= dpolicy->max_requests)
-				goto out;
+				break;
 		}
+		blk_finish_plug(&plug);
+		mutex_unlock(&dcc->cmd_lock);
+
+		if (iter >= dpolicy->max_requests)
+			break;
 	}
-out:
-	blk_finish_plug(&plug);
-	mutex_unlock(&dcc->cmd_lock);
 
 	if (!issued && io_interrupted)
 		issued = -1;
-- 
2.14.1.145.gb3622a4ee

[PATCH 4/6] f2fs: reduce cmd_lock coverage in __issue_discard_cmd

[Chao Yu]

From: Chao Yu <yuchao0@huawei.com>

__submit_discard_cmd may lead long latency due to exhaustion of I/O
request resource in block layer, so issuing all discard under cmd_lock
may lead to hangtask, in order to avoid that, let's reduce it's coverage.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/segment.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index c3eb355b0171..412ac3a68be3 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1130,14 +1130,14 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
 	int i, iter = 0, issued = 0;
 	bool io_interrupted = false;
 
-	mutex_lock(&dcc->cmd_lock);
-	f2fs_bug_on(sbi,
-		!__check_rb_tree_consistence(sbi, &dcc->root));
-	blk_start_plug(&plug);
 	for (i = MAX_PLIST_NUM - 1; i >= 0; i--) {
 		if (i + 1 < dpolicy->granularity)
 			break;
 		pend_list = &dcc->pend_list[i];
+
+		mutex_lock(&dcc->cmd_lock);
+		f2fs_bug_on(sbi, !__check_rb_tree_consistence(sbi, &dcc->root));
+		blk_start_plug(&plug);
 		list_for_each_entry_safe(dc, tmp, pend_list, list) {
 			f2fs_bug_on(sbi, dc->state != D_PREP);
 
@@ -1151,12 +1151,14 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
 			issued++;
 skip:
 			if (++iter >= dpolicy->max_requests)
-				goto out;
+				break;
 		}
+		blk_finish_plug(&plug);
+		mutex_unlock(&dcc->cmd_lock);
+
+		if (iter >= dpolicy->max_requests)
+			break;
 	}
-out:
-	blk_finish_plug(&plug);
-	mutex_unlock(&dcc->cmd_lock);
 
 	if (!issued && io_interrupted)
 		issued = -1;
-- 
2.14.1.145.gb3622a4ee


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH 5/6] f2fs: trace f2fs_remove_discard

[Chao Yu]

From: Chao Yu <yuchao0@huawei.com>

This patch adds tracepoint to trace f2fs_remove_discard.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/segment.c           | 2 ++
 include/trace/events/f2fs.h | 7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 412ac3a68be3..85f909419b69 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -794,6 +794,8 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi,
 {
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
 
+	trace_f2fs_remove_discard(dc->bdev, dc->start, dc->len);
+
 	f2fs_bug_on(sbi, dc->ref);
 
 	if (dc->error == -EOPNOTSUPP)
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 8955e75dd48e..407713ef5981 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -1286,6 +1286,13 @@ DEFINE_EVENT(f2fs_discard, f2fs_issue_discard,
 	TP_ARGS(dev, blkstart, blklen)
 );
 
+DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
+
+	TP_PROTO(struct block_device *dev, block_t blkstart, block_t blklen),
+
+	TP_ARGS(dev, blkstart, blklen)
+);
+
 TRACE_EVENT(f2fs_issue_reset_zone,
 
 	TP_PROTO(struct block_device *dev, block_t blkstart),
-- 
2.14.1.145.gb3622a4ee

[PATCH 6/6] f2fs: give up CP_TRIMMED_FLAG if it drops discards

[Chao Yu]

From: Chao Yu <yuchao0@huawei.com>

In ->umount, once we drop remained discard entries, we should not
set CP_TRIMMED_FLAG with another checkpoint.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/f2fs.h    |  2 +-
 fs/f2fs/segment.c | 15 +++++++++++----
 fs/f2fs/super.c   |  5 +++--
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e6f88171cbc9..7138610beacb 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2563,7 +2563,7 @@ void init_discard_policy(struct discard_policy *dpolicy, int discard_type,
 						unsigned int granularity);
 void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new);
 void stop_discard_thread(struct f2fs_sb_info *sbi);
-void f2fs_wait_discard_bios(struct f2fs_sb_info *sbi);
+bool f2fs_wait_discard_bios(struct f2fs_sb_info *sbi);
 void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc);
 void release_discard_addrs(struct f2fs_sb_info *sbi);
 int npages_for_summary_flush(struct f2fs_sb_info *sbi, bool for_ra);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 85f909419b69..8c9040960b29 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1168,12 +1168,13 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
 	return issued;
 }
 
-static void __drop_discard_cmd(struct f2fs_sb_info *sbi)
+static bool __drop_discard_cmd(struct f2fs_sb_info *sbi)
 {
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
 	struct list_head *pend_list;
 	struct discard_cmd *dc, *tmp;
 	int i;
+	bool dropped = false;
 
 	mutex_lock(&dcc->cmd_lock);
 	for (i = MAX_PLIST_NUM - 1; i >= 0; i--) {
@@ -1181,9 +1182,12 @@ static void __drop_discard_cmd(struct f2fs_sb_info *sbi)
 		list_for_each_entry_safe(dc, tmp, pend_list, list) {
 			f2fs_bug_on(sbi, dc->state != D_PREP);
 			__remove_discard_cmd(sbi, dc);
+			dropped = true;
 		}
 	}
 	mutex_unlock(&dcc->cmd_lock);
+
+	return dropped;
 }
 
 static void __wait_one_discard_bio(struct f2fs_sb_info *sbi,
@@ -1278,15 +1282,18 @@ void stop_discard_thread(struct f2fs_sb_info *sbi)
 }
 
 /* This comes from f2fs_put_super */
-void f2fs_wait_discard_bios(struct f2fs_sb_info *sbi)
+bool f2fs_wait_discard_bios(struct f2fs_sb_info *sbi)
 {
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
 	struct discard_policy dpolicy;
+	bool dropped;
 
 	init_discard_policy(&dpolicy, DPOLICY_UMOUNT, dcc->discard_granularity);
 	__issue_discard_cmd(sbi, &dpolicy);
-	__drop_discard_cmd(sbi);
+	dropped = __drop_discard_cmd(sbi);
 	__wait_all_discard_cmd(sbi, &dpolicy);
+
+	return dropped;
 }
 
 static int issue_discard_thread(void *data)
@@ -1631,7 +1638,7 @@ void init_discard_policy(struct discard_policy *dpolicy,
 		dpolicy->max_interval = DEF_MAX_DISCARD_ISSUE_TIME;
 		dpolicy->max_requests = DEF_MAX_DISCARD_REQUEST;
 		dpolicy->io_aware_gran = MAX_PLIST_NUM;
-		dpolicy->io_aware = false;
+		dpolicy->io_aware = true;
 	} else if (discard_type == DPOLICY_FSTRIM) {
 		dpolicy->max_requests = DEF_MAX_DISCARD_REQUEST;
 		dpolicy->io_aware_gran = MAX_PLIST_NUM;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index f4a5407e9998..b4152ef723b0 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -807,6 +807,7 @@ static void f2fs_put_super(struct super_block *sb)
 {
 	struct f2fs_sb_info *sbi = F2FS_SB(sb);
 	int i;
+	bool dropped;
 
 	f2fs_quota_off_umount(sb);
 
@@ -827,9 +828,9 @@ static void f2fs_put_super(struct super_block *sb)
 	}
 
 	/* be sure to wait for any on-going discard commands */
-	f2fs_wait_discard_bios(sbi);
+	dropped = f2fs_wait_discard_bios(sbi);
 
-	if (f2fs_discard_en(sbi) && !sbi->discard_blks) {
+	if (f2fs_discard_en(sbi) && !sbi->discard_blks && !dropped) {
 		struct cp_control cpc = {
 			.reason = CP_UMOUNT | CP_TRIMMED,
 		};
-- 
2.14.1.145.gb3622a4ee

Re: [PATCH 1/6] f2fs: support issuing/waiting discard in range

[Jaegeuk Kim]

On 09/19, Chao Yu wrote:
> From: Chao Yu <yuchao0@huawei.com>
> 
> 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 <yuchao0@huawei.com>
> ---
>  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;

Where did you move this in the change? Or, we don't need this at all?

>  				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);
> @@ -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

Re: [PATCH 1/6] f2fs: support issuing/waiting discard in range

[Chao Yu]

On 2017/9/27 1:00, Jaegeuk Kim wrote:
> On 09/19, Chao Yu wrote:
>> From: Chao Yu <yuchao0@huawei.com>
>>
>> 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 <yuchao0@huawei.com>
>> ---
>>  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))

Just moved here. ;)

Thanks,

>> +			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;
> 
> Where did you move this in the change? Or, we don't need this at all?
> 
>>  				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);
>> @@ -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
> 
> .
> 

Re: [PATCH 1/6] f2fs: support issuing/waiting discard in range

[Chao Yu]

On 2017/9/27 1:00, Jaegeuk Kim wrote:
> On 09/19, Chao Yu wrote:
>> From: Chao Yu <yuchao0@huawei.com>
>>
>> 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 <yuchao0@huawei.com>
>> ---
>>  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))

Just moved here. ;)

Thanks,

>> +			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;
> 
> Where did you move this in the change? Or, we don't need this at all?
> 
>>  				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);
>> @@ -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
> 
> .
> 

Re: [PATCH 1/6] f2fs: support issuing/waiting discard in range

[Jaegeuk Kim]

On 09/19, Chao Yu wrote:
> From: Chao Yu <yuchao0@huawei.com>
> 
> 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 <yuchao0@huawei.com>
> ---
>  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

Re: [PATCH 1/6] f2fs: support issuing/waiting discard in range

[Jaegeuk Kim]

Hi Chao,

Could you please rebase the following patches on top of dev-test?

f2fs-support-issuing-waiting-discard-in-range
f2fs-wrap-discard-policy
f2fs-split-discard-policy
f2fs-reduce-cmd_lock-coverage-in-__issue_discard_cmd
f2fs-trace-f2fs_remove_discard
f2fs-give-up-CP_TRIMMED_FLAG-if-it-drops-discards

It seems we need to rearrange some of patches for better review.

Thanks,

On 10/03, Jaegeuk Kim wrote:
> On 09/19, Chao Yu wrote:
> > From: Chao Yu <yuchao0@huawei.com>
> > 
> > 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 <yuchao0@huawei.com>
> > ---
> >  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

Re: [f2fs-dev] [PATCH 1/6] f2fs: support issuing/waiting discard in range

[Chao Yu]

Hi Jaegeuk,

On 2017/10/4 4:52, Jaegeuk Kim wrote:
> Hi Chao,
> 
> Could you please rebase the following patches on top of dev-test?
> 
> f2fs-support-issuing-waiting-discard-in-range
> f2fs-wrap-discard-policy
> f2fs-split-discard-policy
> f2fs-reduce-cmd_lock-coverage-in-__issue_discard_cmd
> f2fs-trace-f2fs_remove_discard
> f2fs-give-up-CP_TRIMMED_FLAG-if-it-drops-discards
> 
> It seems we need to rearrange some of patches for better review.

No problem, let me rebase them. :)

Thanks,

> 
> Thanks,
> 
> On 10/03, Jaegeuk Kim wrote:
>> On 09/19, Chao Yu wrote:
>>> From: Chao Yu <yuchao0@huawei.com>
>>>
>>> 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 <yuchao0@huawei.com>
>>> ---
>>>  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
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 

Re: [PATCH 1/6] f2fs: support issuing/waiting discard in range

[Chao Yu]

Hi Jaegeuk,

On 2017/10/4 4:16, Jaegeuk Kim wrote:
> On 09/19, Chao Yu wrote:
>> From: Chao Yu <yuchao0@huawei.com>
>>
>> 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 <yuchao0@huawei.com>
>> ---
>>  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;

+	__wait_discard_cmd_range(sbi, false, start_block, end_block,
+						cpc.trim_minlen, true);

We can control wait_cond through the parameter in __wait_discard_cmd_range,
let me fix that wrong parameter passed from f2fs_trim_fs. :)

Thanks,

> 
> 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