All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] f2fs: add support for counting time of submit discard cmd
@ 2022-12-12 12:51 ` Yangtao Li via Linux-f2fs-devel
  0 siblings, 0 replies; 20+ messages in thread
From: Yangtao Li @ 2022-12-12 12:51 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: linux-f2fs-devel, linux-kernel, Yangtao Li

This patch adds support for counting the average time and
peak time of submit discard command, and we can see its
value in debugfs.

It is not sure whether the block layer has recorded these
data, and these data are allowed to be accessed by fs,
or they are only exported to user space.

On the one hand, I added these data to better understand
the current device operating status, and to further control
the discard process in a more detailed manner based on the
discard submit time in the future.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/f2fs/debug.c   | 10 +++++++---
 fs/f2fs/f2fs.h    |  6 ++++++
 fs/f2fs/segment.c | 21 +++++++++++++++++++--
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index 32af4f0c5735..142c256b89d9 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -120,6 +120,10 @@ static void update_general_status(struct f2fs_sb_info *sbi)
 			llist_empty(&SM_I(sbi)->fcc_info->issue_list);
 	}
 	if (SM_I(sbi)->dcc_info) {
+		struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
+
+		si->discard_avg = dcc->discard_time_avg;
+		si->discard_peak = dcc->discard_time_peak;
 		si->nr_discarded =
 			atomic_read(&SM_I(sbi)->dcc_info->issued_discard);
 		si->nr_discarding =
@@ -545,9 +549,9 @@ static int stat_show(struct seq_file *s, void *v)
 			   si->nr_wb_cp_data, si->nr_wb_data,
 			   si->nr_flushing, si->nr_flushed,
 			   si->flush_list_empty);
-		seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
-			   si->nr_discarding, si->nr_discarded,
-			   si->nr_discard_cmd, si->undiscard_blks);
+		seq_printf(s, "Discard: (%4d %4d, avg:%4lldns, peak:%4lldns)) cmd: %4d undiscard:%4u\n",
+			   si->nr_discarding, si->nr_discarded, ktime_to_us(si->discard_avg),
+			   ktime_to_us(si->discard_peak), si->nr_discard_cmd, si->undiscard_blks);
 		seq_printf(s, "  - atomic IO: %4d (Max. %4d)\n",
 			   si->aw_cnt, si->max_aw_cnt);
 		seq_printf(s, "  - compress: %4d, hit:%8d\n", si->compress_pages, si->compress_page_hit);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e8953c3dc81a..2cd55cb981ff 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -371,6 +371,8 @@ struct discard_cmd {
 	int error;			/* bio error */
 	spinlock_t lock;		/* for state/bio_ref updating */
 	unsigned short bio_ref;		/* bio reference count */
+	struct discard_cmd_control *dcc;	/* global discard cmd control */
+	ktime_t submit_start;	/* submit start time */
 };
 
 enum {
@@ -415,6 +417,9 @@ struct discard_cmd_control {
 	unsigned int max_ordered_discard;	/* maximum discard granularity issued by lba order */
 	unsigned int undiscard_blks;		/* # of undiscard blocks */
 	unsigned int next_pos;			/* next discard position */
+	spinlock_t discard_time_lock;	/* for discard time statistics */
+	ktime_t discard_time_avg;		/* issued discard cmd avg time */
+	ktime_t discard_time_peak;		/* issued discard cmd peak time */
 	atomic_t issued_discard;		/* # of issued discard */
 	atomic_t queued_discard;		/* # of queued discard */
 	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
@@ -3896,6 +3901,7 @@ struct f2fs_stat_info {
 	int nr_dio_read, nr_dio_write;
 	unsigned int io_skip_bggc, other_skip_bggc;
 	int nr_flushing, nr_flushed, flush_list_empty;
+	ktime_t discard_avg, discard_peak;
 	int nr_discarding, nr_discarded;
 	int nr_discard_cmd;
 	unsigned int undiscard_blks;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index a9099a754dd2..73cd05bb3f4a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -937,6 +937,7 @@ static struct discard_cmd *__create_discard_cmd(struct f2fs_sb_info *sbi,
 	list_add_tail(&dc->list, pend_list);
 	spin_lock_init(&dc->lock);
 	dc->bio_ref = 0;
+	dc->dcc = dcc;
 	atomic_inc(&dcc->discard_cmd_cnt);
 	dcc->undiscard_blks += len;
 
@@ -1006,9 +1007,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi,
 static void f2fs_submit_discard_endio(struct bio *bio)
 {
 	struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
+	struct discard_cmd_control *dcc = dc->dcc;
 	unsigned long flags;
+	ktime_t submit_time;
+	int nr_discarded;
 
 	spin_lock_irqsave(&dc->lock, flags);
+	submit_time = ktime_sub(ktime_get(), dc->submit_start);
 	if (!dc->error)
 		dc->error = blk_status_to_errno(bio->bi_status);
 	dc->bio_ref--;
@@ -1018,6 +1023,16 @@ static void f2fs_submit_discard_endio(struct bio *bio)
 	}
 	spin_unlock_irqrestore(&dc->lock, flags);
 	bio_put(bio);
+
+	spin_lock_irqsave(&dcc->discard_time_lock, flags);
+	nr_discarded = atomic_read(&dcc->issued_discard);
+	dcc->discard_time_avg = div_u64(ktime_add(nr_discarded * dcc->discard_time_avg,
+										submit_time),
+									nr_discarded + 1);
+	if (dcc->discard_time_avg > dcc->discard_time_peak)
+		dcc->discard_time_peak = dcc->discard_time_avg;
+	atomic_inc(&dcc->issued_discard);
+	spin_unlock_irqrestore(&dcc->discard_time_lock, flags);
 }
 
 static void __check_sit_bitmap(struct f2fs_sb_info *sbi,
@@ -1166,6 +1181,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
 		 * right away
 		 */
 		spin_lock_irqsave(&dc->lock, flags);
+		dc->submit_start = ktime_get();
 		if (last)
 			dc->state = D_SUBMIT;
 		else
@@ -1185,8 +1201,6 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
 		bio->bi_opf |= flag;
 		submit_bio(bio);
 
-		atomic_inc(&dcc->issued_discard);
-
 		f2fs_update_iostat(sbi, NULL, FS_DISCARD, len * F2FS_BLKSIZE);
 
 		lstart += len;
@@ -2079,9 +2093,12 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
 	INIT_LIST_HEAD(&dcc->wait_list);
 	INIT_LIST_HEAD(&dcc->fstrim_list);
 	mutex_init(&dcc->cmd_lock);
+	spin_lock_init(&dcc->discard_time_lock);
 	atomic_set(&dcc->issued_discard, 0);
 	atomic_set(&dcc->queued_discard, 0);
 	atomic_set(&dcc->discard_cmd_cnt, 0);
+	dcc->discard_time_avg = 0;
+	dcc->discard_time_peak = 0;
 	dcc->nr_discards = 0;
 	dcc->max_discards = MAIN_SEGS(sbi) << sbi->log_blocks_per_seg;
 	dcc->max_discard_request = DEF_MAX_DISCARD_REQUEST;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
@ 2022-12-12 12:51 ` Yangtao Li via Linux-f2fs-devel
  0 siblings, 0 replies; 20+ messages in thread
From: Yangtao Li via Linux-f2fs-devel @ 2022-12-12 12:51 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: Yangtao Li, linux-kernel, linux-f2fs-devel

This patch adds support for counting the average time and
peak time of submit discard command, and we can see its
value in debugfs.

It is not sure whether the block layer has recorded these
data, and these data are allowed to be accessed by fs,
or they are only exported to user space.

On the one hand, I added these data to better understand
the current device operating status, and to further control
the discard process in a more detailed manner based on the
discard submit time in the future.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/f2fs/debug.c   | 10 +++++++---
 fs/f2fs/f2fs.h    |  6 ++++++
 fs/f2fs/segment.c | 21 +++++++++++++++++++--
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index 32af4f0c5735..142c256b89d9 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -120,6 +120,10 @@ static void update_general_status(struct f2fs_sb_info *sbi)
 			llist_empty(&SM_I(sbi)->fcc_info->issue_list);
 	}
 	if (SM_I(sbi)->dcc_info) {
+		struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
+
+		si->discard_avg = dcc->discard_time_avg;
+		si->discard_peak = dcc->discard_time_peak;
 		si->nr_discarded =
 			atomic_read(&SM_I(sbi)->dcc_info->issued_discard);
 		si->nr_discarding =
@@ -545,9 +549,9 @@ static int stat_show(struct seq_file *s, void *v)
 			   si->nr_wb_cp_data, si->nr_wb_data,
 			   si->nr_flushing, si->nr_flushed,
 			   si->flush_list_empty);
-		seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
-			   si->nr_discarding, si->nr_discarded,
-			   si->nr_discard_cmd, si->undiscard_blks);
+		seq_printf(s, "Discard: (%4d %4d, avg:%4lldns, peak:%4lldns)) cmd: %4d undiscard:%4u\n",
+			   si->nr_discarding, si->nr_discarded, ktime_to_us(si->discard_avg),
+			   ktime_to_us(si->discard_peak), si->nr_discard_cmd, si->undiscard_blks);
 		seq_printf(s, "  - atomic IO: %4d (Max. %4d)\n",
 			   si->aw_cnt, si->max_aw_cnt);
 		seq_printf(s, "  - compress: %4d, hit:%8d\n", si->compress_pages, si->compress_page_hit);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e8953c3dc81a..2cd55cb981ff 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -371,6 +371,8 @@ struct discard_cmd {
 	int error;			/* bio error */
 	spinlock_t lock;		/* for state/bio_ref updating */
 	unsigned short bio_ref;		/* bio reference count */
+	struct discard_cmd_control *dcc;	/* global discard cmd control */
+	ktime_t submit_start;	/* submit start time */
 };
 
 enum {
@@ -415,6 +417,9 @@ struct discard_cmd_control {
 	unsigned int max_ordered_discard;	/* maximum discard granularity issued by lba order */
 	unsigned int undiscard_blks;		/* # of undiscard blocks */
 	unsigned int next_pos;			/* next discard position */
+	spinlock_t discard_time_lock;	/* for discard time statistics */
+	ktime_t discard_time_avg;		/* issued discard cmd avg time */
+	ktime_t discard_time_peak;		/* issued discard cmd peak time */
 	atomic_t issued_discard;		/* # of issued discard */
 	atomic_t queued_discard;		/* # of queued discard */
 	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
@@ -3896,6 +3901,7 @@ struct f2fs_stat_info {
 	int nr_dio_read, nr_dio_write;
 	unsigned int io_skip_bggc, other_skip_bggc;
 	int nr_flushing, nr_flushed, flush_list_empty;
+	ktime_t discard_avg, discard_peak;
 	int nr_discarding, nr_discarded;
 	int nr_discard_cmd;
 	unsigned int undiscard_blks;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index a9099a754dd2..73cd05bb3f4a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -937,6 +937,7 @@ static struct discard_cmd *__create_discard_cmd(struct f2fs_sb_info *sbi,
 	list_add_tail(&dc->list, pend_list);
 	spin_lock_init(&dc->lock);
 	dc->bio_ref = 0;
+	dc->dcc = dcc;
 	atomic_inc(&dcc->discard_cmd_cnt);
 	dcc->undiscard_blks += len;
 
@@ -1006,9 +1007,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi,
 static void f2fs_submit_discard_endio(struct bio *bio)
 {
 	struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
+	struct discard_cmd_control *dcc = dc->dcc;
 	unsigned long flags;
+	ktime_t submit_time;
+	int nr_discarded;
 
 	spin_lock_irqsave(&dc->lock, flags);
+	submit_time = ktime_sub(ktime_get(), dc->submit_start);
 	if (!dc->error)
 		dc->error = blk_status_to_errno(bio->bi_status);
 	dc->bio_ref--;
@@ -1018,6 +1023,16 @@ static void f2fs_submit_discard_endio(struct bio *bio)
 	}
 	spin_unlock_irqrestore(&dc->lock, flags);
 	bio_put(bio);
+
+	spin_lock_irqsave(&dcc->discard_time_lock, flags);
+	nr_discarded = atomic_read(&dcc->issued_discard);
+	dcc->discard_time_avg = div_u64(ktime_add(nr_discarded * dcc->discard_time_avg,
+										submit_time),
+									nr_discarded + 1);
+	if (dcc->discard_time_avg > dcc->discard_time_peak)
+		dcc->discard_time_peak = dcc->discard_time_avg;
+	atomic_inc(&dcc->issued_discard);
+	spin_unlock_irqrestore(&dcc->discard_time_lock, flags);
 }
 
 static void __check_sit_bitmap(struct f2fs_sb_info *sbi,
@@ -1166,6 +1181,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
 		 * right away
 		 */
 		spin_lock_irqsave(&dc->lock, flags);
+		dc->submit_start = ktime_get();
 		if (last)
 			dc->state = D_SUBMIT;
 		else
@@ -1185,8 +1201,6 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
 		bio->bi_opf |= flag;
 		submit_bio(bio);
 
-		atomic_inc(&dcc->issued_discard);
-
 		f2fs_update_iostat(sbi, NULL, FS_DISCARD, len * F2FS_BLKSIZE);
 
 		lstart += len;
@@ -2079,9 +2093,12 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
 	INIT_LIST_HEAD(&dcc->wait_list);
 	INIT_LIST_HEAD(&dcc->fstrim_list);
 	mutex_init(&dcc->cmd_lock);
+	spin_lock_init(&dcc->discard_time_lock);
 	atomic_set(&dcc->issued_discard, 0);
 	atomic_set(&dcc->queued_discard, 0);
 	atomic_set(&dcc->discard_cmd_cnt, 0);
+	dcc->discard_time_avg = 0;
+	dcc->discard_time_peak = 0;
 	dcc->nr_discards = 0;
 	dcc->max_discards = MAIN_SEGS(sbi) << sbi->log_blocks_per_seg;
 	dcc->max_discard_request = DEF_MAX_DISCARD_REQUEST;
-- 
2.25.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] f2fs: add support for counting time of submit discard cmd
  2022-12-12 12:51 ` [f2fs-dev] " Yangtao Li via Linux-f2fs-devel
@ 2022-12-12 13:40   ` Chao Yu
  -1 siblings, 0 replies; 20+ messages in thread
From: Chao Yu @ 2022-12-12 13:40 UTC (permalink / raw)
  To: Yangtao Li, jaegeuk; +Cc: linux-f2fs-devel, linux-kernel

On 2022/12/12 20:51, Yangtao Li wrote:
> This patch adds support for counting the average time and
> peak time of submit discard command, and we can see its
> value in debugfs.
> 
> It is not sure whether the block layer has recorded these
> data, and these data are allowed to be accessed by fs,
> or they are only exported to user space.
> 
> On the one hand, I added these data to better understand
> the current device operating status, and to further control
> the discard process in a more detailed manner based on the
> discard submit time in the future.

Again, w'd better to consider this functionality only when DEBUG_FS is
enabled.

> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>   fs/f2fs/debug.c   | 10 +++++++---
>   fs/f2fs/f2fs.h    |  6 ++++++
>   fs/f2fs/segment.c | 21 +++++++++++++++++++--
>   3 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index 32af4f0c5735..142c256b89d9 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -120,6 +120,10 @@ static void update_general_status(struct f2fs_sb_info *sbi)
>   			llist_empty(&SM_I(sbi)->fcc_info->issue_list);
>   	}
>   	if (SM_I(sbi)->dcc_info) {
> +		struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> +
> +		si->discard_avg = dcc->discard_time_avg;
> +		si->discard_peak = dcc->discard_time_peak;
>   		si->nr_discarded =
>   			atomic_read(&SM_I(sbi)->dcc_info->issued_discard);
>   		si->nr_discarding =
> @@ -545,9 +549,9 @@ static int stat_show(struct seq_file *s, void *v)
>   			   si->nr_wb_cp_data, si->nr_wb_data,
>   			   si->nr_flushing, si->nr_flushed,
>   			   si->flush_list_empty);
> -		seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
> -			   si->nr_discarding, si->nr_discarded,
> -			   si->nr_discard_cmd, si->undiscard_blks);
> +		seq_printf(s, "Discard: (%4d %4d, avg:%4lldns, peak:%4lldns)) cmd: %4d undiscard:%4u\n",
> +			   si->nr_discarding, si->nr_discarded, ktime_to_us(si->discard_avg),
> +			   ktime_to_us(si->discard_peak), si->nr_discard_cmd, si->undiscard_blks);
>   		seq_printf(s, "  - atomic IO: %4d (Max. %4d)\n",
>   			   si->aw_cnt, si->max_aw_cnt);
>   		seq_printf(s, "  - compress: %4d, hit:%8d\n", si->compress_pages, si->compress_page_hit);
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index e8953c3dc81a..2cd55cb981ff 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -371,6 +371,8 @@ struct discard_cmd {
>   	int error;			/* bio error */
>   	spinlock_t lock;		/* for state/bio_ref updating */
>   	unsigned short bio_ref;		/* bio reference count */
> +	struct discard_cmd_control *dcc;	/* global discard cmd control */
> +	ktime_t submit_start;	/* submit start time */
>   };
>   
>   enum {
> @@ -415,6 +417,9 @@ struct discard_cmd_control {
>   	unsigned int max_ordered_discard;	/* maximum discard granularity issued by lba order */
>   	unsigned int undiscard_blks;		/* # of undiscard blocks */
>   	unsigned int next_pos;			/* next discard position */
> +	spinlock_t discard_time_lock;	/* for discard time statistics */
> +	ktime_t discard_time_avg;		/* issued discard cmd avg time */
> +	ktime_t discard_time_peak;		/* issued discard cmd peak time */
>   	atomic_t issued_discard;		/* # of issued discard */
>   	atomic_t queued_discard;		/* # of queued discard */
>   	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
> @@ -3896,6 +3901,7 @@ struct f2fs_stat_info {
>   	int nr_dio_read, nr_dio_write;
>   	unsigned int io_skip_bggc, other_skip_bggc;
>   	int nr_flushing, nr_flushed, flush_list_empty;
> +	ktime_t discard_avg, discard_peak;
>   	int nr_discarding, nr_discarded;
>   	int nr_discard_cmd;
>   	unsigned int undiscard_blks;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index a9099a754dd2..73cd05bb3f4a 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -937,6 +937,7 @@ static struct discard_cmd *__create_discard_cmd(struct f2fs_sb_info *sbi,
>   	list_add_tail(&dc->list, pend_list);
>   	spin_lock_init(&dc->lock);
>   	dc->bio_ref = 0;
> +	dc->dcc = dcc;
>   	atomic_inc(&dcc->discard_cmd_cnt);
>   	dcc->undiscard_blks += len;
>   
> @@ -1006,9 +1007,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi,
>   static void f2fs_submit_discard_endio(struct bio *bio)
>   {
>   	struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
> +	struct discard_cmd_control *dcc = dc->dcc;
>   	unsigned long flags;
> +	ktime_t submit_time;
> +	int nr_discarded;
>   
>   	spin_lock_irqsave(&dc->lock, flags);
> +	submit_time = ktime_sub(ktime_get(), dc->submit_start);
>   	if (!dc->error)
>   		dc->error = blk_status_to_errno(bio->bi_status);
>   	dc->bio_ref--;
> @@ -1018,6 +1023,16 @@ static void f2fs_submit_discard_endio(struct bio *bio)
>   	}
>   	spin_unlock_irqrestore(&dc->lock, flags);
>   	bio_put(bio);
> +
> +	spin_lock_irqsave(&dcc->discard_time_lock, flags);
> +	nr_discarded = atomic_read(&dcc->issued_discard);
> +	dcc->discard_time_avg = div_u64(ktime_add(nr_discarded * dcc->discard_time_avg,
> +										submit_time),
> +									nr_discarded + 1);
> +	if (dcc->discard_time_avg > dcc->discard_time_peak)
> +		dcc->discard_time_peak = dcc->discard_time_avg;
> +	atomic_inc(&dcc->issued_discard);
> +	spin_unlock_irqrestore(&dcc->discard_time_lock, flags);

Why not calculating average time only in update_general_status()? and here,
we just need to account total_{discard_time, discard_count} w/o additional
spinlock.

Thanks,

>   }
>   
>   static void __check_sit_bitmap(struct f2fs_sb_info *sbi,
> @@ -1166,6 +1181,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
>   		 * right away
>   		 */
>   		spin_lock_irqsave(&dc->lock, flags);
> +		dc->submit_start = ktime_get();
>   		if (last)
>   			dc->state = D_SUBMIT;
>   		else
> @@ -1185,8 +1201,6 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
>   		bio->bi_opf |= flag;
>   		submit_bio(bio);
>   
> -		atomic_inc(&dcc->issued_discard);
> -
>   		f2fs_update_iostat(sbi, NULL, FS_DISCARD, len * F2FS_BLKSIZE);
>   
>   		lstart += len;
> @@ -2079,9 +2093,12 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
>   	INIT_LIST_HEAD(&dcc->wait_list);
>   	INIT_LIST_HEAD(&dcc->fstrim_list);
>   	mutex_init(&dcc->cmd_lock);
> +	spin_lock_init(&dcc->discard_time_lock);
>   	atomic_set(&dcc->issued_discard, 0);
>   	atomic_set(&dcc->queued_discard, 0);
>   	atomic_set(&dcc->discard_cmd_cnt, 0);
> +	dcc->discard_time_avg = 0;
> +	dcc->discard_time_peak = 0;
>   	dcc->nr_discards = 0;
>   	dcc->max_discards = MAIN_SEGS(sbi) << sbi->log_blocks_per_seg;
>   	dcc->max_discard_request = DEF_MAX_DISCARD_REQUEST;

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
@ 2022-12-12 13:40   ` Chao Yu
  0 siblings, 0 replies; 20+ messages in thread
From: Chao Yu @ 2022-12-12 13:40 UTC (permalink / raw)
  To: Yangtao Li, jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

On 2022/12/12 20:51, Yangtao Li wrote:
> This patch adds support for counting the average time and
> peak time of submit discard command, and we can see its
> value in debugfs.
> 
> It is not sure whether the block layer has recorded these
> data, and these data are allowed to be accessed by fs,
> or they are only exported to user space.
> 
> On the one hand, I added these data to better understand
> the current device operating status, and to further control
> the discard process in a more detailed manner based on the
> discard submit time in the future.

Again, w'd better to consider this functionality only when DEBUG_FS is
enabled.

> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>   fs/f2fs/debug.c   | 10 +++++++---
>   fs/f2fs/f2fs.h    |  6 ++++++
>   fs/f2fs/segment.c | 21 +++++++++++++++++++--
>   3 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index 32af4f0c5735..142c256b89d9 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -120,6 +120,10 @@ static void update_general_status(struct f2fs_sb_info *sbi)
>   			llist_empty(&SM_I(sbi)->fcc_info->issue_list);
>   	}
>   	if (SM_I(sbi)->dcc_info) {
> +		struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> +
> +		si->discard_avg = dcc->discard_time_avg;
> +		si->discard_peak = dcc->discard_time_peak;
>   		si->nr_discarded =
>   			atomic_read(&SM_I(sbi)->dcc_info->issued_discard);
>   		si->nr_discarding =
> @@ -545,9 +549,9 @@ static int stat_show(struct seq_file *s, void *v)
>   			   si->nr_wb_cp_data, si->nr_wb_data,
>   			   si->nr_flushing, si->nr_flushed,
>   			   si->flush_list_empty);
> -		seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
> -			   si->nr_discarding, si->nr_discarded,
> -			   si->nr_discard_cmd, si->undiscard_blks);
> +		seq_printf(s, "Discard: (%4d %4d, avg:%4lldns, peak:%4lldns)) cmd: %4d undiscard:%4u\n",
> +			   si->nr_discarding, si->nr_discarded, ktime_to_us(si->discard_avg),
> +			   ktime_to_us(si->discard_peak), si->nr_discard_cmd, si->undiscard_blks);
>   		seq_printf(s, "  - atomic IO: %4d (Max. %4d)\n",
>   			   si->aw_cnt, si->max_aw_cnt);
>   		seq_printf(s, "  - compress: %4d, hit:%8d\n", si->compress_pages, si->compress_page_hit);
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index e8953c3dc81a..2cd55cb981ff 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -371,6 +371,8 @@ struct discard_cmd {
>   	int error;			/* bio error */
>   	spinlock_t lock;		/* for state/bio_ref updating */
>   	unsigned short bio_ref;		/* bio reference count */
> +	struct discard_cmd_control *dcc;	/* global discard cmd control */
> +	ktime_t submit_start;	/* submit start time */
>   };
>   
>   enum {
> @@ -415,6 +417,9 @@ struct discard_cmd_control {
>   	unsigned int max_ordered_discard;	/* maximum discard granularity issued by lba order */
>   	unsigned int undiscard_blks;		/* # of undiscard blocks */
>   	unsigned int next_pos;			/* next discard position */
> +	spinlock_t discard_time_lock;	/* for discard time statistics */
> +	ktime_t discard_time_avg;		/* issued discard cmd avg time */
> +	ktime_t discard_time_peak;		/* issued discard cmd peak time */
>   	atomic_t issued_discard;		/* # of issued discard */
>   	atomic_t queued_discard;		/* # of queued discard */
>   	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
> @@ -3896,6 +3901,7 @@ struct f2fs_stat_info {
>   	int nr_dio_read, nr_dio_write;
>   	unsigned int io_skip_bggc, other_skip_bggc;
>   	int nr_flushing, nr_flushed, flush_list_empty;
> +	ktime_t discard_avg, discard_peak;
>   	int nr_discarding, nr_discarded;
>   	int nr_discard_cmd;
>   	unsigned int undiscard_blks;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index a9099a754dd2..73cd05bb3f4a 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -937,6 +937,7 @@ static struct discard_cmd *__create_discard_cmd(struct f2fs_sb_info *sbi,
>   	list_add_tail(&dc->list, pend_list);
>   	spin_lock_init(&dc->lock);
>   	dc->bio_ref = 0;
> +	dc->dcc = dcc;
>   	atomic_inc(&dcc->discard_cmd_cnt);
>   	dcc->undiscard_blks += len;
>   
> @@ -1006,9 +1007,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi,
>   static void f2fs_submit_discard_endio(struct bio *bio)
>   {
>   	struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
> +	struct discard_cmd_control *dcc = dc->dcc;
>   	unsigned long flags;
> +	ktime_t submit_time;
> +	int nr_discarded;
>   
>   	spin_lock_irqsave(&dc->lock, flags);
> +	submit_time = ktime_sub(ktime_get(), dc->submit_start);
>   	if (!dc->error)
>   		dc->error = blk_status_to_errno(bio->bi_status);
>   	dc->bio_ref--;
> @@ -1018,6 +1023,16 @@ static void f2fs_submit_discard_endio(struct bio *bio)
>   	}
>   	spin_unlock_irqrestore(&dc->lock, flags);
>   	bio_put(bio);
> +
> +	spin_lock_irqsave(&dcc->discard_time_lock, flags);
> +	nr_discarded = atomic_read(&dcc->issued_discard);
> +	dcc->discard_time_avg = div_u64(ktime_add(nr_discarded * dcc->discard_time_avg,
> +										submit_time),
> +									nr_discarded + 1);
> +	if (dcc->discard_time_avg > dcc->discard_time_peak)
> +		dcc->discard_time_peak = dcc->discard_time_avg;
> +	atomic_inc(&dcc->issued_discard);
> +	spin_unlock_irqrestore(&dcc->discard_time_lock, flags);

Why not calculating average time only in update_general_status()? and here,
we just need to account total_{discard_time, discard_count} w/o additional
spinlock.

Thanks,

>   }
>   
>   static void __check_sit_bitmap(struct f2fs_sb_info *sbi,
> @@ -1166,6 +1181,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
>   		 * right away
>   		 */
>   		spin_lock_irqsave(&dc->lock, flags);
> +		dc->submit_start = ktime_get();
>   		if (last)
>   			dc->state = D_SUBMIT;
>   		else
> @@ -1185,8 +1201,6 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
>   		bio->bi_opf |= flag;
>   		submit_bio(bio);
>   
> -		atomic_inc(&dcc->issued_discard);
> -
>   		f2fs_update_iostat(sbi, NULL, FS_DISCARD, len * F2FS_BLKSIZE);
>   
>   		lstart += len;
> @@ -2079,9 +2093,12 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
>   	INIT_LIST_HEAD(&dcc->wait_list);
>   	INIT_LIST_HEAD(&dcc->fstrim_list);
>   	mutex_init(&dcc->cmd_lock);
> +	spin_lock_init(&dcc->discard_time_lock);
>   	atomic_set(&dcc->issued_discard, 0);
>   	atomic_set(&dcc->queued_discard, 0);
>   	atomic_set(&dcc->discard_cmd_cnt, 0);
> +	dcc->discard_time_avg = 0;
> +	dcc->discard_time_peak = 0;
>   	dcc->nr_discards = 0;
>   	dcc->max_discards = MAIN_SEGS(sbi) << sbi->log_blocks_per_seg;
>   	dcc->max_discard_request = DEF_MAX_DISCARD_REQUEST;


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
  2022-12-12 13:40   ` [f2fs-dev] " Chao Yu
@ 2022-12-12 22:47     ` Jaegeuk Kim
  -1 siblings, 0 replies; 20+ messages in thread
From: Jaegeuk Kim @ 2022-12-12 22:47 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, Yangtao Li

On 12/12, Chao Yu wrote:
> On 2022/12/12 20:51, Yangtao Li wrote:
> > This patch adds support for counting the average time and
> > peak time of submit discard command, and we can see its
> > value in debugfs.
> > 
> > It is not sure whether the block layer has recorded these
> > data, and these data are allowed to be accessed by fs,
> > or they are only exported to user space.
> > 
> > On the one hand, I added these data to better understand
> > the current device operating status, and to further control
> > the discard process in a more detailed manner based on the
> > discard submit time in the future.
> 
> Again, w'd better to consider this functionality only when DEBUG_FS is
> enabled.

BTW, why can't we use iostat to get the discard latencies?

> 
> > 
> > Signed-off-by: Yangtao Li <frank.li@vivo.com>
> > ---
> >   fs/f2fs/debug.c   | 10 +++++++---
> >   fs/f2fs/f2fs.h    |  6 ++++++
> >   fs/f2fs/segment.c | 21 +++++++++++++++++++--
> >   3 files changed, 32 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> > index 32af4f0c5735..142c256b89d9 100644
> > --- a/fs/f2fs/debug.c
> > +++ b/fs/f2fs/debug.c
> > @@ -120,6 +120,10 @@ static void update_general_status(struct f2fs_sb_info *sbi)
> >   			llist_empty(&SM_I(sbi)->fcc_info->issue_list);
> >   	}
> >   	if (SM_I(sbi)->dcc_info) {
> > +		struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > +
> > +		si->discard_avg = dcc->discard_time_avg;
> > +		si->discard_peak = dcc->discard_time_peak;
> >   		si->nr_discarded =
> >   			atomic_read(&SM_I(sbi)->dcc_info->issued_discard);
> >   		si->nr_discarding =
> > @@ -545,9 +549,9 @@ static int stat_show(struct seq_file *s, void *v)
> >   			   si->nr_wb_cp_data, si->nr_wb_data,
> >   			   si->nr_flushing, si->nr_flushed,
> >   			   si->flush_list_empty);
> > -		seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
> > -			   si->nr_discarding, si->nr_discarded,
> > -			   si->nr_discard_cmd, si->undiscard_blks);
> > +		seq_printf(s, "Discard: (%4d %4d, avg:%4lldns, peak:%4lldns)) cmd: %4d undiscard:%4u\n",
> > +			   si->nr_discarding, si->nr_discarded, ktime_to_us(si->discard_avg),
> > +			   ktime_to_us(si->discard_peak), si->nr_discard_cmd, si->undiscard_blks);
> >   		seq_printf(s, "  - atomic IO: %4d (Max. %4d)\n",
> >   			   si->aw_cnt, si->max_aw_cnt);
> >   		seq_printf(s, "  - compress: %4d, hit:%8d\n", si->compress_pages, si->compress_page_hit);
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index e8953c3dc81a..2cd55cb981ff 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -371,6 +371,8 @@ struct discard_cmd {
> >   	int error;			/* bio error */
> >   	spinlock_t lock;		/* for state/bio_ref updating */
> >   	unsigned short bio_ref;		/* bio reference count */
> > +	struct discard_cmd_control *dcc;	/* global discard cmd control */
> > +	ktime_t submit_start;	/* submit start time */
> >   };
> >   enum {
> > @@ -415,6 +417,9 @@ struct discard_cmd_control {
> >   	unsigned int max_ordered_discard;	/* maximum discard granularity issued by lba order */
> >   	unsigned int undiscard_blks;		/* # of undiscard blocks */
> >   	unsigned int next_pos;			/* next discard position */
> > +	spinlock_t discard_time_lock;	/* for discard time statistics */
> > +	ktime_t discard_time_avg;		/* issued discard cmd avg time */
> > +	ktime_t discard_time_peak;		/* issued discard cmd peak time */
> >   	atomic_t issued_discard;		/* # of issued discard */
> >   	atomic_t queued_discard;		/* # of queued discard */
> >   	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
> > @@ -3896,6 +3901,7 @@ struct f2fs_stat_info {
> >   	int nr_dio_read, nr_dio_write;
> >   	unsigned int io_skip_bggc, other_skip_bggc;
> >   	int nr_flushing, nr_flushed, flush_list_empty;
> > +	ktime_t discard_avg, discard_peak;
> >   	int nr_discarding, nr_discarded;
> >   	int nr_discard_cmd;
> >   	unsigned int undiscard_blks;
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index a9099a754dd2..73cd05bb3f4a 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -937,6 +937,7 @@ static struct discard_cmd *__create_discard_cmd(struct f2fs_sb_info *sbi,
> >   	list_add_tail(&dc->list, pend_list);
> >   	spin_lock_init(&dc->lock);
> >   	dc->bio_ref = 0;
> > +	dc->dcc = dcc;
> >   	atomic_inc(&dcc->discard_cmd_cnt);
> >   	dcc->undiscard_blks += len;
> > @@ -1006,9 +1007,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi,
> >   static void f2fs_submit_discard_endio(struct bio *bio)
> >   {
> >   	struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
> > +	struct discard_cmd_control *dcc = dc->dcc;
> >   	unsigned long flags;
> > +	ktime_t submit_time;
> > +	int nr_discarded;
> >   	spin_lock_irqsave(&dc->lock, flags);
> > +	submit_time = ktime_sub(ktime_get(), dc->submit_start);
> >   	if (!dc->error)
> >   		dc->error = blk_status_to_errno(bio->bi_status);
> >   	dc->bio_ref--;
> > @@ -1018,6 +1023,16 @@ static void f2fs_submit_discard_endio(struct bio *bio)
> >   	}
> >   	spin_unlock_irqrestore(&dc->lock, flags);
> >   	bio_put(bio);
> > +
> > +	spin_lock_irqsave(&dcc->discard_time_lock, flags);
> > +	nr_discarded = atomic_read(&dcc->issued_discard);
> > +	dcc->discard_time_avg = div_u64(ktime_add(nr_discarded * dcc->discard_time_avg,
> > +										submit_time),
> > +									nr_discarded + 1);
> > +	if (dcc->discard_time_avg > dcc->discard_time_peak)
> > +		dcc->discard_time_peak = dcc->discard_time_avg;
> > +	atomic_inc(&dcc->issued_discard);
> > +	spin_unlock_irqrestore(&dcc->discard_time_lock, flags);
> 
> Why not calculating average time only in update_general_status()? and here,
> we just need to account total_{discard_time, discard_count} w/o additional
> spinlock.
> 
> Thanks,
> 
> >   }
> >   static void __check_sit_bitmap(struct f2fs_sb_info *sbi,
> > @@ -1166,6 +1181,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> >   		 * right away
> >   		 */
> >   		spin_lock_irqsave(&dc->lock, flags);
> > +		dc->submit_start = ktime_get();
> >   		if (last)
> >   			dc->state = D_SUBMIT;
> >   		else
> > @@ -1185,8 +1201,6 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> >   		bio->bi_opf |= flag;
> >   		submit_bio(bio);
> > -		atomic_inc(&dcc->issued_discard);
> > -
> >   		f2fs_update_iostat(sbi, NULL, FS_DISCARD, len * F2FS_BLKSIZE);
> >   		lstart += len;
> > @@ -2079,9 +2093,12 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
> >   	INIT_LIST_HEAD(&dcc->wait_list);
> >   	INIT_LIST_HEAD(&dcc->fstrim_list);
> >   	mutex_init(&dcc->cmd_lock);
> > +	spin_lock_init(&dcc->discard_time_lock);
> >   	atomic_set(&dcc->issued_discard, 0);
> >   	atomic_set(&dcc->queued_discard, 0);
> >   	atomic_set(&dcc->discard_cmd_cnt, 0);
> > +	dcc->discard_time_avg = 0;
> > +	dcc->discard_time_peak = 0;
> >   	dcc->nr_discards = 0;
> >   	dcc->max_discards = MAIN_SEGS(sbi) << sbi->log_blocks_per_seg;
> >   	dcc->max_discard_request = DEF_MAX_DISCARD_REQUEST;


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] f2fs: add support for counting time of submit discard cmd
@ 2022-12-12 22:47     ` Jaegeuk Kim
  0 siblings, 0 replies; 20+ messages in thread
From: Jaegeuk Kim @ 2022-12-12 22:47 UTC (permalink / raw)
  To: Chao Yu; +Cc: Yangtao Li, linux-f2fs-devel, linux-kernel

On 12/12, Chao Yu wrote:
> On 2022/12/12 20:51, Yangtao Li wrote:
> > This patch adds support for counting the average time and
> > peak time of submit discard command, and we can see its
> > value in debugfs.
> > 
> > It is not sure whether the block layer has recorded these
> > data, and these data are allowed to be accessed by fs,
> > or they are only exported to user space.
> > 
> > On the one hand, I added these data to better understand
> > the current device operating status, and to further control
> > the discard process in a more detailed manner based on the
> > discard submit time in the future.
> 
> Again, w'd better to consider this functionality only when DEBUG_FS is
> enabled.

BTW, why can't we use iostat to get the discard latencies?

> 
> > 
> > Signed-off-by: Yangtao Li <frank.li@vivo.com>
> > ---
> >   fs/f2fs/debug.c   | 10 +++++++---
> >   fs/f2fs/f2fs.h    |  6 ++++++
> >   fs/f2fs/segment.c | 21 +++++++++++++++++++--
> >   3 files changed, 32 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> > index 32af4f0c5735..142c256b89d9 100644
> > --- a/fs/f2fs/debug.c
> > +++ b/fs/f2fs/debug.c
> > @@ -120,6 +120,10 @@ static void update_general_status(struct f2fs_sb_info *sbi)
> >   			llist_empty(&SM_I(sbi)->fcc_info->issue_list);
> >   	}
> >   	if (SM_I(sbi)->dcc_info) {
> > +		struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > +
> > +		si->discard_avg = dcc->discard_time_avg;
> > +		si->discard_peak = dcc->discard_time_peak;
> >   		si->nr_discarded =
> >   			atomic_read(&SM_I(sbi)->dcc_info->issued_discard);
> >   		si->nr_discarding =
> > @@ -545,9 +549,9 @@ static int stat_show(struct seq_file *s, void *v)
> >   			   si->nr_wb_cp_data, si->nr_wb_data,
> >   			   si->nr_flushing, si->nr_flushed,
> >   			   si->flush_list_empty);
> > -		seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
> > -			   si->nr_discarding, si->nr_discarded,
> > -			   si->nr_discard_cmd, si->undiscard_blks);
> > +		seq_printf(s, "Discard: (%4d %4d, avg:%4lldns, peak:%4lldns)) cmd: %4d undiscard:%4u\n",
> > +			   si->nr_discarding, si->nr_discarded, ktime_to_us(si->discard_avg),
> > +			   ktime_to_us(si->discard_peak), si->nr_discard_cmd, si->undiscard_blks);
> >   		seq_printf(s, "  - atomic IO: %4d (Max. %4d)\n",
> >   			   si->aw_cnt, si->max_aw_cnt);
> >   		seq_printf(s, "  - compress: %4d, hit:%8d\n", si->compress_pages, si->compress_page_hit);
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index e8953c3dc81a..2cd55cb981ff 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -371,6 +371,8 @@ struct discard_cmd {
> >   	int error;			/* bio error */
> >   	spinlock_t lock;		/* for state/bio_ref updating */
> >   	unsigned short bio_ref;		/* bio reference count */
> > +	struct discard_cmd_control *dcc;	/* global discard cmd control */
> > +	ktime_t submit_start;	/* submit start time */
> >   };
> >   enum {
> > @@ -415,6 +417,9 @@ struct discard_cmd_control {
> >   	unsigned int max_ordered_discard;	/* maximum discard granularity issued by lba order */
> >   	unsigned int undiscard_blks;		/* # of undiscard blocks */
> >   	unsigned int next_pos;			/* next discard position */
> > +	spinlock_t discard_time_lock;	/* for discard time statistics */
> > +	ktime_t discard_time_avg;		/* issued discard cmd avg time */
> > +	ktime_t discard_time_peak;		/* issued discard cmd peak time */
> >   	atomic_t issued_discard;		/* # of issued discard */
> >   	atomic_t queued_discard;		/* # of queued discard */
> >   	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
> > @@ -3896,6 +3901,7 @@ struct f2fs_stat_info {
> >   	int nr_dio_read, nr_dio_write;
> >   	unsigned int io_skip_bggc, other_skip_bggc;
> >   	int nr_flushing, nr_flushed, flush_list_empty;
> > +	ktime_t discard_avg, discard_peak;
> >   	int nr_discarding, nr_discarded;
> >   	int nr_discard_cmd;
> >   	unsigned int undiscard_blks;
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index a9099a754dd2..73cd05bb3f4a 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -937,6 +937,7 @@ static struct discard_cmd *__create_discard_cmd(struct f2fs_sb_info *sbi,
> >   	list_add_tail(&dc->list, pend_list);
> >   	spin_lock_init(&dc->lock);
> >   	dc->bio_ref = 0;
> > +	dc->dcc = dcc;
> >   	atomic_inc(&dcc->discard_cmd_cnt);
> >   	dcc->undiscard_blks += len;
> > @@ -1006,9 +1007,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi,
> >   static void f2fs_submit_discard_endio(struct bio *bio)
> >   {
> >   	struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
> > +	struct discard_cmd_control *dcc = dc->dcc;
> >   	unsigned long flags;
> > +	ktime_t submit_time;
> > +	int nr_discarded;
> >   	spin_lock_irqsave(&dc->lock, flags);
> > +	submit_time = ktime_sub(ktime_get(), dc->submit_start);
> >   	if (!dc->error)
> >   		dc->error = blk_status_to_errno(bio->bi_status);
> >   	dc->bio_ref--;
> > @@ -1018,6 +1023,16 @@ static void f2fs_submit_discard_endio(struct bio *bio)
> >   	}
> >   	spin_unlock_irqrestore(&dc->lock, flags);
> >   	bio_put(bio);
> > +
> > +	spin_lock_irqsave(&dcc->discard_time_lock, flags);
> > +	nr_discarded = atomic_read(&dcc->issued_discard);
> > +	dcc->discard_time_avg = div_u64(ktime_add(nr_discarded * dcc->discard_time_avg,
> > +										submit_time),
> > +									nr_discarded + 1);
> > +	if (dcc->discard_time_avg > dcc->discard_time_peak)
> > +		dcc->discard_time_peak = dcc->discard_time_avg;
> > +	atomic_inc(&dcc->issued_discard);
> > +	spin_unlock_irqrestore(&dcc->discard_time_lock, flags);
> 
> Why not calculating average time only in update_general_status()? and here,
> we just need to account total_{discard_time, discard_count} w/o additional
> spinlock.
> 
> Thanks,
> 
> >   }
> >   static void __check_sit_bitmap(struct f2fs_sb_info *sbi,
> > @@ -1166,6 +1181,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> >   		 * right away
> >   		 */
> >   		spin_lock_irqsave(&dc->lock, flags);
> > +		dc->submit_start = ktime_get();
> >   		if (last)
> >   			dc->state = D_SUBMIT;
> >   		else
> > @@ -1185,8 +1201,6 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> >   		bio->bi_opf |= flag;
> >   		submit_bio(bio);
> > -		atomic_inc(&dcc->issued_discard);
> > -
> >   		f2fs_update_iostat(sbi, NULL, FS_DISCARD, len * F2FS_BLKSIZE);
> >   		lstart += len;
> > @@ -2079,9 +2093,12 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
> >   	INIT_LIST_HEAD(&dcc->wait_list);
> >   	INIT_LIST_HEAD(&dcc->fstrim_list);
> >   	mutex_init(&dcc->cmd_lock);
> > +	spin_lock_init(&dcc->discard_time_lock);
> >   	atomic_set(&dcc->issued_discard, 0);
> >   	atomic_set(&dcc->queued_discard, 0);
> >   	atomic_set(&dcc->discard_cmd_cnt, 0);
> > +	dcc->discard_time_avg = 0;
> > +	dcc->discard_time_peak = 0;
> >   	dcc->nr_discards = 0;
> >   	dcc->max_discards = MAIN_SEGS(sbi) << sbi->log_blocks_per_seg;
> >   	dcc->max_discard_request = DEF_MAX_DISCARD_REQUEST;

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] f2fs: add support for counting time of submit discard cmd
  2022-12-12 22:47     ` Jaegeuk Kim
@ 2022-12-13  1:22       ` Chao Yu
  -1 siblings, 0 replies; 20+ messages in thread
From: Chao Yu @ 2022-12-13  1:22 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: Yangtao Li, linux-f2fs-devel, linux-kernel

On 2022/12/13 6:47, Jaegeuk Kim wrote:
> On 12/12, Chao Yu wrote:
>> On 2022/12/12 20:51, Yangtao Li wrote:
>>> This patch adds support for counting the average time and
>>> peak time of submit discard command, and we can see its
>>> value in debugfs.
>>>
>>> It is not sure whether the block layer has recorded these
>>> data, and these data are allowed to be accessed by fs,
>>> or they are only exported to user space.
>>>
>>> On the one hand, I added these data to better understand
>>> the current device operating status, and to further control
>>> the discard process in a more detailed manner based on the
>>> discard submit time in the future.
>>
>> Again, w'd better to consider this functionality only when DEBUG_FS is
>> enabled.
> 
> BTW, why can't we use iostat to get the discard latencies?

Agreed.

Thanks,

> 
>>
>>>
>>> Signed-off-by: Yangtao Li <frank.li@vivo.com>
>>> ---
>>>    fs/f2fs/debug.c   | 10 +++++++---
>>>    fs/f2fs/f2fs.h    |  6 ++++++
>>>    fs/f2fs/segment.c | 21 +++++++++++++++++++--
>>>    3 files changed, 32 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
>>> index 32af4f0c5735..142c256b89d9 100644
>>> --- a/fs/f2fs/debug.c
>>> +++ b/fs/f2fs/debug.c
>>> @@ -120,6 +120,10 @@ static void update_general_status(struct f2fs_sb_info *sbi)
>>>    			llist_empty(&SM_I(sbi)->fcc_info->issue_list);
>>>    	}
>>>    	if (SM_I(sbi)->dcc_info) {
>>> +		struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
>>> +
>>> +		si->discard_avg = dcc->discard_time_avg;
>>> +		si->discard_peak = dcc->discard_time_peak;
>>>    		si->nr_discarded =
>>>    			atomic_read(&SM_I(sbi)->dcc_info->issued_discard);
>>>    		si->nr_discarding =
>>> @@ -545,9 +549,9 @@ static int stat_show(struct seq_file *s, void *v)
>>>    			   si->nr_wb_cp_data, si->nr_wb_data,
>>>    			   si->nr_flushing, si->nr_flushed,
>>>    			   si->flush_list_empty);
>>> -		seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
>>> -			   si->nr_discarding, si->nr_discarded,
>>> -			   si->nr_discard_cmd, si->undiscard_blks);
>>> +		seq_printf(s, "Discard: (%4d %4d, avg:%4lldns, peak:%4lldns)) cmd: %4d undiscard:%4u\n",
>>> +			   si->nr_discarding, si->nr_discarded, ktime_to_us(si->discard_avg),
>>> +			   ktime_to_us(si->discard_peak), si->nr_discard_cmd, si->undiscard_blks);
>>>    		seq_printf(s, "  - atomic IO: %4d (Max. %4d)\n",
>>>    			   si->aw_cnt, si->max_aw_cnt);
>>>    		seq_printf(s, "  - compress: %4d, hit:%8d\n", si->compress_pages, si->compress_page_hit);
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index e8953c3dc81a..2cd55cb981ff 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -371,6 +371,8 @@ struct discard_cmd {
>>>    	int error;			/* bio error */
>>>    	spinlock_t lock;		/* for state/bio_ref updating */
>>>    	unsigned short bio_ref;		/* bio reference count */
>>> +	struct discard_cmd_control *dcc;	/* global discard cmd control */
>>> +	ktime_t submit_start;	/* submit start time */
>>>    };
>>>    enum {
>>> @@ -415,6 +417,9 @@ struct discard_cmd_control {
>>>    	unsigned int max_ordered_discard;	/* maximum discard granularity issued by lba order */
>>>    	unsigned int undiscard_blks;		/* # of undiscard blocks */
>>>    	unsigned int next_pos;			/* next discard position */
>>> +	spinlock_t discard_time_lock;	/* for discard time statistics */
>>> +	ktime_t discard_time_avg;		/* issued discard cmd avg time */
>>> +	ktime_t discard_time_peak;		/* issued discard cmd peak time */
>>>    	atomic_t issued_discard;		/* # of issued discard */
>>>    	atomic_t queued_discard;		/* # of queued discard */
>>>    	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
>>> @@ -3896,6 +3901,7 @@ struct f2fs_stat_info {
>>>    	int nr_dio_read, nr_dio_write;
>>>    	unsigned int io_skip_bggc, other_skip_bggc;
>>>    	int nr_flushing, nr_flushed, flush_list_empty;
>>> +	ktime_t discard_avg, discard_peak;
>>>    	int nr_discarding, nr_discarded;
>>>    	int nr_discard_cmd;
>>>    	unsigned int undiscard_blks;
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index a9099a754dd2..73cd05bb3f4a 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -937,6 +937,7 @@ static struct discard_cmd *__create_discard_cmd(struct f2fs_sb_info *sbi,
>>>    	list_add_tail(&dc->list, pend_list);
>>>    	spin_lock_init(&dc->lock);
>>>    	dc->bio_ref = 0;
>>> +	dc->dcc = dcc;
>>>    	atomic_inc(&dcc->discard_cmd_cnt);
>>>    	dcc->undiscard_blks += len;
>>> @@ -1006,9 +1007,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi,
>>>    static void f2fs_submit_discard_endio(struct bio *bio)
>>>    {
>>>    	struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
>>> +	struct discard_cmd_control *dcc = dc->dcc;
>>>    	unsigned long flags;
>>> +	ktime_t submit_time;
>>> +	int nr_discarded;
>>>    	spin_lock_irqsave(&dc->lock, flags);
>>> +	submit_time = ktime_sub(ktime_get(), dc->submit_start);
>>>    	if (!dc->error)
>>>    		dc->error = blk_status_to_errno(bio->bi_status);
>>>    	dc->bio_ref--;
>>> @@ -1018,6 +1023,16 @@ static void f2fs_submit_discard_endio(struct bio *bio)
>>>    	}
>>>    	spin_unlock_irqrestore(&dc->lock, flags);
>>>    	bio_put(bio);
>>> +
>>> +	spin_lock_irqsave(&dcc->discard_time_lock, flags);
>>> +	nr_discarded = atomic_read(&dcc->issued_discard);
>>> +	dcc->discard_time_avg = div_u64(ktime_add(nr_discarded * dcc->discard_time_avg,
>>> +										submit_time),
>>> +									nr_discarded + 1);
>>> +	if (dcc->discard_time_avg > dcc->discard_time_peak)
>>> +		dcc->discard_time_peak = dcc->discard_time_avg;
>>> +	atomic_inc(&dcc->issued_discard);
>>> +	spin_unlock_irqrestore(&dcc->discard_time_lock, flags);
>>
>> Why not calculating average time only in update_general_status()? and here,
>> we just need to account total_{discard_time, discard_count} w/o additional
>> spinlock.
>>
>> Thanks,
>>
>>>    }
>>>    static void __check_sit_bitmap(struct f2fs_sb_info *sbi,
>>> @@ -1166,6 +1181,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
>>>    		 * right away
>>>    		 */
>>>    		spin_lock_irqsave(&dc->lock, flags);
>>> +		dc->submit_start = ktime_get();
>>>    		if (last)
>>>    			dc->state = D_SUBMIT;
>>>    		else
>>> @@ -1185,8 +1201,6 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
>>>    		bio->bi_opf |= flag;
>>>    		submit_bio(bio);
>>> -		atomic_inc(&dcc->issued_discard);
>>> -
>>>    		f2fs_update_iostat(sbi, NULL, FS_DISCARD, len * F2FS_BLKSIZE);
>>>    		lstart += len;
>>> @@ -2079,9 +2093,12 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
>>>    	INIT_LIST_HEAD(&dcc->wait_list);
>>>    	INIT_LIST_HEAD(&dcc->fstrim_list);
>>>    	mutex_init(&dcc->cmd_lock);
>>> +	spin_lock_init(&dcc->discard_time_lock);
>>>    	atomic_set(&dcc->issued_discard, 0);
>>>    	atomic_set(&dcc->queued_discard, 0);
>>>    	atomic_set(&dcc->discard_cmd_cnt, 0);
>>> +	dcc->discard_time_avg = 0;
>>> +	dcc->discard_time_peak = 0;
>>>    	dcc->nr_discards = 0;
>>>    	dcc->max_discards = MAIN_SEGS(sbi) << sbi->log_blocks_per_seg;
>>>    	dcc->max_discard_request = DEF_MAX_DISCARD_REQUEST;

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
@ 2022-12-13  1:22       ` Chao Yu
  0 siblings, 0 replies; 20+ messages in thread
From: Chao Yu @ 2022-12-13  1:22 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, Yangtao Li

On 2022/12/13 6:47, Jaegeuk Kim wrote:
> On 12/12, Chao Yu wrote:
>> On 2022/12/12 20:51, Yangtao Li wrote:
>>> This patch adds support for counting the average time and
>>> peak time of submit discard command, and we can see its
>>> value in debugfs.
>>>
>>> It is not sure whether the block layer has recorded these
>>> data, and these data are allowed to be accessed by fs,
>>> or they are only exported to user space.
>>>
>>> On the one hand, I added these data to better understand
>>> the current device operating status, and to further control
>>> the discard process in a more detailed manner based on the
>>> discard submit time in the future.
>>
>> Again, w'd better to consider this functionality only when DEBUG_FS is
>> enabled.
> 
> BTW, why can't we use iostat to get the discard latencies?

Agreed.

Thanks,

> 
>>
>>>
>>> Signed-off-by: Yangtao Li <frank.li@vivo.com>
>>> ---
>>>    fs/f2fs/debug.c   | 10 +++++++---
>>>    fs/f2fs/f2fs.h    |  6 ++++++
>>>    fs/f2fs/segment.c | 21 +++++++++++++++++++--
>>>    3 files changed, 32 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
>>> index 32af4f0c5735..142c256b89d9 100644
>>> --- a/fs/f2fs/debug.c
>>> +++ b/fs/f2fs/debug.c
>>> @@ -120,6 +120,10 @@ static void update_general_status(struct f2fs_sb_info *sbi)
>>>    			llist_empty(&SM_I(sbi)->fcc_info->issue_list);
>>>    	}
>>>    	if (SM_I(sbi)->dcc_info) {
>>> +		struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
>>> +
>>> +		si->discard_avg = dcc->discard_time_avg;
>>> +		si->discard_peak = dcc->discard_time_peak;
>>>    		si->nr_discarded =
>>>    			atomic_read(&SM_I(sbi)->dcc_info->issued_discard);
>>>    		si->nr_discarding =
>>> @@ -545,9 +549,9 @@ static int stat_show(struct seq_file *s, void *v)
>>>    			   si->nr_wb_cp_data, si->nr_wb_data,
>>>    			   si->nr_flushing, si->nr_flushed,
>>>    			   si->flush_list_empty);
>>> -		seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
>>> -			   si->nr_discarding, si->nr_discarded,
>>> -			   si->nr_discard_cmd, si->undiscard_blks);
>>> +		seq_printf(s, "Discard: (%4d %4d, avg:%4lldns, peak:%4lldns)) cmd: %4d undiscard:%4u\n",
>>> +			   si->nr_discarding, si->nr_discarded, ktime_to_us(si->discard_avg),
>>> +			   ktime_to_us(si->discard_peak), si->nr_discard_cmd, si->undiscard_blks);
>>>    		seq_printf(s, "  - atomic IO: %4d (Max. %4d)\n",
>>>    			   si->aw_cnt, si->max_aw_cnt);
>>>    		seq_printf(s, "  - compress: %4d, hit:%8d\n", si->compress_pages, si->compress_page_hit);
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index e8953c3dc81a..2cd55cb981ff 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -371,6 +371,8 @@ struct discard_cmd {
>>>    	int error;			/* bio error */
>>>    	spinlock_t lock;		/* for state/bio_ref updating */
>>>    	unsigned short bio_ref;		/* bio reference count */
>>> +	struct discard_cmd_control *dcc;	/* global discard cmd control */
>>> +	ktime_t submit_start;	/* submit start time */
>>>    };
>>>    enum {
>>> @@ -415,6 +417,9 @@ struct discard_cmd_control {
>>>    	unsigned int max_ordered_discard;	/* maximum discard granularity issued by lba order */
>>>    	unsigned int undiscard_blks;		/* # of undiscard blocks */
>>>    	unsigned int next_pos;			/* next discard position */
>>> +	spinlock_t discard_time_lock;	/* for discard time statistics */
>>> +	ktime_t discard_time_avg;		/* issued discard cmd avg time */
>>> +	ktime_t discard_time_peak;		/* issued discard cmd peak time */
>>>    	atomic_t issued_discard;		/* # of issued discard */
>>>    	atomic_t queued_discard;		/* # of queued discard */
>>>    	atomic_t discard_cmd_cnt;		/* # of cached cmd count */
>>> @@ -3896,6 +3901,7 @@ struct f2fs_stat_info {
>>>    	int nr_dio_read, nr_dio_write;
>>>    	unsigned int io_skip_bggc, other_skip_bggc;
>>>    	int nr_flushing, nr_flushed, flush_list_empty;
>>> +	ktime_t discard_avg, discard_peak;
>>>    	int nr_discarding, nr_discarded;
>>>    	int nr_discard_cmd;
>>>    	unsigned int undiscard_blks;
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index a9099a754dd2..73cd05bb3f4a 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -937,6 +937,7 @@ static struct discard_cmd *__create_discard_cmd(struct f2fs_sb_info *sbi,
>>>    	list_add_tail(&dc->list, pend_list);
>>>    	spin_lock_init(&dc->lock);
>>>    	dc->bio_ref = 0;
>>> +	dc->dcc = dcc;
>>>    	atomic_inc(&dcc->discard_cmd_cnt);
>>>    	dcc->undiscard_blks += len;
>>> @@ -1006,9 +1007,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi,
>>>    static void f2fs_submit_discard_endio(struct bio *bio)
>>>    {
>>>    	struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
>>> +	struct discard_cmd_control *dcc = dc->dcc;
>>>    	unsigned long flags;
>>> +	ktime_t submit_time;
>>> +	int nr_discarded;
>>>    	spin_lock_irqsave(&dc->lock, flags);
>>> +	submit_time = ktime_sub(ktime_get(), dc->submit_start);
>>>    	if (!dc->error)
>>>    		dc->error = blk_status_to_errno(bio->bi_status);
>>>    	dc->bio_ref--;
>>> @@ -1018,6 +1023,16 @@ static void f2fs_submit_discard_endio(struct bio *bio)
>>>    	}
>>>    	spin_unlock_irqrestore(&dc->lock, flags);
>>>    	bio_put(bio);
>>> +
>>> +	spin_lock_irqsave(&dcc->discard_time_lock, flags);
>>> +	nr_discarded = atomic_read(&dcc->issued_discard);
>>> +	dcc->discard_time_avg = div_u64(ktime_add(nr_discarded * dcc->discard_time_avg,
>>> +										submit_time),
>>> +									nr_discarded + 1);
>>> +	if (dcc->discard_time_avg > dcc->discard_time_peak)
>>> +		dcc->discard_time_peak = dcc->discard_time_avg;
>>> +	atomic_inc(&dcc->issued_discard);
>>> +	spin_unlock_irqrestore(&dcc->discard_time_lock, flags);
>>
>> Why not calculating average time only in update_general_status()? and here,
>> we just need to account total_{discard_time, discard_count} w/o additional
>> spinlock.
>>
>> Thanks,
>>
>>>    }
>>>    static void __check_sit_bitmap(struct f2fs_sb_info *sbi,
>>> @@ -1166,6 +1181,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
>>>    		 * right away
>>>    		 */
>>>    		spin_lock_irqsave(&dc->lock, flags);
>>> +		dc->submit_start = ktime_get();
>>>    		if (last)
>>>    			dc->state = D_SUBMIT;
>>>    		else
>>> @@ -1185,8 +1201,6 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
>>>    		bio->bi_opf |= flag;
>>>    		submit_bio(bio);
>>> -		atomic_inc(&dcc->issued_discard);
>>> -
>>>    		f2fs_update_iostat(sbi, NULL, FS_DISCARD, len * F2FS_BLKSIZE);
>>>    		lstart += len;
>>> @@ -2079,9 +2093,12 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
>>>    	INIT_LIST_HEAD(&dcc->wait_list);
>>>    	INIT_LIST_HEAD(&dcc->fstrim_list);
>>>    	mutex_init(&dcc->cmd_lock);
>>> +	spin_lock_init(&dcc->discard_time_lock);
>>>    	atomic_set(&dcc->issued_discard, 0);
>>>    	atomic_set(&dcc->queued_discard, 0);
>>>    	atomic_set(&dcc->discard_cmd_cnt, 0);
>>> +	dcc->discard_time_avg = 0;
>>> +	dcc->discard_time_peak = 0;
>>>    	dcc->nr_discards = 0;
>>>    	dcc->max_discards = MAIN_SEGS(sbi) << sbi->log_blocks_per_seg;
>>>    	dcc->max_discard_request = DEF_MAX_DISCARD_REQUEST;


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] f2fs: add support for counting time of submit discard cmd
  2022-12-13  1:22       ` [f2fs-dev] " Chao Yu
@ 2022-12-13 12:21         ` Yangtao Li via Linux-f2fs-devel
  -1 siblings, 0 replies; 20+ messages in thread
From: Yangtao Li @ 2022-12-13 12:21 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: linux-f2fs-devel, linux-kernel

Hi Jaegeuk,

>>> Again, w'd better to consider this functionality only when DEBUG_FS 
>>> is enabled.
>> 
>> BTW, why can't we use iostat to get the discard latencies?
> 
> Agreed.

Let me spend some time on this. So, I guess this patch can't catch up with the merge window.
And I still have some patches that have not been picked, can you take a look, hope they can
catch up with the window.

How long is the 6.2 merge window left and when will you send the f2fs 6.2 pull request?

Thx,
Yangtao

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
@ 2022-12-13 12:21         ` Yangtao Li via Linux-f2fs-devel
  0 siblings, 0 replies; 20+ messages in thread
From: Yangtao Li via Linux-f2fs-devel @ 2022-12-13 12:21 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: linux-kernel, linux-f2fs-devel

Hi Jaegeuk,

>>> Again, w'd better to consider this functionality only when DEBUG_FS 
>>> is enabled.
>> 
>> BTW, why can't we use iostat to get the discard latencies?
> 
> Agreed.

Let me spend some time on this. So, I guess this patch can't catch up with the merge window.
And I still have some patches that have not been picked, can you take a look, hope they can
catch up with the window.

How long is the 6.2 merge window left and when will you send the f2fs 6.2 pull request?

Thx,
Yangtao


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] f2fs: add support for counting time of submit discard cmd
  2022-12-13 12:21         ` [f2fs-dev] " Yangtao Li via Linux-f2fs-devel
@ 2022-12-13 19:17           ` Jaegeuk Kim
  -1 siblings, 0 replies; 20+ messages in thread
From: Jaegeuk Kim @ 2022-12-13 19:17 UTC (permalink / raw)
  To: Yangtao Li; +Cc: chao, linux-f2fs-devel, linux-kernel

On 12/13, Yangtao Li wrote:
> Hi Jaegeuk,
> 
> >>> Again, w'd better to consider this functionality only when DEBUG_FS 
> >>> is enabled.
> >> 
> >> BTW, why can't we use iostat to get the discard latencies?
> > 
> > Agreed.
> 
> Let me spend some time on this. So, I guess this patch can't catch up with the merge window.
> And I still have some patches that have not been picked, can you take a look, hope they can
> catch up with the window.
> 
> How long is the 6.2 merge window left and when will you send the f2fs 6.2 pull request?

I cut off the patches for this merge window. Please consider next release.
BTW, could you please send a patch set instead of random posts? It's quite hard
to find which one was merged or not.

Thanks,

> 
> Thx,
> Yangtao

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
@ 2022-12-13 19:17           ` Jaegeuk Kim
  0 siblings, 0 replies; 20+ messages in thread
From: Jaegeuk Kim @ 2022-12-13 19:17 UTC (permalink / raw)
  To: Yangtao Li; +Cc: linux-kernel, linux-f2fs-devel

On 12/13, Yangtao Li wrote:
> Hi Jaegeuk,
> 
> >>> Again, w'd better to consider this functionality only when DEBUG_FS 
> >>> is enabled.
> >> 
> >> BTW, why can't we use iostat to get the discard latencies?
> > 
> > Agreed.
> 
> Let me spend some time on this. So, I guess this patch can't catch up with the merge window.
> And I still have some patches that have not been picked, can you take a look, hope they can
> catch up with the window.
> 
> How long is the 6.2 merge window left and when will you send the f2fs 6.2 pull request?

I cut off the patches for this merge window. Please consider next release.
BTW, could you please send a patch set instead of random posts? It's quite hard
to find which one was merged or not.

Thanks,

> 
> Thx,
> Yangtao


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] f2fs: add support for counting time of submit discard cmd
  2022-12-13 19:17           ` [f2fs-dev] " Jaegeuk Kim
@ 2022-12-14  7:28             ` Yangtao Li via Linux-f2fs-devel
  -1 siblings, 0 replies; 20+ messages in thread
From: Yangtao Li @ 2022-12-14  7:28 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: linux-f2fs-devel, linux-kernel

Hi Jaegeuk,

> I cut off the patches for this merge window. Please consider next release.

Alright, thanks for your reminder.

> BTW, could you please send a patch set instead of random posts?

Most of the patches were noticed when I looked at the code, and they were scattered.
On the one hand, there is not much relationship between them, so I send patches based
on the f2fs-dev branch every time. On the other hand, it is also to avoid that a patch
may not be received and block subsequent patches.

> It's quite hard to find which one was merged or not.

Why not to use patchwork to manage patches?
This tool is used by many kernel subsystems. And This tool has a more friendly
interface and can mark the status of the patch.

https://korg.wiki.kernel.org/userdoc/patchwork#adding_patchwork-bot_integration

Thx,
Yangtao

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
@ 2022-12-14  7:28             ` Yangtao Li via Linux-f2fs-devel
  0 siblings, 0 replies; 20+ messages in thread
From: Yangtao Li via Linux-f2fs-devel @ 2022-12-14  7:28 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: linux-kernel, linux-f2fs-devel

Hi Jaegeuk,

> I cut off the patches for this merge window. Please consider next release.

Alright, thanks for your reminder.

> BTW, could you please send a patch set instead of random posts?

Most of the patches were noticed when I looked at the code, and they were scattered.
On the one hand, there is not much relationship between them, so I send patches based
on the f2fs-dev branch every time. On the other hand, it is also to avoid that a patch
may not be received and block subsequent patches.

> It's quite hard to find which one was merged or not.

Why not to use patchwork to manage patches?
This tool is used by many kernel subsystems. And This tool has a more friendly
interface and can mark the status of the patch.

https://korg.wiki.kernel.org/userdoc/patchwork#adding_patchwork-bot_integration

Thx,
Yangtao


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] f2fs: add support for counting time of submit discard cmd
  2022-12-14  7:28             ` [f2fs-dev] " Yangtao Li via Linux-f2fs-devel
@ 2022-12-14 21:52               ` Jaegeuk Kim
  -1 siblings, 0 replies; 20+ messages in thread
From: Jaegeuk Kim @ 2022-12-14 21:52 UTC (permalink / raw)
  To: Yangtao Li; +Cc: chao, linux-f2fs-devel, linux-kernel

On 12/14, Yangtao Li wrote:
> Hi Jaegeuk,
> 
> > I cut off the patches for this merge window. Please consider next release.
> 
> Alright, thanks for your reminder.
> 
> > BTW, could you please send a patch set instead of random posts?
> 
> Most of the patches were noticed when I looked at the code, and they were scattered.
> On the one hand, there is not much relationship between them, so I send patches based
> on the f2fs-dev branch every time. On the other hand, it is also to avoid that a patch
> may not be received and block subsequent patches.
> 
> > It's quite hard to find which one was merged or not.
> 
> Why not to use patchwork to manage patches?
> This tool is used by many kernel subsystems. And This tool has a more friendly
> interface and can mark the status of the patch.
> 
> https://korg.wiki.kernel.org/userdoc/patchwork#adding_patchwork-bot_integration

Good idea. I requested to add f2fs project there. :) Let's see.

> 
> Thx,
> Yangtao

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
@ 2022-12-14 21:52               ` Jaegeuk Kim
  0 siblings, 0 replies; 20+ messages in thread
From: Jaegeuk Kim @ 2022-12-14 21:52 UTC (permalink / raw)
  To: Yangtao Li; +Cc: linux-kernel, linux-f2fs-devel

On 12/14, Yangtao Li wrote:
> Hi Jaegeuk,
> 
> > I cut off the patches for this merge window. Please consider next release.
> 
> Alright, thanks for your reminder.
> 
> > BTW, could you please send a patch set instead of random posts?
> 
> Most of the patches were noticed when I looked at the code, and they were scattered.
> On the one hand, there is not much relationship between them, so I send patches based
> on the f2fs-dev branch every time. On the other hand, it is also to avoid that a patch
> may not be received and block subsequent patches.
> 
> > It's quite hard to find which one was merged or not.
> 
> Why not to use patchwork to manage patches?
> This tool is used by many kernel subsystems. And This tool has a more friendly
> interface and can mark the status of the patch.
> 
> https://korg.wiki.kernel.org/userdoc/patchwork#adding_patchwork-bot_integration

Good idea. I requested to add f2fs project there. :) Let's see.

> 
> Thx,
> Yangtao


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] f2fs: add support for counting time of submit discard cmd
  2022-12-14 21:52               ` [f2fs-dev] " Jaegeuk Kim
@ 2022-12-16  1:04                 ` Chao Yu
  -1 siblings, 0 replies; 20+ messages in thread
From: Chao Yu @ 2022-12-16  1:04 UTC (permalink / raw)
  To: Jaegeuk Kim, Yangtao Li; +Cc: linux-f2fs-devel, linux-kernel

On 2022/12/15 5:52, Jaegeuk Kim wrote:
>> https://korg.wiki.kernel.org/userdoc/patchwork#adding_patchwork-bot_integration
> 
> Good idea. I requested to add f2fs project there. :) Let's see.

The website is available now, cool... :)

https://patchwork.kernel.org/project/f2fs/

Jaegeuk, your email address is out-of-update in above link, it needs to
be updated.

Maintainer	Jaegeuk Kim <jaegeuk.kim@samsung.com>

Thanks,

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
@ 2022-12-16  1:04                 ` Chao Yu
  0 siblings, 0 replies; 20+ messages in thread
From: Chao Yu @ 2022-12-16  1:04 UTC (permalink / raw)
  To: Jaegeuk Kim, Yangtao Li; +Cc: linux-kernel, linux-f2fs-devel

On 2022/12/15 5:52, Jaegeuk Kim wrote:
>> https://korg.wiki.kernel.org/userdoc/patchwork#adding_patchwork-bot_integration
> 
> Good idea. I requested to add f2fs project there. :) Let's see.

The website is available now, cool... :)

https://patchwork.kernel.org/project/f2fs/

Jaegeuk, your email address is out-of-update in above link, it needs to
be updated.

Maintainer	Jaegeuk Kim <jaegeuk.kim@samsung.com>

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
  2022-12-16  1:04                 ` [f2fs-dev] " Chao Yu
@ 2022-12-16  2:49                   ` Yangtao Li via Linux-f2fs-devel
  -1 siblings, 0 replies; 20+ messages in thread
From: Yangtao Li @ 2022-12-16  2:49 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: linux-f2fs-devel, linux-kernel

> The website is available now, cool... :)
> 
> https://patchwork.kernel.org/project/f2fs/

The website link is accessible, but it seems like it still needs some work?
According to my test, the patches from last night and today are not visible on the website.

> Jaegeuk, your email address is out-of-update in above link, it needs to
> be updated.

> Maintainer	Jaegeuk Kim <jaegeuk.kim@samsung.com>

Thx,
Yangtao

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
@ 2022-12-16  2:49                   ` Yangtao Li via Linux-f2fs-devel
  0 siblings, 0 replies; 20+ messages in thread
From: Yangtao Li via Linux-f2fs-devel @ 2022-12-16  2:49 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: linux-kernel, linux-f2fs-devel

> The website is available now, cool... :)
> 
> https://patchwork.kernel.org/project/f2fs/

The website link is accessible, but it seems like it still needs some work?
According to my test, the patches from last night and today are not visible on the website.

> Jaegeuk, your email address is out-of-update in above link, it needs to
> be updated.

> Maintainer	Jaegeuk Kim <jaegeuk.kim@samsung.com>

Thx,
Yangtao


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-12-16  2:50 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-12 12:51 [PATCH v2] f2fs: add support for counting time of submit discard cmd Yangtao Li
2022-12-12 12:51 ` [f2fs-dev] " Yangtao Li via Linux-f2fs-devel
2022-12-12 13:40 ` Chao Yu
2022-12-12 13:40   ` [f2fs-dev] " Chao Yu
2022-12-12 22:47   ` Jaegeuk Kim
2022-12-12 22:47     ` Jaegeuk Kim
2022-12-13  1:22     ` Chao Yu
2022-12-13  1:22       ` [f2fs-dev] " Chao Yu
2022-12-13 12:21       ` Yangtao Li
2022-12-13 12:21         ` [f2fs-dev] " Yangtao Li via Linux-f2fs-devel
2022-12-13 19:17         ` Jaegeuk Kim
2022-12-13 19:17           ` [f2fs-dev] " Jaegeuk Kim
2022-12-14  7:28           ` Yangtao Li
2022-12-14  7:28             ` [f2fs-dev] " Yangtao Li via Linux-f2fs-devel
2022-12-14 21:52             ` Jaegeuk Kim
2022-12-14 21:52               ` [f2fs-dev] " Jaegeuk Kim
2022-12-16  1:04               ` Chao Yu
2022-12-16  1:04                 ` [f2fs-dev] " Chao Yu
2022-12-16  2:49                 ` Yangtao Li
2022-12-16  2:49                   ` Yangtao Li via Linux-f2fs-devel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.