* [PATCH v6] f2fs: add async reset zone command support
[not found] <CGME20230508081042epcms2p8a637deae7de1829f54614e09d5fde5e5@epcms2p8>
@ 2023-05-08 8:10 ` Daejun Park
0 siblings, 0 replies; 18+ messages in thread
From: Daejun Park @ 2023-05-08 8:10 UTC (permalink / raw)
To: jaegeuk, chao, rostedt, mhiramat, linux-f2fs-devel, linux-kernel,
linux-trace-kernel
Cc: Seokhwan Kim, Yonggil Song, beomsu kim, Daejun Park
v5 -> v6
Added trace_f2fs_iostat support for zone reset command.
v4 -> v5
Added f2fs iostat for zone reset command.
v3 -> v4
Fixed build error caused by unused function.
v2 -> v3
Modified arguments to be correct for ftrace parameter.
Changed __submit_zone_reset_cmd to void return.
Refactored the f2fs_wait_discard_bio function.
Fixed code that was previously incorrectly merged.
v1 -> v2
Changed to apply the optional async reset write pointer by default.
This patch enables submit reset zone command asynchornously. It helps
decrease average latency of write IOs in high utilization scenario by
faster checkpointing.
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
fs/f2fs/f2fs.h | 1 +
fs/f2fs/iostat.c | 1 +
fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
include/trace/events/f2fs.h | 24 +++++++++--
4 files changed, 104 insertions(+), 6 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index d211ee89c158..51b68a629814 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1175,6 +1175,7 @@ enum iostat_type {
/* other */
FS_DISCARD_IO, /* discard */
FS_FLUSH_IO, /* flush */
+ FS_ZONE_RESET_IO, /* zone reset */
NR_IO_TYPE,
};
diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
index 3d5bfb1ad585..f8703038e1d8 100644
--- a/fs/f2fs/iostat.c
+++ b/fs/f2fs/iostat.c
@@ -80,6 +80,7 @@ int __maybe_unused iostat_info_seq_show(struct seq_file *seq, void *offset)
seq_puts(seq, "[OTHER]\n");
IOSTAT_INFO_SHOW("fs discard", FS_DISCARD_IO);
IOSTAT_INFO_SHOW("fs flush", FS_FLUSH_IO);
+ IOSTAT_INFO_SHOW("fs zone reset", FS_ZONE_RESET_IO);
return 0;
}
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 6db410f1bb8c..4802b05a795b 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1196,6 +1196,45 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
struct block_device *bdev, block_t lstart,
block_t start, block_t len);
+
+#ifdef CONFIG_BLK_DEV_ZONED
+static void __submit_zone_reset_cmd(struct f2fs_sb_info *sbi,
+ struct discard_cmd *dc, blk_opf_t flag,
+ struct list_head *wait_list,
+ unsigned int *issued)
+{
+ struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
+ struct block_device *bdev = dc->bdev;
+ struct bio *bio = bio_alloc(bdev, 0, REQ_OP_ZONE_RESET | flag, GFP_NOFS);
+ unsigned long flags;
+
+ trace_f2fs_issue_reset_zone(bdev, dc->di.start);
+
+ spin_lock_irqsave(&dc->lock, flags);
+ dc->state = D_SUBMIT;
+ dc->bio_ref++;
+ spin_unlock_irqrestore(&dc->lock, flags);
+
+ if (issued)
+ (*issued)++;
+
+ atomic_inc(&dcc->queued_discard);
+ dc->queued++;
+ list_move_tail(&dc->list, wait_list);
+
+ /* sanity check on discard range */
+ __check_sit_bitmap(sbi, dc->di.lstart, dc->di.lstart + dc->di.len);
+
+ bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(dc->di.start);
+ bio->bi_private = dc;
+ bio->bi_end_io = f2fs_submit_discard_endio;
+ submit_bio(bio);
+
+ atomic_inc(&dcc->issued_discard);
+ f2fs_update_iostat(sbi, NULL, FS_ZONE_RESET_IO, dc->di.len * F2FS_BLKSIZE);
+}
+#endif
+
/* this function is copied from blkdev_issue_discard from block/blk-lib.c */
static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
struct discard_policy *dpolicy,
@@ -1217,6 +1256,13 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
return 0;
+#ifdef CONFIG_BLK_DEV_ZONED
+ if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev)) {
+ __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
+ return 0;
+ }
+#endif
+
trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
lstart = dc->di.lstart;
@@ -1461,6 +1507,19 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
}
}
+#ifdef CONFIG_BLK_DEV_ZONED
+static void __queue_zone_reset_cmd(struct f2fs_sb_info *sbi,
+ struct block_device *bdev, block_t blkstart, block_t lblkstart,
+ block_t blklen)
+{
+ trace_f2fs_queue_reset_zone(bdev, blkstart);
+
+ mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
+ __insert_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
+ mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
+}
+#endif
+
static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
struct block_device *bdev, block_t blkstart, block_t blklen)
{
@@ -1724,6 +1783,19 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
mutex_lock(&dcc->cmd_lock);
dc = __lookup_discard_cmd(sbi, blkaddr);
+#ifdef CONFIG_BLK_DEV_ZONED
+ if (dc && f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
+ /* force submit zone reset */
+ if (dc->state == D_PREP)
+ __submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
+ &dcc->wait_list, NULL);
+ dc->ref++;
+ mutex_unlock(&dcc->cmd_lock);
+ /* wait zone reset */
+ __wait_one_discard_bio(sbi, dc);
+ return;
+ }
+#endif
if (dc) {
if (dc->state == D_PREP) {
__punch_discard_cmd(sbi, dc, blkaddr);
@@ -1876,9 +1948,15 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
blkstart, blklen);
return -EIO;
}
- trace_f2fs_issue_reset_zone(bdev, blkstart);
- return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
- sector, nr_sects, GFP_NOFS);
+
+ if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) {
+ trace_f2fs_issue_reset_zone(bdev, blkstart);
+ return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
+ sector, nr_sects, GFP_NOFS);
+ }
+
+ __queue_zone_reset_cmd(sbi, bdev, blkstart, lblkstart, blklen);
+ return 0;
}
/* For conventional zones, use regular discard if supported */
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 99cbc5949e3c..793f82cc1515 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -1512,7 +1512,7 @@ DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
TP_ARGS(dev, blkstart, blklen)
);
-TRACE_EVENT(f2fs_issue_reset_zone,
+DECLARE_EVENT_CLASS(f2fs_reset_zone,
TP_PROTO(struct block_device *dev, block_t blkstart),
@@ -1528,11 +1528,25 @@ TRACE_EVENT(f2fs_issue_reset_zone,
__entry->blkstart = blkstart;
),
- TP_printk("dev = (%d,%d), reset zone at block = 0x%llx",
+ TP_printk("dev = (%d,%d), zone at block = 0x%llx",
show_dev(__entry->dev),
(unsigned long long)__entry->blkstart)
);
+DEFINE_EVENT(f2fs_reset_zone, f2fs_queue_reset_zone,
+
+ TP_PROTO(struct block_device *dev, block_t blkstart),
+
+ TP_ARGS(dev, blkstart)
+);
+
+DEFINE_EVENT(f2fs_reset_zone, f2fs_issue_reset_zone,
+
+ TP_PROTO(struct block_device *dev, block_t blkstart),
+
+ TP_ARGS(dev, blkstart)
+);
+
TRACE_EVENT(f2fs_issue_flush,
TP_PROTO(struct block_device *dev, unsigned int nobarrier,
@@ -1979,6 +1993,7 @@ TRACE_EVENT(f2fs_iostat,
__field(unsigned long long, fs_nrio)
__field(unsigned long long, fs_mrio)
__field(unsigned long long, fs_discard)
+ __field(unsigned long long, fs_reset_zone)
),
TP_fast_assign(
@@ -2010,12 +2025,14 @@ TRACE_EVENT(f2fs_iostat,
__entry->fs_nrio = iostat[FS_NODE_READ_IO];
__entry->fs_mrio = iostat[FS_META_READ_IO];
__entry->fs_discard = iostat[FS_DISCARD_IO];
+ __entry->fs_reset_zone = iostat[FS_ZONE_RESET_IO];
),
TP_printk("dev = (%d,%d), "
"app [write=%llu (direct=%llu, buffered=%llu), mapped=%llu, "
"compr(buffered=%llu, mapped=%llu)], "
- "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu], "
+ "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu, "
+ "reset_zone=%llu], "
"gc [data=%llu, node=%llu], "
"cp [data=%llu, node=%llu, meta=%llu], "
"app [read=%llu (direct=%llu, buffered=%llu), mapped=%llu], "
@@ -2026,6 +2043,7 @@ TRACE_EVENT(f2fs_iostat,
__entry->app_bio, __entry->app_mio, __entry->app_bcdio,
__entry->app_mcdio, __entry->fs_dio, __entry->fs_cdio,
__entry->fs_nio, __entry->fs_mio, __entry->fs_discard,
+ __entry->fs_reset_zone,
__entry->fs_gc_dio, __entry->fs_gc_nio, __entry->fs_cp_dio,
__entry->fs_cp_nio, __entry->fs_cp_mio,
__entry->app_rio, __entry->app_drio, __entry->app_brio,
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [f2fs-dev] [PATCH v6] f2fs: add async reset zone command support
@ 2023-05-08 8:10 ` Daejun Park
0 siblings, 0 replies; 18+ messages in thread
From: Daejun Park @ 2023-05-08 8:10 UTC (permalink / raw)
To: jaegeuk, chao, rostedt, mhiramat, linux-f2fs-devel, linux-kernel,
linux-trace-kernel
Cc: Seokhwan Kim, beomsu kim
v5 -> v6
Added trace_f2fs_iostat support for zone reset command.
v4 -> v5
Added f2fs iostat for zone reset command.
v3 -> v4
Fixed build error caused by unused function.
v2 -> v3
Modified arguments to be correct for ftrace parameter.
Changed __submit_zone_reset_cmd to void return.
Refactored the f2fs_wait_discard_bio function.
Fixed code that was previously incorrectly merged.
v1 -> v2
Changed to apply the optional async reset write pointer by default.
This patch enables submit reset zone command asynchornously. It helps
decrease average latency of write IOs in high utilization scenario by
faster checkpointing.
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
fs/f2fs/f2fs.h | 1 +
fs/f2fs/iostat.c | 1 +
fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
include/trace/events/f2fs.h | 24 +++++++++--
4 files changed, 104 insertions(+), 6 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index d211ee89c158..51b68a629814 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1175,6 +1175,7 @@ enum iostat_type {
/* other */
FS_DISCARD_IO, /* discard */
FS_FLUSH_IO, /* flush */
+ FS_ZONE_RESET_IO, /* zone reset */
NR_IO_TYPE,
};
diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
index 3d5bfb1ad585..f8703038e1d8 100644
--- a/fs/f2fs/iostat.c
+++ b/fs/f2fs/iostat.c
@@ -80,6 +80,7 @@ int __maybe_unused iostat_info_seq_show(struct seq_file *seq, void *offset)
seq_puts(seq, "[OTHER]\n");
IOSTAT_INFO_SHOW("fs discard", FS_DISCARD_IO);
IOSTAT_INFO_SHOW("fs flush", FS_FLUSH_IO);
+ IOSTAT_INFO_SHOW("fs zone reset", FS_ZONE_RESET_IO);
return 0;
}
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 6db410f1bb8c..4802b05a795b 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1196,6 +1196,45 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
struct block_device *bdev, block_t lstart,
block_t start, block_t len);
+
+#ifdef CONFIG_BLK_DEV_ZONED
+static void __submit_zone_reset_cmd(struct f2fs_sb_info *sbi,
+ struct discard_cmd *dc, blk_opf_t flag,
+ struct list_head *wait_list,
+ unsigned int *issued)
+{
+ struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
+ struct block_device *bdev = dc->bdev;
+ struct bio *bio = bio_alloc(bdev, 0, REQ_OP_ZONE_RESET | flag, GFP_NOFS);
+ unsigned long flags;
+
+ trace_f2fs_issue_reset_zone(bdev, dc->di.start);
+
+ spin_lock_irqsave(&dc->lock, flags);
+ dc->state = D_SUBMIT;
+ dc->bio_ref++;
+ spin_unlock_irqrestore(&dc->lock, flags);
+
+ if (issued)
+ (*issued)++;
+
+ atomic_inc(&dcc->queued_discard);
+ dc->queued++;
+ list_move_tail(&dc->list, wait_list);
+
+ /* sanity check on discard range */
+ __check_sit_bitmap(sbi, dc->di.lstart, dc->di.lstart + dc->di.len);
+
+ bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(dc->di.start);
+ bio->bi_private = dc;
+ bio->bi_end_io = f2fs_submit_discard_endio;
+ submit_bio(bio);
+
+ atomic_inc(&dcc->issued_discard);
+ f2fs_update_iostat(sbi, NULL, FS_ZONE_RESET_IO, dc->di.len * F2FS_BLKSIZE);
+}
+#endif
+
/* this function is copied from blkdev_issue_discard from block/blk-lib.c */
static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
struct discard_policy *dpolicy,
@@ -1217,6 +1256,13 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
return 0;
+#ifdef CONFIG_BLK_DEV_ZONED
+ if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev)) {
+ __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
+ return 0;
+ }
+#endif
+
trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
lstart = dc->di.lstart;
@@ -1461,6 +1507,19 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
}
}
+#ifdef CONFIG_BLK_DEV_ZONED
+static void __queue_zone_reset_cmd(struct f2fs_sb_info *sbi,
+ struct block_device *bdev, block_t blkstart, block_t lblkstart,
+ block_t blklen)
+{
+ trace_f2fs_queue_reset_zone(bdev, blkstart);
+
+ mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
+ __insert_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
+ mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
+}
+#endif
+
static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
struct block_device *bdev, block_t blkstart, block_t blklen)
{
@@ -1724,6 +1783,19 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
mutex_lock(&dcc->cmd_lock);
dc = __lookup_discard_cmd(sbi, blkaddr);
+#ifdef CONFIG_BLK_DEV_ZONED
+ if (dc && f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
+ /* force submit zone reset */
+ if (dc->state == D_PREP)
+ __submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
+ &dcc->wait_list, NULL);
+ dc->ref++;
+ mutex_unlock(&dcc->cmd_lock);
+ /* wait zone reset */
+ __wait_one_discard_bio(sbi, dc);
+ return;
+ }
+#endif
if (dc) {
if (dc->state == D_PREP) {
__punch_discard_cmd(sbi, dc, blkaddr);
@@ -1876,9 +1948,15 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
blkstart, blklen);
return -EIO;
}
- trace_f2fs_issue_reset_zone(bdev, blkstart);
- return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
- sector, nr_sects, GFP_NOFS);
+
+ if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) {
+ trace_f2fs_issue_reset_zone(bdev, blkstart);
+ return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
+ sector, nr_sects, GFP_NOFS);
+ }
+
+ __queue_zone_reset_cmd(sbi, bdev, blkstart, lblkstart, blklen);
+ return 0;
}
/* For conventional zones, use regular discard if supported */
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 99cbc5949e3c..793f82cc1515 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -1512,7 +1512,7 @@ DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
TP_ARGS(dev, blkstart, blklen)
);
-TRACE_EVENT(f2fs_issue_reset_zone,
+DECLARE_EVENT_CLASS(f2fs_reset_zone,
TP_PROTO(struct block_device *dev, block_t blkstart),
@@ -1528,11 +1528,25 @@ TRACE_EVENT(f2fs_issue_reset_zone,
__entry->blkstart = blkstart;
),
- TP_printk("dev = (%d,%d), reset zone at block = 0x%llx",
+ TP_printk("dev = (%d,%d), zone at block = 0x%llx",
show_dev(__entry->dev),
(unsigned long long)__entry->blkstart)
);
+DEFINE_EVENT(f2fs_reset_zone, f2fs_queue_reset_zone,
+
+ TP_PROTO(struct block_device *dev, block_t blkstart),
+
+ TP_ARGS(dev, blkstart)
+);
+
+DEFINE_EVENT(f2fs_reset_zone, f2fs_issue_reset_zone,
+
+ TP_PROTO(struct block_device *dev, block_t blkstart),
+
+ TP_ARGS(dev, blkstart)
+);
+
TRACE_EVENT(f2fs_issue_flush,
TP_PROTO(struct block_device *dev, unsigned int nobarrier,
@@ -1979,6 +1993,7 @@ TRACE_EVENT(f2fs_iostat,
__field(unsigned long long, fs_nrio)
__field(unsigned long long, fs_mrio)
__field(unsigned long long, fs_discard)
+ __field(unsigned long long, fs_reset_zone)
),
TP_fast_assign(
@@ -2010,12 +2025,14 @@ TRACE_EVENT(f2fs_iostat,
__entry->fs_nrio = iostat[FS_NODE_READ_IO];
__entry->fs_mrio = iostat[FS_META_READ_IO];
__entry->fs_discard = iostat[FS_DISCARD_IO];
+ __entry->fs_reset_zone = iostat[FS_ZONE_RESET_IO];
),
TP_printk("dev = (%d,%d), "
"app [write=%llu (direct=%llu, buffered=%llu), mapped=%llu, "
"compr(buffered=%llu, mapped=%llu)], "
- "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu], "
+ "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu, "
+ "reset_zone=%llu], "
"gc [data=%llu, node=%llu], "
"cp [data=%llu, node=%llu, meta=%llu], "
"app [read=%llu (direct=%llu, buffered=%llu), mapped=%llu], "
@@ -2026,6 +2043,7 @@ TRACE_EVENT(f2fs_iostat,
__entry->app_bio, __entry->app_mio, __entry->app_bcdio,
__entry->app_mcdio, __entry->fs_dio, __entry->fs_cdio,
__entry->fs_nio, __entry->fs_mio, __entry->fs_discard,
+ __entry->fs_reset_zone,
__entry->fs_gc_dio, __entry->fs_gc_nio, __entry->fs_cp_dio,
__entry->fs_cp_nio, __entry->fs_cp_mio,
__entry->app_rio, __entry->app_drio, __entry->app_brio,
--
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] 18+ messages in thread
* Re: [PATCH v6] f2fs: add async reset zone command support
2023-05-08 8:10 ` [f2fs-dev] " Daejun Park
@ 2023-05-09 18:53 ` Jaegeuk Kim
-1 siblings, 0 replies; 18+ messages in thread
From: Jaegeuk Kim @ 2023-05-09 18:53 UTC (permalink / raw)
To: Daejun Park
Cc: chao, rostedt, mhiramat, linux-f2fs-devel, linux-kernel,
linux-trace-kernel, Seokhwan Kim, Yonggil Song, beomsu kim
On 05/08, Daejun Park wrote:
> v5 -> v6
> Added trace_f2fs_iostat support for zone reset command.
>
> v4 -> v5
> Added f2fs iostat for zone reset command.
>
> v3 -> v4
> Fixed build error caused by unused function.
>
> v2 -> v3
> Modified arguments to be correct for ftrace parameter.
> Changed __submit_zone_reset_cmd to void return.
> Refactored the f2fs_wait_discard_bio function.
> Fixed code that was previously incorrectly merged.
>
> v1 -> v2
> Changed to apply the optional async reset write pointer by default.
Don't add the history in the patch description.
>
> This patch enables submit reset zone command asynchornously. It helps
> decrease average latency of write IOs in high utilization scenario by
> faster checkpointing.
>
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> ---
> fs/f2fs/f2fs.h | 1 +
> fs/f2fs/iostat.c | 1 +
> fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
> include/trace/events/f2fs.h | 24 +++++++++--
> 4 files changed, 104 insertions(+), 6 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index d211ee89c158..51b68a629814 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1175,6 +1175,7 @@ enum iostat_type {
> /* other */
> FS_DISCARD_IO, /* discard */
> FS_FLUSH_IO, /* flush */
> + FS_ZONE_RESET_IO, /* zone reset */
> NR_IO_TYPE,
> };
>
> diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
> index 3d5bfb1ad585..f8703038e1d8 100644
> --- a/fs/f2fs/iostat.c
> +++ b/fs/f2fs/iostat.c
> @@ -80,6 +80,7 @@ int __maybe_unused iostat_info_seq_show(struct seq_file *seq, void *offset)
> seq_puts(seq, "[OTHER]\n");
> IOSTAT_INFO_SHOW("fs discard", FS_DISCARD_IO);
> IOSTAT_INFO_SHOW("fs flush", FS_FLUSH_IO);
> + IOSTAT_INFO_SHOW("fs zone reset", FS_ZONE_RESET_IO);
>
> return 0;
> }
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 6db410f1bb8c..4802b05a795b 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1196,6 +1196,45 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
> static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> struct block_device *bdev, block_t lstart,
> block_t start, block_t len);
> +
> +#ifdef CONFIG_BLK_DEV_ZONED
> +static void __submit_zone_reset_cmd(struct f2fs_sb_info *sbi,
Why can't we use __f2fs_issue_discard_zone()?
> + struct discard_cmd *dc, blk_opf_t flag,
> + struct list_head *wait_list,
> + unsigned int *issued)
> +{
> + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> + struct block_device *bdev = dc->bdev;
> + struct bio *bio = bio_alloc(bdev, 0, REQ_OP_ZONE_RESET | flag, GFP_NOFS);
> + unsigned long flags;
> +
> + trace_f2fs_issue_reset_zone(bdev, dc->di.start);
> +
> + spin_lock_irqsave(&dc->lock, flags);
> + dc->state = D_SUBMIT;
> + dc->bio_ref++;
> + spin_unlock_irqrestore(&dc->lock, flags);
> +
> + if (issued)
> + (*issued)++;
> +
> + atomic_inc(&dcc->queued_discard);
> + dc->queued++;
> + list_move_tail(&dc->list, wait_list);
> +
> + /* sanity check on discard range */
> + __check_sit_bitmap(sbi, dc->di.lstart, dc->di.lstart + dc->di.len);
> +
> + bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(dc->di.start);
> + bio->bi_private = dc;
> + bio->bi_end_io = f2fs_submit_discard_endio;
> + submit_bio(bio);
> +
> + atomic_inc(&dcc->issued_discard);
> + f2fs_update_iostat(sbi, NULL, FS_ZONE_RESET_IO, dc->di.len * F2FS_BLKSIZE);
> +}
> +#endif
> +
> /* this function is copied from blkdev_issue_discard from block/blk-lib.c */
> static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> struct discard_policy *dpolicy,
> @@ -1217,6 +1256,13 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> return 0;
>
> +#ifdef CONFIG_BLK_DEV_ZONED
> + if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev)) {
> + __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> + return 0;
> + }
> +#endif
> +
> trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
>
> lstart = dc->di.lstart;
> @@ -1461,6 +1507,19 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> }
> }
>
> +#ifdef CONFIG_BLK_DEV_ZONED
> +static void __queue_zone_reset_cmd(struct f2fs_sb_info *sbi,
> + struct block_device *bdev, block_t blkstart, block_t lblkstart,
> + block_t blklen)
> +{
> + trace_f2fs_queue_reset_zone(bdev, blkstart);
> +
> + mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> + __insert_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
> + mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> +}
> +#endif
> +
> static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
> struct block_device *bdev, block_t blkstart, block_t blklen)
> {
> @@ -1724,6 +1783,19 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
>
> mutex_lock(&dcc->cmd_lock);
> dc = __lookup_discard_cmd(sbi, blkaddr);
> +#ifdef CONFIG_BLK_DEV_ZONED
> + if (dc && f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
> + /* force submit zone reset */
> + if (dc->state == D_PREP)
> + __submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
> + &dcc->wait_list, NULL);
> + dc->ref++;
> + mutex_unlock(&dcc->cmd_lock);
> + /* wait zone reset */
> + __wait_one_discard_bio(sbi, dc);
> + return;
> + }
> +#endif
> if (dc) {
> if (dc->state == D_PREP) {
> __punch_discard_cmd(sbi, dc, blkaddr);
> @@ -1876,9 +1948,15 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
> blkstart, blklen);
> return -EIO;
> }
> - trace_f2fs_issue_reset_zone(bdev, blkstart);
> - return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> - sector, nr_sects, GFP_NOFS);
> +
> + if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) {
> + trace_f2fs_issue_reset_zone(bdev, blkstart);
> + return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> + sector, nr_sects, GFP_NOFS);
> + }
> +
> + __queue_zone_reset_cmd(sbi, bdev, blkstart, lblkstart, blklen);
> + return 0;
> }
>
> /* For conventional zones, use regular discard if supported */
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index 99cbc5949e3c..793f82cc1515 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -1512,7 +1512,7 @@ DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
> TP_ARGS(dev, blkstart, blklen)
> );
>
> -TRACE_EVENT(f2fs_issue_reset_zone,
> +DECLARE_EVENT_CLASS(f2fs_reset_zone,
>
> TP_PROTO(struct block_device *dev, block_t blkstart),
>
> @@ -1528,11 +1528,25 @@ TRACE_EVENT(f2fs_issue_reset_zone,
> __entry->blkstart = blkstart;
> ),
>
> - TP_printk("dev = (%d,%d), reset zone at block = 0x%llx",
> + TP_printk("dev = (%d,%d), zone at block = 0x%llx",
> show_dev(__entry->dev),
> (unsigned long long)__entry->blkstart)
> );
>
> +DEFINE_EVENT(f2fs_reset_zone, f2fs_queue_reset_zone,
> +
> + TP_PROTO(struct block_device *dev, block_t blkstart),
> +
> + TP_ARGS(dev, blkstart)
> +);
> +
> +DEFINE_EVENT(f2fs_reset_zone, f2fs_issue_reset_zone,
> +
> + TP_PROTO(struct block_device *dev, block_t blkstart),
> +
> + TP_ARGS(dev, blkstart)
> +);
> +
> TRACE_EVENT(f2fs_issue_flush,
>
> TP_PROTO(struct block_device *dev, unsigned int nobarrier,
> @@ -1979,6 +1993,7 @@ TRACE_EVENT(f2fs_iostat,
> __field(unsigned long long, fs_nrio)
> __field(unsigned long long, fs_mrio)
> __field(unsigned long long, fs_discard)
> + __field(unsigned long long, fs_reset_zone)
> ),
>
> TP_fast_assign(
> @@ -2010,12 +2025,14 @@ TRACE_EVENT(f2fs_iostat,
> __entry->fs_nrio = iostat[FS_NODE_READ_IO];
> __entry->fs_mrio = iostat[FS_META_READ_IO];
> __entry->fs_discard = iostat[FS_DISCARD_IO];
> + __entry->fs_reset_zone = iostat[FS_ZONE_RESET_IO];
> ),
>
> TP_printk("dev = (%d,%d), "
> "app [write=%llu (direct=%llu, buffered=%llu), mapped=%llu, "
> "compr(buffered=%llu, mapped=%llu)], "
> - "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu], "
> + "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu, "
> + "reset_zone=%llu], "
> "gc [data=%llu, node=%llu], "
> "cp [data=%llu, node=%llu, meta=%llu], "
> "app [read=%llu (direct=%llu, buffered=%llu), mapped=%llu], "
> @@ -2026,6 +2043,7 @@ TRACE_EVENT(f2fs_iostat,
> __entry->app_bio, __entry->app_mio, __entry->app_bcdio,
> __entry->app_mcdio, __entry->fs_dio, __entry->fs_cdio,
> __entry->fs_nio, __entry->fs_mio, __entry->fs_discard,
> + __entry->fs_reset_zone,
> __entry->fs_gc_dio, __entry->fs_gc_nio, __entry->fs_cp_dio,
> __entry->fs_cp_nio, __entry->fs_cp_mio,
> __entry->app_rio, __entry->app_drio, __entry->app_brio,
> --
> 2.25.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH v6] f2fs: add async reset zone command support
@ 2023-05-09 18:53 ` Jaegeuk Kim
0 siblings, 0 replies; 18+ messages in thread
From: Jaegeuk Kim @ 2023-05-09 18:53 UTC (permalink / raw)
To: Daejun Park
Cc: linux-kernel, rostedt, linux-f2fs-devel, mhiramat, Seokhwan Kim,
beomsu kim, linux-trace-kernel
On 05/08, Daejun Park wrote:
> v5 -> v6
> Added trace_f2fs_iostat support for zone reset command.
>
> v4 -> v5
> Added f2fs iostat for zone reset command.
>
> v3 -> v4
> Fixed build error caused by unused function.
>
> v2 -> v3
> Modified arguments to be correct for ftrace parameter.
> Changed __submit_zone_reset_cmd to void return.
> Refactored the f2fs_wait_discard_bio function.
> Fixed code that was previously incorrectly merged.
>
> v1 -> v2
> Changed to apply the optional async reset write pointer by default.
Don't add the history in the patch description.
>
> This patch enables submit reset zone command asynchornously. It helps
> decrease average latency of write IOs in high utilization scenario by
> faster checkpointing.
>
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> ---
> fs/f2fs/f2fs.h | 1 +
> fs/f2fs/iostat.c | 1 +
> fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
> include/trace/events/f2fs.h | 24 +++++++++--
> 4 files changed, 104 insertions(+), 6 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index d211ee89c158..51b68a629814 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1175,6 +1175,7 @@ enum iostat_type {
> /* other */
> FS_DISCARD_IO, /* discard */
> FS_FLUSH_IO, /* flush */
> + FS_ZONE_RESET_IO, /* zone reset */
> NR_IO_TYPE,
> };
>
> diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
> index 3d5bfb1ad585..f8703038e1d8 100644
> --- a/fs/f2fs/iostat.c
> +++ b/fs/f2fs/iostat.c
> @@ -80,6 +80,7 @@ int __maybe_unused iostat_info_seq_show(struct seq_file *seq, void *offset)
> seq_puts(seq, "[OTHER]\n");
> IOSTAT_INFO_SHOW("fs discard", FS_DISCARD_IO);
> IOSTAT_INFO_SHOW("fs flush", FS_FLUSH_IO);
> + IOSTAT_INFO_SHOW("fs zone reset", FS_ZONE_RESET_IO);
>
> return 0;
> }
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 6db410f1bb8c..4802b05a795b 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1196,6 +1196,45 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
> static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> struct block_device *bdev, block_t lstart,
> block_t start, block_t len);
> +
> +#ifdef CONFIG_BLK_DEV_ZONED
> +static void __submit_zone_reset_cmd(struct f2fs_sb_info *sbi,
Why can't we use __f2fs_issue_discard_zone()?
> + struct discard_cmd *dc, blk_opf_t flag,
> + struct list_head *wait_list,
> + unsigned int *issued)
> +{
> + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> + struct block_device *bdev = dc->bdev;
> + struct bio *bio = bio_alloc(bdev, 0, REQ_OP_ZONE_RESET | flag, GFP_NOFS);
> + unsigned long flags;
> +
> + trace_f2fs_issue_reset_zone(bdev, dc->di.start);
> +
> + spin_lock_irqsave(&dc->lock, flags);
> + dc->state = D_SUBMIT;
> + dc->bio_ref++;
> + spin_unlock_irqrestore(&dc->lock, flags);
> +
> + if (issued)
> + (*issued)++;
> +
> + atomic_inc(&dcc->queued_discard);
> + dc->queued++;
> + list_move_tail(&dc->list, wait_list);
> +
> + /* sanity check on discard range */
> + __check_sit_bitmap(sbi, dc->di.lstart, dc->di.lstart + dc->di.len);
> +
> + bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(dc->di.start);
> + bio->bi_private = dc;
> + bio->bi_end_io = f2fs_submit_discard_endio;
> + submit_bio(bio);
> +
> + atomic_inc(&dcc->issued_discard);
> + f2fs_update_iostat(sbi, NULL, FS_ZONE_RESET_IO, dc->di.len * F2FS_BLKSIZE);
> +}
> +#endif
> +
> /* this function is copied from blkdev_issue_discard from block/blk-lib.c */
> static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> struct discard_policy *dpolicy,
> @@ -1217,6 +1256,13 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> return 0;
>
> +#ifdef CONFIG_BLK_DEV_ZONED
> + if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev)) {
> + __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> + return 0;
> + }
> +#endif
> +
> trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
>
> lstart = dc->di.lstart;
> @@ -1461,6 +1507,19 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> }
> }
>
> +#ifdef CONFIG_BLK_DEV_ZONED
> +static void __queue_zone_reset_cmd(struct f2fs_sb_info *sbi,
> + struct block_device *bdev, block_t blkstart, block_t lblkstart,
> + block_t blklen)
> +{
> + trace_f2fs_queue_reset_zone(bdev, blkstart);
> +
> + mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> + __insert_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
> + mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> +}
> +#endif
> +
> static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
> struct block_device *bdev, block_t blkstart, block_t blklen)
> {
> @@ -1724,6 +1783,19 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
>
> mutex_lock(&dcc->cmd_lock);
> dc = __lookup_discard_cmd(sbi, blkaddr);
> +#ifdef CONFIG_BLK_DEV_ZONED
> + if (dc && f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
> + /* force submit zone reset */
> + if (dc->state == D_PREP)
> + __submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
> + &dcc->wait_list, NULL);
> + dc->ref++;
> + mutex_unlock(&dcc->cmd_lock);
> + /* wait zone reset */
> + __wait_one_discard_bio(sbi, dc);
> + return;
> + }
> +#endif
> if (dc) {
> if (dc->state == D_PREP) {
> __punch_discard_cmd(sbi, dc, blkaddr);
> @@ -1876,9 +1948,15 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
> blkstart, blklen);
> return -EIO;
> }
> - trace_f2fs_issue_reset_zone(bdev, blkstart);
> - return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> - sector, nr_sects, GFP_NOFS);
> +
> + if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) {
> + trace_f2fs_issue_reset_zone(bdev, blkstart);
> + return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> + sector, nr_sects, GFP_NOFS);
> + }
> +
> + __queue_zone_reset_cmd(sbi, bdev, blkstart, lblkstart, blklen);
> + return 0;
> }
>
> /* For conventional zones, use regular discard if supported */
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index 99cbc5949e3c..793f82cc1515 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -1512,7 +1512,7 @@ DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
> TP_ARGS(dev, blkstart, blklen)
> );
>
> -TRACE_EVENT(f2fs_issue_reset_zone,
> +DECLARE_EVENT_CLASS(f2fs_reset_zone,
>
> TP_PROTO(struct block_device *dev, block_t blkstart),
>
> @@ -1528,11 +1528,25 @@ TRACE_EVENT(f2fs_issue_reset_zone,
> __entry->blkstart = blkstart;
> ),
>
> - TP_printk("dev = (%d,%d), reset zone at block = 0x%llx",
> + TP_printk("dev = (%d,%d), zone at block = 0x%llx",
> show_dev(__entry->dev),
> (unsigned long long)__entry->blkstart)
> );
>
> +DEFINE_EVENT(f2fs_reset_zone, f2fs_queue_reset_zone,
> +
> + TP_PROTO(struct block_device *dev, block_t blkstart),
> +
> + TP_ARGS(dev, blkstart)
> +);
> +
> +DEFINE_EVENT(f2fs_reset_zone, f2fs_issue_reset_zone,
> +
> + TP_PROTO(struct block_device *dev, block_t blkstart),
> +
> + TP_ARGS(dev, blkstart)
> +);
> +
> TRACE_EVENT(f2fs_issue_flush,
>
> TP_PROTO(struct block_device *dev, unsigned int nobarrier,
> @@ -1979,6 +1993,7 @@ TRACE_EVENT(f2fs_iostat,
> __field(unsigned long long, fs_nrio)
> __field(unsigned long long, fs_mrio)
> __field(unsigned long long, fs_discard)
> + __field(unsigned long long, fs_reset_zone)
> ),
>
> TP_fast_assign(
> @@ -2010,12 +2025,14 @@ TRACE_EVENT(f2fs_iostat,
> __entry->fs_nrio = iostat[FS_NODE_READ_IO];
> __entry->fs_mrio = iostat[FS_META_READ_IO];
> __entry->fs_discard = iostat[FS_DISCARD_IO];
> + __entry->fs_reset_zone = iostat[FS_ZONE_RESET_IO];
> ),
>
> TP_printk("dev = (%d,%d), "
> "app [write=%llu (direct=%llu, buffered=%llu), mapped=%llu, "
> "compr(buffered=%llu, mapped=%llu)], "
> - "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu], "
> + "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu, "
> + "reset_zone=%llu], "
> "gc [data=%llu, node=%llu], "
> "cp [data=%llu, node=%llu, meta=%llu], "
> "app [read=%llu (direct=%llu, buffered=%llu), mapped=%llu], "
> @@ -2026,6 +2043,7 @@ TRACE_EVENT(f2fs_iostat,
> __entry->app_bio, __entry->app_mio, __entry->app_bcdio,
> __entry->app_mcdio, __entry->fs_dio, __entry->fs_cdio,
> __entry->fs_nio, __entry->fs_mio, __entry->fs_discard,
> + __entry->fs_reset_zone,
> __entry->fs_gc_dio, __entry->fs_gc_nio, __entry->fs_cp_dio,
> __entry->fs_cp_nio, __entry->fs_cp_mio,
> __entry->app_rio, __entry->app_drio, __entry->app_brio,
> --
> 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 [flat|nested] 18+ messages in thread
* RE:(2) [PATCH v6] f2fs: add async reset zone command support
[not found] ` <CGME20230508081042epcms2p8a637deae7de1829f54614e09d5fde5e5@epcms2p6>
@ 2023-05-11 5:24 ` Daejun Park
0 siblings, 0 replies; 18+ messages in thread
From: Daejun Park @ 2023-05-11 5:24 UTC (permalink / raw)
To: Jaegeuk Kim, Daejun Park
Cc: chao, rostedt, mhiramat, linux-f2fs-devel, linux-kernel,
linux-trace-kernel, Seokhwan Kim, Yonggil Song, beomsu kim
> Sender : Jaegeuk Kim <jaegeuk@kernel.org>
> Date : 2023-05-10 03:53 (GMT+9)
> Title : Re: [PATCH v6] f2fs: add async reset zone command support
> To : 박대준<daejun7.park@samsung.com>
> CC : chao@kernel.org<chao@kernel.org>, rostedt@goodmis.org<rostedt@goodmis.org>, mhiramat@kernel.org<mhiramat@kernel.org>, linux-f2fs-devel@lists.sourceforge.net<linux-f2fs-devel@lists.sourceforge.net>, linux-kernel@vger.kernel.org<linux-kernel@vger.kernel.org>, linux-trace-kernel@vger.kernel.org<linux-trace-kernel@vger.kernel.org>, 김석환<sukka.kim@samsung.com>, 송용길<yonggil.song@samsung.com>, 김범수<beomsu7.kim@samsung.com>
>
> On 05/08, Daejun Park wrote:
> > v5 -> v6
> > Added trace_f2fs_iostat support for zone reset command.
> >
> > v4 -> v5
> > Added f2fs iostat for zone reset command.
> >
> > v3 -> v4
> > Fixed build error caused by unused function.
> >
> > v2 -> v3
> > Modified arguments to be correct for ftrace parameter.
> > Changed __submit_zone_reset_cmd to void return.
> > Refactored the f2fs_wait_discard_bio function.
> > Fixed code that was previously incorrectly merged.
> >
> > v1 -> v2
> > Changed to apply the optional async reset write pointer by default.
>
> Don't add the history in the patch description.
OK, I will do.
> >
> > This patch enables submit reset zone command asynchornously. It helps
> > decrease average latency of write IOs in high utilization scenario by
> > faster checkpointing.
> >
> > Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> > ---
> > fs/f2fs/f2fs.h | 1 +
> > fs/f2fs/iostat.c | 1 +
> > fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
> > include/trace/events/f2fs.h | 24 +++++++++--
> > 4 files changed, 104 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index d211ee89c158..51b68a629814 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1175,6 +1175,7 @@ enum iostat_type {
> > /* other */
> > FS_DISCARD_IO, /* discard */
> > FS_FLUSH_IO, /* flush */
> > + FS_ZONE_RESET_IO, /* zone reset */
> > NR_IO_TYPE,
> > };
> >
> > diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
> > index 3d5bfb1ad585..f8703038e1d8 100644
> > --- a/fs/f2fs/iostat.c
> > +++ b/fs/f2fs/iostat.c
> > @@ -80,6 +80,7 @@ int __maybe_unused iostat_info_seq_show(struct seq_file *seq, void *offset)
> > seq_puts(seq, "[OTHER]\n");
> > IOSTAT_INFO_SHOW("fs discard", FS_DISCARD_IO);
> > IOSTAT_INFO_SHOW("fs flush", FS_FLUSH_IO);
> > + IOSTAT_INFO_SHOW("fs zone reset", FS_ZONE_RESET_IO);
> >
> > return 0;
> > }
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 6db410f1bb8c..4802b05a795b 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -1196,6 +1196,45 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
> > static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > struct block_device *bdev, block_t lstart,
> > block_t start, block_t len);
> > +
> > +#ifdef CONFIG_BLK_DEV_ZONED
> > +static void __submit_zone_reset_cmd(struct f2fs_sb_info *sbi,
>
> Why can't we use __f2fs_issue_discard_zone()?
In my understanding, __f2fs_issue_discard_zone is used to queue the zone reset command.
Which point do you think is the issue?
>
> > + struct discard_cmd *dc, blk_opf_t flag,
> > + struct list_head *wait_list,
> > + unsigned int *issued)
> > +{
> > + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > + struct block_device *bdev = dc->bdev;
> > + struct bio *bio = bio_alloc(bdev, 0, REQ_OP_ZONE_RESET | flag, GFP_NOFS);
> > + unsigned long flags;
> > +
> > + trace_f2fs_issue_reset_zone(bdev, dc->di.start);
> > +
> > + spin_lock_irqsave(&dc->lock, flags);
> > + dc->state = D_SUBMIT;
> > + dc->bio_ref++;
> > + spin_unlock_irqrestore(&dc->lock, flags);
> > +
> > + if (issued)
> > + (*issued)++;
> > +
> > + atomic_inc(&dcc->queued_discard);
> > + dc->queued++;
> > + list_move_tail(&dc->list, wait_list);
> > +
> > + /* sanity check on discard range */
> > + __check_sit_bitmap(sbi, dc->di.lstart, dc->di.lstart + dc->di.len);
> > +
> > + bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(dc->di.start);
> > + bio->bi_private = dc;
> > + bio->bi_end_io = f2fs_submit_discard_endio;
> > + submit_bio(bio);
> > +
> > + atomic_inc(&dcc->issued_discard);
> > + f2fs_update_iostat(sbi, NULL, FS_ZONE_RESET_IO, dc->di.len * F2FS_BLKSIZE);
> > +}
> > +#endif
> > +
> > /* this function is copied from blkdev_issue_discard from block/blk-lib.c */
> > static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > struct discard_policy *dpolicy,
> > @@ -1217,6 +1256,13 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> > return 0;
> >
> > +#ifdef CONFIG_BLK_DEV_ZONED
> > + if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev)) {
> > + __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> > + return 0;
> > + }
> > +#endif
> > +
> > trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
> >
> > lstart = dc->di.lstart;
> > @@ -1461,6 +1507,19 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > }
> > }
> >
> > +#ifdef CONFIG_BLK_DEV_ZONED
> > +static void __queue_zone_reset_cmd(struct f2fs_sb_info *sbi,
> > + struct block_device *bdev, block_t blkstart, block_t lblkstart,
> > + block_t blklen)
> > +{
> > + trace_f2fs_queue_reset_zone(bdev, blkstart);
> > +
> > + mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> > + __insert_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
> > + mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> > +}
> > +#endif
> > +
> > static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
> > struct block_device *bdev, block_t blkstart, block_t blklen)
> > {
> > @@ -1724,6 +1783,19 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
> >
> > mutex_lock(&dcc->cmd_lock);
> > dc = __lookup_discard_cmd(sbi, blkaddr);
> > +#ifdef CONFIG_BLK_DEV_ZONED
> > + if (dc && f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
> > + /* force submit zone reset */
> > + if (dc->state == D_PREP)
> > + __submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
> > + &dcc->wait_list, NULL);
> > + dc->ref++;
> > + mutex_unlock(&dcc->cmd_lock);
> > + /* wait zone reset */
> > + __wait_one_discard_bio(sbi, dc);
> > + return;
> > + }
> > +#endif
> > if (dc) {
> > if (dc->state == D_PREP) {
> > __punch_discard_cmd(sbi, dc, blkaddr);
> > @@ -1876,9 +1948,15 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
> > blkstart, blklen);
> > return -EIO;
> > }
> > - trace_f2fs_issue_reset_zone(bdev, blkstart);
> > - return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > - sector, nr_sects, GFP_NOFS);
> > +
> > + if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) {
> > + trace_f2fs_issue_reset_zone(bdev, blkstart);
> > + return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > + sector, nr_sects, GFP_NOFS);
> > + }
> > +
> > + __queue_zone_reset_cmd(sbi, bdev, blkstart, lblkstart, blklen);
> > + return 0;
> > }
> >
> > /* For conventional zones, use regular discard if supported */
> > diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> > index 99cbc5949e3c..793f82cc1515 100644
> > --- a/include/trace/events/f2fs.h
> > +++ b/include/trace/events/f2fs.h
> > @@ -1512,7 +1512,7 @@ DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
> > TP_ARGS(dev, blkstart, blklen)
> > );
> >
> > -TRACE_EVENT(f2fs_issue_reset_zone,
> > +DECLARE_EVENT_CLASS(f2fs_reset_zone,
> >
> > TP_PROTO(struct block_device *dev, block_t blkstart),
> >
> > @@ -1528,11 +1528,25 @@ TRACE_EVENT(f2fs_issue_reset_zone,
> > __entry->blkstart = blkstart;
> > ),
> >
> > - TP_printk("dev = (%d,%d), reset zone at block = 0x%llx",
> > + TP_printk("dev = (%d,%d), zone at block = 0x%llx",
> > show_dev(__entry->dev),
> > (unsigned long long)__entry->blkstart)
> > );
> >
> > +DEFINE_EVENT(f2fs_reset_zone, f2fs_queue_reset_zone,
> > +
> > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > +
> > + TP_ARGS(dev, blkstart)
> > +);
> > +
> > +DEFINE_EVENT(f2fs_reset_zone, f2fs_issue_reset_zone,
> > +
> > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > +
> > + TP_ARGS(dev, blkstart)
> > +);
> > +
> > TRACE_EVENT(f2fs_issue_flush,
> >
> > TP_PROTO(struct block_device *dev, unsigned int nobarrier,
> > @@ -1979,6 +1993,7 @@ TRACE_EVENT(f2fs_iostat,
> > __field(unsigned long long, fs_nrio)
> > __field(unsigned long long, fs_mrio)
> > __field(unsigned long long, fs_discard)
> > + __field(unsigned long long, fs_reset_zone)
> > ),
> >
> > TP_fast_assign(
> > @@ -2010,12 +2025,14 @@ TRACE_EVENT(f2fs_iostat,
> > __entry->fs_nrio = iostat[FS_NODE_READ_IO];
> > __entry->fs_mrio = iostat[FS_META_READ_IO];
> > __entry->fs_discard = iostat[FS_DISCARD_IO];
> > + __entry->fs_reset_zone = iostat[FS_ZONE_RESET_IO];
> > ),
> >
> > TP_printk("dev = (%d,%d), "
> > "app [write=%llu (direct=%llu, buffered=%llu), mapped=%llu, "
> > "compr(buffered=%llu, mapped=%llu)], "
> > - "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu], "
> > + "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu, "
> > + "reset_zone=%llu], "
> > "gc [data=%llu, node=%llu], "
> > "cp [data=%llu, node=%llu, meta=%llu], "
> > "app [read=%llu (direct=%llu, buffered=%llu), mapped=%llu], "
> > @@ -2026,6 +2043,7 @@ TRACE_EVENT(f2fs_iostat,
> > __entry->app_bio, __entry->app_mio, __entry->app_bcdio,
> > __entry->app_mcdio, __entry->fs_dio, __entry->fs_cdio,
> > __entry->fs_nio, __entry->fs_mio, __entry->fs_discard,
> > + __entry->fs_reset_zone,
> > __entry->fs_gc_dio, __entry->fs_gc_nio, __entry->fs_cp_dio,
> > __entry->fs_cp_nio, __entry->fs_cp_mio,
> > __entry->app_rio, __entry->app_drio, __entry->app_brio,
> > --
> > 2.25.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] (2) [PATCH v6] f2fs: add async reset zone command support
@ 2023-05-11 5:24 ` Daejun Park
0 siblings, 0 replies; 18+ messages in thread
From: Daejun Park @ 2023-05-11 5:24 UTC (permalink / raw)
To: Jaegeuk Kim, Daejun Park
Cc: linux-kernel, rostedt, linux-f2fs-devel, mhiramat, Seokhwan Kim,
beomsu kim, linux-trace-kernel
> Sender : Jaegeuk Kim <jaegeuk@kernel.org>
> Date : 2023-05-10 03:53 (GMT+9)
> Title : Re: [PATCH v6] f2fs: add async reset zone command support
> To : 박대준<daejun7.park@samsung.com>
> CC : chao@kernel.org<chao@kernel.org>, rostedt@goodmis.org<rostedt@goodmis.org>, mhiramat@kernel.org<mhiramat@kernel.org>, linux-f2fs-devel@lists.sourceforge.net<linux-f2fs-devel@lists.sourceforge.net>, linux-kernel@vger.kernel.org<linux-kernel@vger.kernel.org>, linux-trace-kernel@vger.kernel.org<linux-trace-kernel@vger.kernel.org>, 김석환<sukka.kim@samsung.com>, 송용길<yonggil.song@samsung.com>, 김범수<beomsu7.kim@samsung.com>
>
> On 05/08, Daejun Park wrote:
> > v5 -> v6
> > Added trace_f2fs_iostat support for zone reset command.
> >
> > v4 -> v5
> > Added f2fs iostat for zone reset command.
> >
> > v3 -> v4
> > Fixed build error caused by unused function.
> >
> > v2 -> v3
> > Modified arguments to be correct for ftrace parameter.
> > Changed __submit_zone_reset_cmd to void return.
> > Refactored the f2fs_wait_discard_bio function.
> > Fixed code that was previously incorrectly merged.
> >
> > v1 -> v2
> > Changed to apply the optional async reset write pointer by default.
>
> Don't add the history in the patch description.
OK, I will do.
> >
> > This patch enables submit reset zone command asynchornously. It helps
> > decrease average latency of write IOs in high utilization scenario by
> > faster checkpointing.
> >
> > Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> > ---
> > fs/f2fs/f2fs.h | 1 +
> > fs/f2fs/iostat.c | 1 +
> > fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
> > include/trace/events/f2fs.h | 24 +++++++++--
> > 4 files changed, 104 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index d211ee89c158..51b68a629814 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1175,6 +1175,7 @@ enum iostat_type {
> > /* other */
> > FS_DISCARD_IO, /* discard */
> > FS_FLUSH_IO, /* flush */
> > + FS_ZONE_RESET_IO, /* zone reset */
> > NR_IO_TYPE,
> > };
> >
> > diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
> > index 3d5bfb1ad585..f8703038e1d8 100644
> > --- a/fs/f2fs/iostat.c
> > +++ b/fs/f2fs/iostat.c
> > @@ -80,6 +80,7 @@ int __maybe_unused iostat_info_seq_show(struct seq_file *seq, void *offset)
> > seq_puts(seq, "[OTHER]\n");
> > IOSTAT_INFO_SHOW("fs discard", FS_DISCARD_IO);
> > IOSTAT_INFO_SHOW("fs flush", FS_FLUSH_IO);
> > + IOSTAT_INFO_SHOW("fs zone reset", FS_ZONE_RESET_IO);
> >
> > return 0;
> > }
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 6db410f1bb8c..4802b05a795b 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -1196,6 +1196,45 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
> > static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > struct block_device *bdev, block_t lstart,
> > block_t start, block_t len);
> > +
> > +#ifdef CONFIG_BLK_DEV_ZONED
> > +static void __submit_zone_reset_cmd(struct f2fs_sb_info *sbi,
>
> Why can't we use __f2fs_issue_discard_zone()?
In my understanding, __f2fs_issue_discard_zone is used to queue the zone reset command.
Which point do you think is the issue?
>
> > + struct discard_cmd *dc, blk_opf_t flag,
> > + struct list_head *wait_list,
> > + unsigned int *issued)
> > +{
> > + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > + struct block_device *bdev = dc->bdev;
> > + struct bio *bio = bio_alloc(bdev, 0, REQ_OP_ZONE_RESET | flag, GFP_NOFS);
> > + unsigned long flags;
> > +
> > + trace_f2fs_issue_reset_zone(bdev, dc->di.start);
> > +
> > + spin_lock_irqsave(&dc->lock, flags);
> > + dc->state = D_SUBMIT;
> > + dc->bio_ref++;
> > + spin_unlock_irqrestore(&dc->lock, flags);
> > +
> > + if (issued)
> > + (*issued)++;
> > +
> > + atomic_inc(&dcc->queued_discard);
> > + dc->queued++;
> > + list_move_tail(&dc->list, wait_list);
> > +
> > + /* sanity check on discard range */
> > + __check_sit_bitmap(sbi, dc->di.lstart, dc->di.lstart + dc->di.len);
> > +
> > + bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(dc->di.start);
> > + bio->bi_private = dc;
> > + bio->bi_end_io = f2fs_submit_discard_endio;
> > + submit_bio(bio);
> > +
> > + atomic_inc(&dcc->issued_discard);
> > + f2fs_update_iostat(sbi, NULL, FS_ZONE_RESET_IO, dc->di.len * F2FS_BLKSIZE);
> > +}
> > +#endif
> > +
> > /* this function is copied from blkdev_issue_discard from block/blk-lib.c */
> > static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > struct discard_policy *dpolicy,
> > @@ -1217,6 +1256,13 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> > return 0;
> >
> > +#ifdef CONFIG_BLK_DEV_ZONED
> > + if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev)) {
> > + __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> > + return 0;
> > + }
> > +#endif
> > +
> > trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
> >
> > lstart = dc->di.lstart;
> > @@ -1461,6 +1507,19 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > }
> > }
> >
> > +#ifdef CONFIG_BLK_DEV_ZONED
> > +static void __queue_zone_reset_cmd(struct f2fs_sb_info *sbi,
> > + struct block_device *bdev, block_t blkstart, block_t lblkstart,
> > + block_t blklen)
> > +{
> > + trace_f2fs_queue_reset_zone(bdev, blkstart);
> > +
> > + mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> > + __insert_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
> > + mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> > +}
> > +#endif
> > +
> > static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
> > struct block_device *bdev, block_t blkstart, block_t blklen)
> > {
> > @@ -1724,6 +1783,19 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
> >
> > mutex_lock(&dcc->cmd_lock);
> > dc = __lookup_discard_cmd(sbi, blkaddr);
> > +#ifdef CONFIG_BLK_DEV_ZONED
> > + if (dc && f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
> > + /* force submit zone reset */
> > + if (dc->state == D_PREP)
> > + __submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
> > + &dcc->wait_list, NULL);
> > + dc->ref++;
> > + mutex_unlock(&dcc->cmd_lock);
> > + /* wait zone reset */
> > + __wait_one_discard_bio(sbi, dc);
> > + return;
> > + }
> > +#endif
> > if (dc) {
> > if (dc->state == D_PREP) {
> > __punch_discard_cmd(sbi, dc, blkaddr);
> > @@ -1876,9 +1948,15 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
> > blkstart, blklen);
> > return -EIO;
> > }
> > - trace_f2fs_issue_reset_zone(bdev, blkstart);
> > - return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > - sector, nr_sects, GFP_NOFS);
> > +
> > + if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) {
> > + trace_f2fs_issue_reset_zone(bdev, blkstart);
> > + return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > + sector, nr_sects, GFP_NOFS);
> > + }
> > +
> > + __queue_zone_reset_cmd(sbi, bdev, blkstart, lblkstart, blklen);
> > + return 0;
> > }
> >
> > /* For conventional zones, use regular discard if supported */
> > diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> > index 99cbc5949e3c..793f82cc1515 100644
> > --- a/include/trace/events/f2fs.h
> > +++ b/include/trace/events/f2fs.h
> > @@ -1512,7 +1512,7 @@ DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
> > TP_ARGS(dev, blkstart, blklen)
> > );
> >
> > -TRACE_EVENT(f2fs_issue_reset_zone,
> > +DECLARE_EVENT_CLASS(f2fs_reset_zone,
> >
> > TP_PROTO(struct block_device *dev, block_t blkstart),
> >
> > @@ -1528,11 +1528,25 @@ TRACE_EVENT(f2fs_issue_reset_zone,
> > __entry->blkstart = blkstart;
> > ),
> >
> > - TP_printk("dev = (%d,%d), reset zone at block = 0x%llx",
> > + TP_printk("dev = (%d,%d), zone at block = 0x%llx",
> > show_dev(__entry->dev),
> > (unsigned long long)__entry->blkstart)
> > );
> >
> > +DEFINE_EVENT(f2fs_reset_zone, f2fs_queue_reset_zone,
> > +
> > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > +
> > + TP_ARGS(dev, blkstart)
> > +);
> > +
> > +DEFINE_EVENT(f2fs_reset_zone, f2fs_issue_reset_zone,
> > +
> > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > +
> > + TP_ARGS(dev, blkstart)
> > +);
> > +
> > TRACE_EVENT(f2fs_issue_flush,
> >
> > TP_PROTO(struct block_device *dev, unsigned int nobarrier,
> > @@ -1979,6 +1993,7 @@ TRACE_EVENT(f2fs_iostat,
> > __field(unsigned long long, fs_nrio)
> > __field(unsigned long long, fs_mrio)
> > __field(unsigned long long, fs_discard)
> > + __field(unsigned long long, fs_reset_zone)
> > ),
> >
> > TP_fast_assign(
> > @@ -2010,12 +2025,14 @@ TRACE_EVENT(f2fs_iostat,
> > __entry->fs_nrio = iostat[FS_NODE_READ_IO];
> > __entry->fs_mrio = iostat[FS_META_READ_IO];
> > __entry->fs_discard = iostat[FS_DISCARD_IO];
> > + __entry->fs_reset_zone = iostat[FS_ZONE_RESET_IO];
> > ),
> >
> > TP_printk("dev = (%d,%d), "
> > "app [write=%llu (direct=%llu, buffered=%llu), mapped=%llu, "
> > "compr(buffered=%llu, mapped=%llu)], "
> > - "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu], "
> > + "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu, "
> > + "reset_zone=%llu], "
> > "gc [data=%llu, node=%llu], "
> > "cp [data=%llu, node=%llu, meta=%llu], "
> > "app [read=%llu (direct=%llu, buffered=%llu), mapped=%llu], "
> > @@ -2026,6 +2043,7 @@ TRACE_EVENT(f2fs_iostat,
> > __entry->app_bio, __entry->app_mio, __entry->app_bcdio,
> > __entry->app_mcdio, __entry->fs_dio, __entry->fs_cdio,
> > __entry->fs_nio, __entry->fs_mio, __entry->fs_discard,
> > + __entry->fs_reset_zone,
> > __entry->fs_gc_dio, __entry->fs_gc_nio, __entry->fs_cp_dio,
> > __entry->fs_cp_nio, __entry->fs_cp_mio,
> > __entry->app_rio, __entry->app_drio, __entry->app_brio,
> > --
> > 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 [flat|nested] 18+ messages in thread
* Re: (2) [PATCH v6] f2fs: add async reset zone command support
2023-05-11 5:24 ` [f2fs-dev] (2) " Daejun Park
@ 2023-05-18 1:05 ` Jaegeuk Kim
-1 siblings, 0 replies; 18+ messages in thread
From: Jaegeuk Kim @ 2023-05-18 1:05 UTC (permalink / raw)
To: Daejun Park
Cc: chao, rostedt, mhiramat, linux-f2fs-devel, linux-kernel,
linux-trace-kernel, Seokhwan Kim, Yonggil Song, beomsu kim
On 05/11, Daejun Park wrote:
> > Sender : Jaegeuk Kim <jaegeuk@kernel.org>
> > Date : 2023-05-10 03:53 (GMT+9)
> > Title : Re: [PATCH v6] f2fs: add async reset zone command support
> > To : 박대준<daejun7.park@samsung.com>
> > CC : chao@kernel.org<chao@kernel.org>, rostedt@goodmis.org<rostedt@goodmis.org>, mhiramat@kernel.org<mhiramat@kernel.org>, linux-f2fs-devel@lists.sourceforge.net<linux-f2fs-devel@lists.sourceforge.net>, linux-kernel@vger.kernel.org<linux-kernel@vger.kernel.org>, linux-trace-kernel@vger.kernel.org<linux-trace-kernel@vger.kernel.org>, 김석환<sukka.kim@samsung.com>, 송용길<yonggil.song@samsung.com>, 김범수<beomsu7.kim@samsung.com>
> >
> > On 05/08, Daejun Park wrote:
> > > v5 -> v6
> > > Added trace_f2fs_iostat support for zone reset command.
> > >
> > > v4 -> v5
> > > Added f2fs iostat for zone reset command.
> > >
> > > v3 -> v4
> > > Fixed build error caused by unused function.
> > >
> > > v2 -> v3
> > > Modified arguments to be correct for ftrace parameter.
> > > Changed __submit_zone_reset_cmd to void return.
> > > Refactored the f2fs_wait_discard_bio function.
> > > Fixed code that was previously incorrectly merged.
> > >
> > > v1 -> v2
> > > Changed to apply the optional async reset write pointer by default.
> >
> > Don't add the history in the patch description.
>
> OK, I will do.
>
> > >
> > > This patch enables submit reset zone command asynchornously. It helps
> > > decrease average latency of write IOs in high utilization scenario by
> > > faster checkpointing.
> > >
> > > Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> > > ---
> > > fs/f2fs/f2fs.h | 1 +
> > > fs/f2fs/iostat.c | 1 +
> > > fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
> > > include/trace/events/f2fs.h | 24 +++++++++--
> > > 4 files changed, 104 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index d211ee89c158..51b68a629814 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -1175,6 +1175,7 @@ enum iostat_type {
> > > /* other */
> > > FS_DISCARD_IO, /* discard */
> > > FS_FLUSH_IO, /* flush */
> > > + FS_ZONE_RESET_IO, /* zone reset */
> > > NR_IO_TYPE,
> > > };
> > >
> > > diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
> > > index 3d5bfb1ad585..f8703038e1d8 100644
> > > --- a/fs/f2fs/iostat.c
> > > +++ b/fs/f2fs/iostat.c
> > > @@ -80,6 +80,7 @@ int __maybe_unused iostat_info_seq_show(struct seq_file *seq, void *offset)
> > > seq_puts(seq, "[OTHER]\n");
> > > IOSTAT_INFO_SHOW("fs discard", FS_DISCARD_IO);
> > > IOSTAT_INFO_SHOW("fs flush", FS_FLUSH_IO);
> > > + IOSTAT_INFO_SHOW("fs zone reset", FS_ZONE_RESET_IO);
> > >
> > > return 0;
> > > }
> > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > > index 6db410f1bb8c..4802b05a795b 100644
> > > --- a/fs/f2fs/segment.c
> > > +++ b/fs/f2fs/segment.c
> > > @@ -1196,6 +1196,45 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
> > > static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > > struct block_device *bdev, block_t lstart,
> > > block_t start, block_t len);
> > > +
> > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > +static void __submit_zone_reset_cmd(struct f2fs_sb_info *sbi,
> >
> > Why can't we use __f2fs_issue_discard_zone()?
>
> In my understanding, __f2fs_issue_discard_zone is used to queue the zone reset command.
> Which point do you think is the issue?
That actually calls blkdev_zone_mgmt() which is doing the same thing.
>
> >
> > > + struct discard_cmd *dc, blk_opf_t flag,
> > > + struct list_head *wait_list,
> > > + unsigned int *issued)
> > > +{
> > > + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > > + struct block_device *bdev = dc->bdev;
> > > + struct bio *bio = bio_alloc(bdev, 0, REQ_OP_ZONE_RESET | flag, GFP_NOFS);
> > > + unsigned long flags;
> > > +
> > > + trace_f2fs_issue_reset_zone(bdev, dc->di.start);
> > > +
> > > + spin_lock_irqsave(&dc->lock, flags);
> > > + dc->state = D_SUBMIT;
> > > + dc->bio_ref++;
> > > + spin_unlock_irqrestore(&dc->lock, flags);
> > > +
> > > + if (issued)
> > > + (*issued)++;
> > > +
> > > + atomic_inc(&dcc->queued_discard);
> > > + dc->queued++;
> > > + list_move_tail(&dc->list, wait_list);
> > > +
> > > + /* sanity check on discard range */
> > > + __check_sit_bitmap(sbi, dc->di.lstart, dc->di.lstart + dc->di.len);
> > > +
> > > + bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(dc->di.start);
> > > + bio->bi_private = dc;
> > > + bio->bi_end_io = f2fs_submit_discard_endio;
> > > + submit_bio(bio);
> > > +
> > > + atomic_inc(&dcc->issued_discard);
> > > + f2fs_update_iostat(sbi, NULL, FS_ZONE_RESET_IO, dc->di.len * F2FS_BLKSIZE);
> > > +}
> > > +#endif
> > > +
> > > /* this function is copied from blkdev_issue_discard from block/blk-lib.c */
> > > static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > > struct discard_policy *dpolicy,
> > > @@ -1217,6 +1256,13 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > > if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> > > return 0;
> > >
> > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > + if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev)) {
> > > + __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> > > + return 0;
> > > + }
> > > +#endif
> > > +
> > > trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
> > >
> > > lstart = dc->di.lstart;
> > > @@ -1461,6 +1507,19 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > > }
> > > }
> > >
> > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > +static void __queue_zone_reset_cmd(struct f2fs_sb_info *sbi,
> > > + struct block_device *bdev, block_t blkstart, block_t lblkstart,
> > > + block_t blklen)
> > > +{
> > > + trace_f2fs_queue_reset_zone(bdev, blkstart);
> > > +
> > > + mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> > > + __insert_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
> > > + mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> > > +}
> > > +#endif
> > > +
> > > static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
> > > struct block_device *bdev, block_t blkstart, block_t blklen)
> > > {
> > > @@ -1724,6 +1783,19 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
> > >
> > > mutex_lock(&dcc->cmd_lock);
> > > dc = __lookup_discard_cmd(sbi, blkaddr);
> > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > + if (dc && f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
> > > + /* force submit zone reset */
> > > + if (dc->state == D_PREP)
> > > + __submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
> > > + &dcc->wait_list, NULL);
> > > + dc->ref++;
> > > + mutex_unlock(&dcc->cmd_lock);
> > > + /* wait zone reset */
> > > + __wait_one_discard_bio(sbi, dc);
> > > + return;
> > > + }
> > > +#endif
> > > if (dc) {
> > > if (dc->state == D_PREP) {
> > > __punch_discard_cmd(sbi, dc, blkaddr);
> > > @@ -1876,9 +1948,15 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
> > > blkstart, blklen);
> > > return -EIO;
> > > }
> > > - trace_f2fs_issue_reset_zone(bdev, blkstart);
> > > - return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > > - sector, nr_sects, GFP_NOFS);
> > > +
> > > + if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) {
> > > + trace_f2fs_issue_reset_zone(bdev, blkstart);
> > > + return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > > + sector, nr_sects, GFP_NOFS);
> > > + }
> > > +
> > > + __queue_zone_reset_cmd(sbi, bdev, blkstart, lblkstart, blklen);
> > > + return 0;
> > > }
> > >
> > > /* For conventional zones, use regular discard if supported */
> > > diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> > > index 99cbc5949e3c..793f82cc1515 100644
> > > --- a/include/trace/events/f2fs.h
> > > +++ b/include/trace/events/f2fs.h
> > > @@ -1512,7 +1512,7 @@ DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
> > > TP_ARGS(dev, blkstart, blklen)
> > > );
> > >
> > > -TRACE_EVENT(f2fs_issue_reset_zone,
> > > +DECLARE_EVENT_CLASS(f2fs_reset_zone,
> > >
> > > TP_PROTO(struct block_device *dev, block_t blkstart),
> > >
> > > @@ -1528,11 +1528,25 @@ TRACE_EVENT(f2fs_issue_reset_zone,
> > > __entry->blkstart = blkstart;
> > > ),
> > >
> > > - TP_printk("dev = (%d,%d), reset zone at block = 0x%llx",
> > > + TP_printk("dev = (%d,%d), zone at block = 0x%llx",
> > > show_dev(__entry->dev),
> > > (unsigned long long)__entry->blkstart)
> > > );
> > >
> > > +DEFINE_EVENT(f2fs_reset_zone, f2fs_queue_reset_zone,
> > > +
> > > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > > +
> > > + TP_ARGS(dev, blkstart)
> > > +);
> > > +
> > > +DEFINE_EVENT(f2fs_reset_zone, f2fs_issue_reset_zone,
> > > +
> > > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > > +
> > > + TP_ARGS(dev, blkstart)
> > > +);
> > > +
> > > TRACE_EVENT(f2fs_issue_flush,
> > >
> > > TP_PROTO(struct block_device *dev, unsigned int nobarrier,
> > > @@ -1979,6 +1993,7 @@ TRACE_EVENT(f2fs_iostat,
> > > __field(unsigned long long, fs_nrio)
> > > __field(unsigned long long, fs_mrio)
> > > __field(unsigned long long, fs_discard)
> > > + __field(unsigned long long, fs_reset_zone)
> > > ),
> > >
> > > TP_fast_assign(
> > > @@ -2010,12 +2025,14 @@ TRACE_EVENT(f2fs_iostat,
> > > __entry->fs_nrio = iostat[FS_NODE_READ_IO];
> > > __entry->fs_mrio = iostat[FS_META_READ_IO];
> > > __entry->fs_discard = iostat[FS_DISCARD_IO];
> > > + __entry->fs_reset_zone = iostat[FS_ZONE_RESET_IO];
> > > ),
> > >
> > > TP_printk("dev = (%d,%d), "
> > > "app [write=%llu (direct=%llu, buffered=%llu), mapped=%llu, "
> > > "compr(buffered=%llu, mapped=%llu)], "
> > > - "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu], "
> > > + "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu, "
> > > + "reset_zone=%llu], "
> > > "gc [data=%llu, node=%llu], "
> > > "cp [data=%llu, node=%llu, meta=%llu], "
> > > "app [read=%llu (direct=%llu, buffered=%llu), mapped=%llu], "
> > > @@ -2026,6 +2043,7 @@ TRACE_EVENT(f2fs_iostat,
> > > __entry->app_bio, __entry->app_mio, __entry->app_bcdio,
> > > __entry->app_mcdio, __entry->fs_dio, __entry->fs_cdio,
> > > __entry->fs_nio, __entry->fs_mio, __entry->fs_discard,
> > > + __entry->fs_reset_zone,
> > > __entry->fs_gc_dio, __entry->fs_gc_nio, __entry->fs_cp_dio,
> > > __entry->fs_cp_nio, __entry->fs_cp_mio,
> > > __entry->app_rio, __entry->app_drio, __entry->app_brio,
> > > --
> > > 2.25.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] (2) [PATCH v6] f2fs: add async reset zone command support
@ 2023-05-18 1:05 ` Jaegeuk Kim
0 siblings, 0 replies; 18+ messages in thread
From: Jaegeuk Kim @ 2023-05-18 1:05 UTC (permalink / raw)
To: Daejun Park
Cc: linux-kernel, rostedt, linux-f2fs-devel, mhiramat, Seokhwan Kim,
beomsu kim, linux-trace-kernel
On 05/11, Daejun Park wrote:
> > Sender : Jaegeuk Kim <jaegeuk@kernel.org>
> > Date : 2023-05-10 03:53 (GMT+9)
> > Title : Re: [PATCH v6] f2fs: add async reset zone command support
> > To : 박대준<daejun7.park@samsung.com>
> > CC : chao@kernel.org<chao@kernel.org>, rostedt@goodmis.org<rostedt@goodmis.org>, mhiramat@kernel.org<mhiramat@kernel.org>, linux-f2fs-devel@lists.sourceforge.net<linux-f2fs-devel@lists.sourceforge.net>, linux-kernel@vger.kernel.org<linux-kernel@vger.kernel.org>, linux-trace-kernel@vger.kernel.org<linux-trace-kernel@vger.kernel.org>, 김석환<sukka.kim@samsung.com>, 송용길<yonggil.song@samsung.com>, 김범수<beomsu7.kim@samsung.com>
> >
> > On 05/08, Daejun Park wrote:
> > > v5 -> v6
> > > Added trace_f2fs_iostat support for zone reset command.
> > >
> > > v4 -> v5
> > > Added f2fs iostat for zone reset command.
> > >
> > > v3 -> v4
> > > Fixed build error caused by unused function.
> > >
> > > v2 -> v3
> > > Modified arguments to be correct for ftrace parameter.
> > > Changed __submit_zone_reset_cmd to void return.
> > > Refactored the f2fs_wait_discard_bio function.
> > > Fixed code that was previously incorrectly merged.
> > >
> > > v1 -> v2
> > > Changed to apply the optional async reset write pointer by default.
> >
> > Don't add the history in the patch description.
>
> OK, I will do.
>
> > >
> > > This patch enables submit reset zone command asynchornously. It helps
> > > decrease average latency of write IOs in high utilization scenario by
> > > faster checkpointing.
> > >
> > > Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> > > ---
> > > fs/f2fs/f2fs.h | 1 +
> > > fs/f2fs/iostat.c | 1 +
> > > fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
> > > include/trace/events/f2fs.h | 24 +++++++++--
> > > 4 files changed, 104 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index d211ee89c158..51b68a629814 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -1175,6 +1175,7 @@ enum iostat_type {
> > > /* other */
> > > FS_DISCARD_IO, /* discard */
> > > FS_FLUSH_IO, /* flush */
> > > + FS_ZONE_RESET_IO, /* zone reset */
> > > NR_IO_TYPE,
> > > };
> > >
> > > diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
> > > index 3d5bfb1ad585..f8703038e1d8 100644
> > > --- a/fs/f2fs/iostat.c
> > > +++ b/fs/f2fs/iostat.c
> > > @@ -80,6 +80,7 @@ int __maybe_unused iostat_info_seq_show(struct seq_file *seq, void *offset)
> > > seq_puts(seq, "[OTHER]\n");
> > > IOSTAT_INFO_SHOW("fs discard", FS_DISCARD_IO);
> > > IOSTAT_INFO_SHOW("fs flush", FS_FLUSH_IO);
> > > + IOSTAT_INFO_SHOW("fs zone reset", FS_ZONE_RESET_IO);
> > >
> > > return 0;
> > > }
> > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > > index 6db410f1bb8c..4802b05a795b 100644
> > > --- a/fs/f2fs/segment.c
> > > +++ b/fs/f2fs/segment.c
> > > @@ -1196,6 +1196,45 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
> > > static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > > struct block_device *bdev, block_t lstart,
> > > block_t start, block_t len);
> > > +
> > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > +static void __submit_zone_reset_cmd(struct f2fs_sb_info *sbi,
> >
> > Why can't we use __f2fs_issue_discard_zone()?
>
> In my understanding, __f2fs_issue_discard_zone is used to queue the zone reset command.
> Which point do you think is the issue?
That actually calls blkdev_zone_mgmt() which is doing the same thing.
>
> >
> > > + struct discard_cmd *dc, blk_opf_t flag,
> > > + struct list_head *wait_list,
> > > + unsigned int *issued)
> > > +{
> > > + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > > + struct block_device *bdev = dc->bdev;
> > > + struct bio *bio = bio_alloc(bdev, 0, REQ_OP_ZONE_RESET | flag, GFP_NOFS);
> > > + unsigned long flags;
> > > +
> > > + trace_f2fs_issue_reset_zone(bdev, dc->di.start);
> > > +
> > > + spin_lock_irqsave(&dc->lock, flags);
> > > + dc->state = D_SUBMIT;
> > > + dc->bio_ref++;
> > > + spin_unlock_irqrestore(&dc->lock, flags);
> > > +
> > > + if (issued)
> > > + (*issued)++;
> > > +
> > > + atomic_inc(&dcc->queued_discard);
> > > + dc->queued++;
> > > + list_move_tail(&dc->list, wait_list);
> > > +
> > > + /* sanity check on discard range */
> > > + __check_sit_bitmap(sbi, dc->di.lstart, dc->di.lstart + dc->di.len);
> > > +
> > > + bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(dc->di.start);
> > > + bio->bi_private = dc;
> > > + bio->bi_end_io = f2fs_submit_discard_endio;
> > > + submit_bio(bio);
> > > +
> > > + atomic_inc(&dcc->issued_discard);
> > > + f2fs_update_iostat(sbi, NULL, FS_ZONE_RESET_IO, dc->di.len * F2FS_BLKSIZE);
> > > +}
> > > +#endif
> > > +
> > > /* this function is copied from blkdev_issue_discard from block/blk-lib.c */
> > > static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > > struct discard_policy *dpolicy,
> > > @@ -1217,6 +1256,13 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > > if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> > > return 0;
> > >
> > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > + if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev)) {
> > > + __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> > > + return 0;
> > > + }
> > > +#endif
> > > +
> > > trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
> > >
> > > lstart = dc->di.lstart;
> > > @@ -1461,6 +1507,19 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > > }
> > > }
> > >
> > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > +static void __queue_zone_reset_cmd(struct f2fs_sb_info *sbi,
> > > + struct block_device *bdev, block_t blkstart, block_t lblkstart,
> > > + block_t blklen)
> > > +{
> > > + trace_f2fs_queue_reset_zone(bdev, blkstart);
> > > +
> > > + mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> > > + __insert_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
> > > + mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> > > +}
> > > +#endif
> > > +
> > > static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
> > > struct block_device *bdev, block_t blkstart, block_t blklen)
> > > {
> > > @@ -1724,6 +1783,19 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
> > >
> > > mutex_lock(&dcc->cmd_lock);
> > > dc = __lookup_discard_cmd(sbi, blkaddr);
> > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > + if (dc && f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
> > > + /* force submit zone reset */
> > > + if (dc->state == D_PREP)
> > > + __submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
> > > + &dcc->wait_list, NULL);
> > > + dc->ref++;
> > > + mutex_unlock(&dcc->cmd_lock);
> > > + /* wait zone reset */
> > > + __wait_one_discard_bio(sbi, dc);
> > > + return;
> > > + }
> > > +#endif
> > > if (dc) {
> > > if (dc->state == D_PREP) {
> > > __punch_discard_cmd(sbi, dc, blkaddr);
> > > @@ -1876,9 +1948,15 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
> > > blkstart, blklen);
> > > return -EIO;
> > > }
> > > - trace_f2fs_issue_reset_zone(bdev, blkstart);
> > > - return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > > - sector, nr_sects, GFP_NOFS);
> > > +
> > > + if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) {
> > > + trace_f2fs_issue_reset_zone(bdev, blkstart);
> > > + return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > > + sector, nr_sects, GFP_NOFS);
> > > + }
> > > +
> > > + __queue_zone_reset_cmd(sbi, bdev, blkstart, lblkstart, blklen);
> > > + return 0;
> > > }
> > >
> > > /* For conventional zones, use regular discard if supported */
> > > diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> > > index 99cbc5949e3c..793f82cc1515 100644
> > > --- a/include/trace/events/f2fs.h
> > > +++ b/include/trace/events/f2fs.h
> > > @@ -1512,7 +1512,7 @@ DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
> > > TP_ARGS(dev, blkstart, blklen)
> > > );
> > >
> > > -TRACE_EVENT(f2fs_issue_reset_zone,
> > > +DECLARE_EVENT_CLASS(f2fs_reset_zone,
> > >
> > > TP_PROTO(struct block_device *dev, block_t blkstart),
> > >
> > > @@ -1528,11 +1528,25 @@ TRACE_EVENT(f2fs_issue_reset_zone,
> > > __entry->blkstart = blkstart;
> > > ),
> > >
> > > - TP_printk("dev = (%d,%d), reset zone at block = 0x%llx",
> > > + TP_printk("dev = (%d,%d), zone at block = 0x%llx",
> > > show_dev(__entry->dev),
> > > (unsigned long long)__entry->blkstart)
> > > );
> > >
> > > +DEFINE_EVENT(f2fs_reset_zone, f2fs_queue_reset_zone,
> > > +
> > > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > > +
> > > + TP_ARGS(dev, blkstart)
> > > +);
> > > +
> > > +DEFINE_EVENT(f2fs_reset_zone, f2fs_issue_reset_zone,
> > > +
> > > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > > +
> > > + TP_ARGS(dev, blkstart)
> > > +);
> > > +
> > > TRACE_EVENT(f2fs_issue_flush,
> > >
> > > TP_PROTO(struct block_device *dev, unsigned int nobarrier,
> > > @@ -1979,6 +1993,7 @@ TRACE_EVENT(f2fs_iostat,
> > > __field(unsigned long long, fs_nrio)
> > > __field(unsigned long long, fs_mrio)
> > > __field(unsigned long long, fs_discard)
> > > + __field(unsigned long long, fs_reset_zone)
> > > ),
> > >
> > > TP_fast_assign(
> > > @@ -2010,12 +2025,14 @@ TRACE_EVENT(f2fs_iostat,
> > > __entry->fs_nrio = iostat[FS_NODE_READ_IO];
> > > __entry->fs_mrio = iostat[FS_META_READ_IO];
> > > __entry->fs_discard = iostat[FS_DISCARD_IO];
> > > + __entry->fs_reset_zone = iostat[FS_ZONE_RESET_IO];
> > > ),
> > >
> > > TP_printk("dev = (%d,%d), "
> > > "app [write=%llu (direct=%llu, buffered=%llu), mapped=%llu, "
> > > "compr(buffered=%llu, mapped=%llu)], "
> > > - "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu], "
> > > + "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu, "
> > > + "reset_zone=%llu], "
> > > "gc [data=%llu, node=%llu], "
> > > "cp [data=%llu, node=%llu, meta=%llu], "
> > > "app [read=%llu (direct=%llu, buffered=%llu), mapped=%llu], "
> > > @@ -2026,6 +2043,7 @@ TRACE_EVENT(f2fs_iostat,
> > > __entry->app_bio, __entry->app_mio, __entry->app_bcdio,
> > > __entry->app_mcdio, __entry->fs_dio, __entry->fs_cdio,
> > > __entry->fs_nio, __entry->fs_mio, __entry->fs_discard,
> > > + __entry->fs_reset_zone,
> > > __entry->fs_gc_dio, __entry->fs_gc_nio, __entry->fs_cp_dio,
> > > __entry->fs_cp_nio, __entry->fs_cp_mio,
> > > __entry->app_rio, __entry->app_drio, __entry->app_brio,
> > > --
> > > 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 [flat|nested] 18+ messages in thread
* RE:(2) (2) [PATCH v6] f2fs: add async reset zone command support
[not found] ` <CGME20230508081042epcms2p8a637deae7de1829f54614e09d5fde5e5@epcms2p7>
@ 2023-05-18 2:08 ` Daejun Park
0 siblings, 0 replies; 18+ messages in thread
From: Daejun Park @ 2023-05-18 2:08 UTC (permalink / raw)
To: Jaegeuk Kim, Daejun Park
Cc: chao, rostedt, mhiramat, linux-f2fs-devel, linux-kernel,
linux-trace-kernel, Seokhwan Kim, Yonggil Song, beomsu kim
> On 05/11, Daejun Park wrote:
> > > Sender : Jaegeuk Kim <jaegeuk@kernel.org>
> > > Date : 2023-05-10 03:53 (GMT+9)
> > > Title : Re: [PATCH v6] f2fs: add async reset zone command support
> > > To : 박대준<daejun7.park@samsung.com>
> > > CC : chao@kernel.org<chao@kernel.org>, rostedt@goodmis.org<rostedt@goodmis.org>, mhiramat@kernel.org<mhiramat@kernel.org>, linux-f2fs-devel@lists.sourceforge.net<linux-f2fs-devel@lists.sourceforge.net>, linux-kernel@vger.kernel.org<linux-kernel@vger.kernel.org>, linux-trace-kernel@vger.kernel.org<linux-trace-kernel@vger.kernel.org>, 김석환<sukka.kim@samsung.com>, 송용길<yonggil.song@samsung.com>, 김범수<beomsu7.kim@samsung.com>
> > >
> > > On 05/08, Daejun Park wrote:
> > > > v5 -> v6
> > > > Added trace_f2fs_iostat support for zone reset command.
> > > >
> > > > v4 -> v5
> > > > Added f2fs iostat for zone reset command.
> > > >
> > > > v3 -> v4
> > > > Fixed build error caused by unused function.
> > > >
> > > > v2 -> v3
> > > > Modified arguments to be correct for ftrace parameter.
> > > > Changed __submit_zone_reset_cmd to void return.
> > > > Refactored the f2fs_wait_discard_bio function.
> > > > Fixed code that was previously incorrectly merged.
> > > >
> > > > v1 -> v2
> > > > Changed to apply the optional async reset write pointer by default.
> > >
> > > Don't add the history in the patch description.
> >
> > OK, I will do.
> >
> > > >
> > > > This patch enables submit reset zone command asynchornously. It helps
> > > > decrease average latency of write IOs in high utilization scenario by
> > > > faster checkpointing.
> > > >
> > > > Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> > > > ---
> > > > fs/f2fs/f2fs.h | 1 +
> > > > fs/f2fs/iostat.c | 1 +
> > > > fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
> > > > include/trace/events/f2fs.h | 24 +++++++++--
> > > > 4 files changed, 104 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > index d211ee89c158..51b68a629814 100644
> > > > --- a/fs/f2fs/f2fs.h
> > > > +++ b/fs/f2fs/f2fs.h
> > > > @@ -1175,6 +1175,7 @@ enum iostat_type {
> > > > /* other */
> > > > FS_DISCARD_IO, /* discard */
> > > > FS_FLUSH_IO, /* flush */
> > > > + FS_ZONE_RESET_IO, /* zone reset */
> > > > NR_IO_TYPE,
> > > > };
> > > >
> > > > diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
> > > > index 3d5bfb1ad585..f8703038e1d8 100644
> > > > --- a/fs/f2fs/iostat.c
> > > > +++ b/fs/f2fs/iostat.c
> > > > @@ -80,6 +80,7 @@ int __maybe_unused iostat_info_seq_show(struct seq_file *seq, void *offset)
> > > > seq_puts(seq, "[OTHER]\n");
> > > > IOSTAT_INFO_SHOW("fs discard", FS_DISCARD_IO);
> > > > IOSTAT_INFO_SHOW("fs flush", FS_FLUSH_IO);
> > > > + IOSTAT_INFO_SHOW("fs zone reset", FS_ZONE_RESET_IO);
> > > >
> > > > return 0;
> > > > }
> > > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > > > index 6db410f1bb8c..4802b05a795b 100644
> > > > --- a/fs/f2fs/segment.c
> > > > +++ b/fs/f2fs/segment.c
> > > > @@ -1196,6 +1196,45 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
> > > > static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > > > struct block_device *bdev, block_t lstart,
> > > > block_t start, block_t len);
> > > > +
> > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > +static void __submit_zone_reset_cmd(struct f2fs_sb_info *sbi,
> > >
> > > Why can't we use __f2fs_issue_discard_zone()?
> >
> > In my understanding, __f2fs_issue_discard_zone is used to queue the zone reset command.
> > Which point do you think is the issue?
>
> That actually calls blkdev_zone_mgmt() which is doing the same thing.
f2fs_clear_prefree_segments -> f2fs_issue_discard -> __issue_discard_async -> __f2fs_issue_discard_zone
In the case of discard in the existing code, discard can be queued by __queue_discard_cmd(). However the reset command is issued immediately.
In this patch, I changed the zone reset command can be queued like discard command.
Therefore, I changed __f2fs_issue_discard_zone() to queue the reset command, and to issue the reset command through __submit_discard_cmd() -> __submit_zone_reset_cmd().
Also, in blkdev_zone_mgmt(), bi_end_io cannot be specified, so waiting in f2fs_wait_discard_bio() is not possible.
That's why I created a new __submit_zone_reset_cmd().
>
> >
> > >
> > > > + struct discard_cmd *dc, blk_opf_t flag,
> > > > + struct list_head *wait_list,
> > > > + unsigned int *issued)
> > > > +{
> > > > + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > > > + struct block_device *bdev = dc->bdev;
> > > > + struct bio *bio = bio_alloc(bdev, 0, REQ_OP_ZONE_RESET | flag, GFP_NOFS);
> > > > + unsigned long flags;
> > > > +
> > > > + trace_f2fs_issue_reset_zone(bdev, dc->di.start);
> > > > +
> > > > + spin_lock_irqsave(&dc->lock, flags);
> > > > + dc->state = D_SUBMIT;
> > > > + dc->bio_ref++;
> > > > + spin_unlock_irqrestore(&dc->lock, flags);
> > > > +
> > > > + if (issued)
> > > > + (*issued)++;
> > > > +
> > > > + atomic_inc(&dcc->queued_discard);
> > > > + dc->queued++;
> > > > + list_move_tail(&dc->list, wait_list);
> > > > +
> > > > + /* sanity check on discard range */
> > > > + __check_sit_bitmap(sbi, dc->di.lstart, dc->di.lstart + dc->di.len);
> > > > +
> > > > + bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(dc->di.start);
> > > > + bio->bi_private = dc;
> > > > + bio->bi_end_io = f2fs_submit_discard_endio;
> > > > + submit_bio(bio);
> > > > +
> > > > + atomic_inc(&dcc->issued_discard);
> > > > + f2fs_update_iostat(sbi, NULL, FS_ZONE_RESET_IO, dc->di.len * F2FS_BLKSIZE);
> > > > +}
> > > > +#endif
> > > > +
> > > > /* this function is copied from blkdev_issue_discard from block/blk-lib.c */
> > > > static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > > > struct discard_policy *dpolicy,
> > > > @@ -1217,6 +1256,13 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > > > if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> > > > return 0;
> > > >
> > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > + if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev)) {
> > > > + __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> > > > + return 0;
> > > > + }
> > > > +#endif
> > > > +
> > > > trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
> > > >
> > > > lstart = dc->di.lstart;
> > > > @@ -1461,6 +1507,19 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > > > }
> > > > }
> > > >
> > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > +static void __queue_zone_reset_cmd(struct f2fs_sb_info *sbi,
> > > > + struct block_device *bdev, block_t blkstart, block_t lblkstart,
> > > > + block_t blklen)
> > > > +{
> > > > + trace_f2fs_queue_reset_zone(bdev, blkstart);
> > > > +
> > > > + mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> > > > + __insert_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
> > > > + mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> > > > +}
> > > > +#endif
> > > > +
> > > > static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
> > > > struct block_device *bdev, block_t blkstart, block_t blklen)
> > > > {
> > > > @@ -1724,6 +1783,19 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
> > > >
> > > > mutex_lock(&dcc->cmd_lock);
> > > > dc = __lookup_discard_cmd(sbi, blkaddr);
> > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > + if (dc && f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
> > > > + /* force submit zone reset */
> > > > + if (dc->state == D_PREP)
> > > > + __submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
> > > > + &dcc->wait_list, NULL);
> > > > + dc->ref++;
> > > > + mutex_unlock(&dcc->cmd_lock);
> > > > + /* wait zone reset */
> > > > + __wait_one_discard_bio(sbi, dc);
> > > > + return;
> > > > + }
> > > > +#endif
> > > > if (dc) {
> > > > if (dc->state == D_PREP) {
> > > > __punch_discard_cmd(sbi, dc, blkaddr);
> > > > @@ -1876,9 +1948,15 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
> > > > blkstart, blklen);
> > > > return -EIO;
> > > > }
> > > > - trace_f2fs_issue_reset_zone(bdev, blkstart);
> > > > - return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > > > - sector, nr_sects, GFP_NOFS);
> > > > +
> > > > + if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) {
> > > > + trace_f2fs_issue_reset_zone(bdev, blkstart);
> > > > + return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > > > + sector, nr_sects, GFP_NOFS);
> > > > + }
> > > > +
> > > > + __queue_zone_reset_cmd(sbi, bdev, blkstart, lblkstart, blklen);
> > > > + return 0;
> > > > }
> > > >
> > > > /* For conventional zones, use regular discard if supported */
> > > > diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> > > > index 99cbc5949e3c..793f82cc1515 100644
> > > > --- a/include/trace/events/f2fs.h
> > > > +++ b/include/trace/events/f2fs.h
> > > > @@ -1512,7 +1512,7 @@ DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
> > > > TP_ARGS(dev, blkstart, blklen)
> > > > );
> > > >
> > > > -TRACE_EVENT(f2fs_issue_reset_zone,
> > > > +DECLARE_EVENT_CLASS(f2fs_reset_zone,
> > > >
> > > > TP_PROTO(struct block_device *dev, block_t blkstart),
> > > >
> > > > @@ -1528,11 +1528,25 @@ TRACE_EVENT(f2fs_issue_reset_zone,
> > > > __entry->blkstart = blkstart;
> > > > ),
> > > >
> > > > - TP_printk("dev = (%d,%d), reset zone at block = 0x%llx",
> > > > + TP_printk("dev = (%d,%d), zone at block = 0x%llx",
> > > > show_dev(__entry->dev),
> > > > (unsigned long long)__entry->blkstart)
> > > > );
> > > >
> > > > +DEFINE_EVENT(f2fs_reset_zone, f2fs_queue_reset_zone,
> > > > +
> > > > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > > > +
> > > > + TP_ARGS(dev, blkstart)
> > > > +);
> > > > +
> > > > +DEFINE_EVENT(f2fs_reset_zone, f2fs_issue_reset_zone,
> > > > +
> > > > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > > > +
> > > > + TP_ARGS(dev, blkstart)
> > > > +);
> > > > +
> > > > TRACE_EVENT(f2fs_issue_flush,
> > > >
> > > > TP_PROTO(struct block_device *dev, unsigned int nobarrier,
> > > > @@ -1979,6 +1993,7 @@ TRACE_EVENT(f2fs_iostat,
> > > > __field(unsigned long long, fs_nrio)
> > > > __field(unsigned long long, fs_mrio)
> > > > __field(unsigned long long, fs_discard)
> > > > + __field(unsigned long long, fs_reset_zone)
> > > > ),
> > > >
> > > > TP_fast_assign(
> > > > @@ -2010,12 +2025,14 @@ TRACE_EVENT(f2fs_iostat,
> > > > __entry->fs_nrio = iostat[FS_NODE_READ_IO];
> > > > __entry->fs_mrio = iostat[FS_META_READ_IO];
> > > > __entry->fs_discard = iostat[FS_DISCARD_IO];
> > > > + __entry->fs_reset_zone = iostat[FS_ZONE_RESET_IO];
> > > > ),
> > > >
> > > > TP_printk("dev = (%d,%d), "
> > > > "app [write=%llu (direct=%llu, buffered=%llu), mapped=%llu, "
> > > > "compr(buffered=%llu, mapped=%llu)], "
> > > > - "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu], "
> > > > + "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu, "
> > > > + "reset_zone=%llu], "
> > > > "gc [data=%llu, node=%llu], "
> > > > "cp [data=%llu, node=%llu, meta=%llu], "
> > > > "app [read=%llu (direct=%llu, buffered=%llu), mapped=%llu], "
> > > > @@ -2026,6 +2043,7 @@ TRACE_EVENT(f2fs_iostat,
> > > > __entry->app_bio, __entry->app_mio, __entry->app_bcdio,
> > > > __entry->app_mcdio, __entry->fs_dio, __entry->fs_cdio,
> > > > __entry->fs_nio, __entry->fs_mio, __entry->fs_discard,
> > > > + __entry->fs_reset_zone,
> > > > __entry->fs_gc_dio, __entry->fs_gc_nio, __entry->fs_cp_dio,
> > > > __entry->fs_cp_nio, __entry->fs_cp_mio,
> > > > __entry->app_rio, __entry->app_drio, __entry->app_brio,
> > > > --
> > > > 2.25.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] (2) (2) [PATCH v6] f2fs: add async reset zone command support
@ 2023-05-18 2:08 ` Daejun Park
0 siblings, 0 replies; 18+ messages in thread
From: Daejun Park @ 2023-05-18 2:08 UTC (permalink / raw)
To: Jaegeuk Kim, Daejun Park
Cc: linux-kernel, rostedt, linux-f2fs-devel, mhiramat, Seokhwan Kim,
beomsu kim, linux-trace-kernel
> On 05/11, Daejun Park wrote:
> > > Sender : Jaegeuk Kim <jaegeuk@kernel.org>
> > > Date : 2023-05-10 03:53 (GMT+9)
> > > Title : Re: [PATCH v6] f2fs: add async reset zone command support
> > > To : 박대준<daejun7.park@samsung.com>
> > > CC : chao@kernel.org<chao@kernel.org>, rostedt@goodmis.org<rostedt@goodmis.org>, mhiramat@kernel.org<mhiramat@kernel.org>, linux-f2fs-devel@lists.sourceforge.net<linux-f2fs-devel@lists.sourceforge.net>, linux-kernel@vger.kernel.org<linux-kernel@vger.kernel.org>, linux-trace-kernel@vger.kernel.org<linux-trace-kernel@vger.kernel.org>, 김석환<sukka.kim@samsung.com>, 송용길<yonggil.song@samsung.com>, 김범수<beomsu7.kim@samsung.com>
> > >
> > > On 05/08, Daejun Park wrote:
> > > > v5 -> v6
> > > > Added trace_f2fs_iostat support for zone reset command.
> > > >
> > > > v4 -> v5
> > > > Added f2fs iostat for zone reset command.
> > > >
> > > > v3 -> v4
> > > > Fixed build error caused by unused function.
> > > >
> > > > v2 -> v3
> > > > Modified arguments to be correct for ftrace parameter.
> > > > Changed __submit_zone_reset_cmd to void return.
> > > > Refactored the f2fs_wait_discard_bio function.
> > > > Fixed code that was previously incorrectly merged.
> > > >
> > > > v1 -> v2
> > > > Changed to apply the optional async reset write pointer by default.
> > >
> > > Don't add the history in the patch description.
> >
> > OK, I will do.
> >
> > > >
> > > > This patch enables submit reset zone command asynchornously. It helps
> > > > decrease average latency of write IOs in high utilization scenario by
> > > > faster checkpointing.
> > > >
> > > > Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> > > > ---
> > > > fs/f2fs/f2fs.h | 1 +
> > > > fs/f2fs/iostat.c | 1 +
> > > > fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
> > > > include/trace/events/f2fs.h | 24 +++++++++--
> > > > 4 files changed, 104 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > index d211ee89c158..51b68a629814 100644
> > > > --- a/fs/f2fs/f2fs.h
> > > > +++ b/fs/f2fs/f2fs.h
> > > > @@ -1175,6 +1175,7 @@ enum iostat_type {
> > > > /* other */
> > > > FS_DISCARD_IO, /* discard */
> > > > FS_FLUSH_IO, /* flush */
> > > > + FS_ZONE_RESET_IO, /* zone reset */
> > > > NR_IO_TYPE,
> > > > };
> > > >
> > > > diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
> > > > index 3d5bfb1ad585..f8703038e1d8 100644
> > > > --- a/fs/f2fs/iostat.c
> > > > +++ b/fs/f2fs/iostat.c
> > > > @@ -80,6 +80,7 @@ int __maybe_unused iostat_info_seq_show(struct seq_file *seq, void *offset)
> > > > seq_puts(seq, "[OTHER]\n");
> > > > IOSTAT_INFO_SHOW("fs discard", FS_DISCARD_IO);
> > > > IOSTAT_INFO_SHOW("fs flush", FS_FLUSH_IO);
> > > > + IOSTAT_INFO_SHOW("fs zone reset", FS_ZONE_RESET_IO);
> > > >
> > > > return 0;
> > > > }
> > > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > > > index 6db410f1bb8c..4802b05a795b 100644
> > > > --- a/fs/f2fs/segment.c
> > > > +++ b/fs/f2fs/segment.c
> > > > @@ -1196,6 +1196,45 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
> > > > static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > > > struct block_device *bdev, block_t lstart,
> > > > block_t start, block_t len);
> > > > +
> > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > +static void __submit_zone_reset_cmd(struct f2fs_sb_info *sbi,
> > >
> > > Why can't we use __f2fs_issue_discard_zone()?
> >
> > In my understanding, __f2fs_issue_discard_zone is used to queue the zone reset command.
> > Which point do you think is the issue?
>
> That actually calls blkdev_zone_mgmt() which is doing the same thing.
f2fs_clear_prefree_segments -> f2fs_issue_discard -> __issue_discard_async -> __f2fs_issue_discard_zone
In the case of discard in the existing code, discard can be queued by __queue_discard_cmd(). However the reset command is issued immediately.
In this patch, I changed the zone reset command can be queued like discard command.
Therefore, I changed __f2fs_issue_discard_zone() to queue the reset command, and to issue the reset command through __submit_discard_cmd() -> __submit_zone_reset_cmd().
Also, in blkdev_zone_mgmt(), bi_end_io cannot be specified, so waiting in f2fs_wait_discard_bio() is not possible.
That's why I created a new __submit_zone_reset_cmd().
>
> >
> > >
> > > > + struct discard_cmd *dc, blk_opf_t flag,
> > > > + struct list_head *wait_list,
> > > > + unsigned int *issued)
> > > > +{
> > > > + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > > > + struct block_device *bdev = dc->bdev;
> > > > + struct bio *bio = bio_alloc(bdev, 0, REQ_OP_ZONE_RESET | flag, GFP_NOFS);
> > > > + unsigned long flags;
> > > > +
> > > > + trace_f2fs_issue_reset_zone(bdev, dc->di.start);
> > > > +
> > > > + spin_lock_irqsave(&dc->lock, flags);
> > > > + dc->state = D_SUBMIT;
> > > > + dc->bio_ref++;
> > > > + spin_unlock_irqrestore(&dc->lock, flags);
> > > > +
> > > > + if (issued)
> > > > + (*issued)++;
> > > > +
> > > > + atomic_inc(&dcc->queued_discard);
> > > > + dc->queued++;
> > > > + list_move_tail(&dc->list, wait_list);
> > > > +
> > > > + /* sanity check on discard range */
> > > > + __check_sit_bitmap(sbi, dc->di.lstart, dc->di.lstart + dc->di.len);
> > > > +
> > > > + bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(dc->di.start);
> > > > + bio->bi_private = dc;
> > > > + bio->bi_end_io = f2fs_submit_discard_endio;
> > > > + submit_bio(bio);
> > > > +
> > > > + atomic_inc(&dcc->issued_discard);
> > > > + f2fs_update_iostat(sbi, NULL, FS_ZONE_RESET_IO, dc->di.len * F2FS_BLKSIZE);
> > > > +}
> > > > +#endif
> > > > +
> > > > /* this function is copied from blkdev_issue_discard from block/blk-lib.c */
> > > > static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > > > struct discard_policy *dpolicy,
> > > > @@ -1217,6 +1256,13 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > > > if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> > > > return 0;
> > > >
> > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > + if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev)) {
> > > > + __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> > > > + return 0;
> > > > + }
> > > > +#endif
> > > > +
> > > > trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
> > > >
> > > > lstart = dc->di.lstart;
> > > > @@ -1461,6 +1507,19 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > > > }
> > > > }
> > > >
> > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > +static void __queue_zone_reset_cmd(struct f2fs_sb_info *sbi,
> > > > + struct block_device *bdev, block_t blkstart, block_t lblkstart,
> > > > + block_t blklen)
> > > > +{
> > > > + trace_f2fs_queue_reset_zone(bdev, blkstart);
> > > > +
> > > > + mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> > > > + __insert_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
> > > > + mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> > > > +}
> > > > +#endif
> > > > +
> > > > static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
> > > > struct block_device *bdev, block_t blkstart, block_t blklen)
> > > > {
> > > > @@ -1724,6 +1783,19 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
> > > >
> > > > mutex_lock(&dcc->cmd_lock);
> > > > dc = __lookup_discard_cmd(sbi, blkaddr);
> > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > + if (dc && f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
> > > > + /* force submit zone reset */
> > > > + if (dc->state == D_PREP)
> > > > + __submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
> > > > + &dcc->wait_list, NULL);
> > > > + dc->ref++;
> > > > + mutex_unlock(&dcc->cmd_lock);
> > > > + /* wait zone reset */
> > > > + __wait_one_discard_bio(sbi, dc);
> > > > + return;
> > > > + }
> > > > +#endif
> > > > if (dc) {
> > > > if (dc->state == D_PREP) {
> > > > __punch_discard_cmd(sbi, dc, blkaddr);
> > > > @@ -1876,9 +1948,15 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
> > > > blkstart, blklen);
> > > > return -EIO;
> > > > }
> > > > - trace_f2fs_issue_reset_zone(bdev, blkstart);
> > > > - return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > > > - sector, nr_sects, GFP_NOFS);
> > > > +
> > > > + if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) {
> > > > + trace_f2fs_issue_reset_zone(bdev, blkstart);
> > > > + return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > > > + sector, nr_sects, GFP_NOFS);
> > > > + }
> > > > +
> > > > + __queue_zone_reset_cmd(sbi, bdev, blkstart, lblkstart, blklen);
> > > > + return 0;
> > > > }
> > > >
> > > > /* For conventional zones, use regular discard if supported */
> > > > diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> > > > index 99cbc5949e3c..793f82cc1515 100644
> > > > --- a/include/trace/events/f2fs.h
> > > > +++ b/include/trace/events/f2fs.h
> > > > @@ -1512,7 +1512,7 @@ DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
> > > > TP_ARGS(dev, blkstart, blklen)
> > > > );
> > > >
> > > > -TRACE_EVENT(f2fs_issue_reset_zone,
> > > > +DECLARE_EVENT_CLASS(f2fs_reset_zone,
> > > >
> > > > TP_PROTO(struct block_device *dev, block_t blkstart),
> > > >
> > > > @@ -1528,11 +1528,25 @@ TRACE_EVENT(f2fs_issue_reset_zone,
> > > > __entry->blkstart = blkstart;
> > > > ),
> > > >
> > > > - TP_printk("dev = (%d,%d), reset zone at block = 0x%llx",
> > > > + TP_printk("dev = (%d,%d), zone at block = 0x%llx",
> > > > show_dev(__entry->dev),
> > > > (unsigned long long)__entry->blkstart)
> > > > );
> > > >
> > > > +DEFINE_EVENT(f2fs_reset_zone, f2fs_queue_reset_zone,
> > > > +
> > > > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > > > +
> > > > + TP_ARGS(dev, blkstart)
> > > > +);
> > > > +
> > > > +DEFINE_EVENT(f2fs_reset_zone, f2fs_issue_reset_zone,
> > > > +
> > > > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > > > +
> > > > + TP_ARGS(dev, blkstart)
> > > > +);
> > > > +
> > > > TRACE_EVENT(f2fs_issue_flush,
> > > >
> > > > TP_PROTO(struct block_device *dev, unsigned int nobarrier,
> > > > @@ -1979,6 +1993,7 @@ TRACE_EVENT(f2fs_iostat,
> > > > __field(unsigned long long, fs_nrio)
> > > > __field(unsigned long long, fs_mrio)
> > > > __field(unsigned long long, fs_discard)
> > > > + __field(unsigned long long, fs_reset_zone)
> > > > ),
> > > >
> > > > TP_fast_assign(
> > > > @@ -2010,12 +2025,14 @@ TRACE_EVENT(f2fs_iostat,
> > > > __entry->fs_nrio = iostat[FS_NODE_READ_IO];
> > > > __entry->fs_mrio = iostat[FS_META_READ_IO];
> > > > __entry->fs_discard = iostat[FS_DISCARD_IO];
> > > > + __entry->fs_reset_zone = iostat[FS_ZONE_RESET_IO];
> > > > ),
> > > >
> > > > TP_printk("dev = (%d,%d), "
> > > > "app [write=%llu (direct=%llu, buffered=%llu), mapped=%llu, "
> > > > "compr(buffered=%llu, mapped=%llu)], "
> > > > - "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu], "
> > > > + "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu, "
> > > > + "reset_zone=%llu], "
> > > > "gc [data=%llu, node=%llu], "
> > > > "cp [data=%llu, node=%llu, meta=%llu], "
> > > > "app [read=%llu (direct=%llu, buffered=%llu), mapped=%llu], "
> > > > @@ -2026,6 +2043,7 @@ TRACE_EVENT(f2fs_iostat,
> > > > __entry->app_bio, __entry->app_mio, __entry->app_bcdio,
> > > > __entry->app_mcdio, __entry->fs_dio, __entry->fs_cdio,
> > > > __entry->fs_nio, __entry->fs_mio, __entry->fs_discard,
> > > > + __entry->fs_reset_zone,
> > > > __entry->fs_gc_dio, __entry->fs_gc_nio, __entry->fs_cp_dio,
> > > > __entry->fs_cp_nio, __entry->fs_cp_mio,
> > > > __entry->app_rio, __entry->app_drio, __entry->app_brio,
> > > > --
> > > > 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 [flat|nested] 18+ messages in thread
* Re: [PATCH v6] f2fs: add async reset zone command support
2023-05-09 18:53 ` [f2fs-dev] " Jaegeuk Kim
@ 2023-05-23 14:43 ` Steven Rostedt
-1 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2023-05-23 14:43 UTC (permalink / raw)
To: Jaegeuk Kim
Cc: Daejun Park, chao, mhiramat, linux-f2fs-devel, linux-kernel,
linux-trace-kernel, Seokhwan Kim, Yonggil Song, beomsu kim
On Tue, 9 May 2023 11:53:35 -0700
Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> On 05/08, Daejun Park wrote:
> > v5 -> v6
> > Added trace_f2fs_iostat support for zone reset command.
> >
> > v4 -> v5
> > Added f2fs iostat for zone reset command.
> >
> > v3 -> v4
> > Fixed build error caused by unused function.
> >
> > v2 -> v3
> > Modified arguments to be correct for ftrace parameter.
> > Changed __submit_zone_reset_cmd to void return.
> > Refactored the f2fs_wait_discard_bio function.
> > Fixed code that was previously incorrectly merged.
> >
> > v1 -> v2
> > Changed to apply the optional async reset write pointer by default.
>
> Don't add the history in the patch description.
Well, you can (and should) ...
>
> >
> > This patch enables submit reset zone command asynchornously. It helps
> > decrease average latency of write IOs in high utilization scenario by
> > faster checkpointing.
> >
> > Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> > ---
But it must go below the three dashes above. That will keep 'git am'
from adding it to the change log.
-- Steve
> > fs/f2fs/f2fs.h | 1 +
> > fs/f2fs/iostat.c | 1 +
> > fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
> > include/trace/events/f2fs.h | 24 +++++++++--
> > 4 files changed, 104 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index d211ee89c158..51b68a629814 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1175,6 +1175,7 @@ enum iostat_type {
> > /* other */
> > FS_DISCARD_IO, /* discard */
> > FS_FLUSH_IO, /* flush */
> > + FS_ZONE_RESET_IO, /* zone reset */
> > NR_IO_TYPE,
> > };
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH v6] f2fs: add async reset zone command support
@ 2023-05-23 14:43 ` Steven Rostedt
0 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2023-05-23 14:43 UTC (permalink / raw)
To: Jaegeuk Kim
Cc: linux-kernel, linux-f2fs-devel, mhiramat, Seokhwan Kim,
beomsu kim, linux-trace-kernel
On Tue, 9 May 2023 11:53:35 -0700
Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> On 05/08, Daejun Park wrote:
> > v5 -> v6
> > Added trace_f2fs_iostat support for zone reset command.
> >
> > v4 -> v5
> > Added f2fs iostat for zone reset command.
> >
> > v3 -> v4
> > Fixed build error caused by unused function.
> >
> > v2 -> v3
> > Modified arguments to be correct for ftrace parameter.
> > Changed __submit_zone_reset_cmd to void return.
> > Refactored the f2fs_wait_discard_bio function.
> > Fixed code that was previously incorrectly merged.
> >
> > v1 -> v2
> > Changed to apply the optional async reset write pointer by default.
>
> Don't add the history in the patch description.
Well, you can (and should) ...
>
> >
> > This patch enables submit reset zone command asynchornously. It helps
> > decrease average latency of write IOs in high utilization scenario by
> > faster checkpointing.
> >
> > Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> > ---
But it must go below the three dashes above. That will keep 'git am'
from adding it to the change log.
-- Steve
> > fs/f2fs/f2fs.h | 1 +
> > fs/f2fs/iostat.c | 1 +
> > fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
> > include/trace/events/f2fs.h | 24 +++++++++--
> > 4 files changed, 104 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index d211ee89c158..51b68a629814 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1175,6 +1175,7 @@ enum iostat_type {
> > /* other */
> > FS_DISCARD_IO, /* discard */
> > FS_FLUSH_IO, /* flush */
> > + FS_ZONE_RESET_IO, /* zone reset */
> > NR_IO_TYPE,
> > };
> >
_______________________________________________
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] 18+ messages in thread
* Re: (2) (2) [PATCH v6] f2fs: add async reset zone command support
2023-05-18 2:08 ` [f2fs-dev] (2) " Daejun Park
@ 2023-05-24 1:19 ` Jaegeuk Kim
-1 siblings, 0 replies; 18+ messages in thread
From: Jaegeuk Kim @ 2023-05-24 1:19 UTC (permalink / raw)
To: Daejun Park
Cc: chao, rostedt, mhiramat, linux-f2fs-devel, linux-kernel,
linux-trace-kernel, Seokhwan Kim, Yonggil Song, beomsu kim
On 05/18, Daejun Park wrote:
> > On 05/11, Daejun Park wrote:
> > > > Sender : Jaegeuk Kim <jaegeuk@kernel.org>
> > > > Date : 2023-05-10 03:53 (GMT+9)
> > > > Title : Re: [PATCH v6] f2fs: add async reset zone command support
> > > > To : 박대준<daejun7.park@samsung.com>
> > > > CC : chao@kernel.org<chao@kernel.org>, rostedt@goodmis.org<rostedt@goodmis.org>, mhiramat@kernel.org<mhiramat@kernel.org>, linux-f2fs-devel@lists.sourceforge.net<linux-f2fs-devel@lists.sourceforge.net>, linux-kernel@vger.kernel.org<linux-kernel@vger.kernel.org>, linux-trace-kernel@vger.kernel.org<linux-trace-kernel@vger.kernel.org>, 김석환<sukka.kim@samsung.com>, 송용길<yonggil.song@samsung.com>, 김범수<beomsu7.kim@samsung.com>
> > > >
> > > > On 05/08, Daejun Park wrote:
> > > > > v5 -> v6
> > > > > Added trace_f2fs_iostat support for zone reset command.
> > > > >
> > > > > v4 -> v5
> > > > > Added f2fs iostat for zone reset command.
> > > > >
> > > > > v3 -> v4
> > > > > Fixed build error caused by unused function.
> > > > >
> > > > > v2 -> v3
> > > > > Modified arguments to be correct for ftrace parameter.
> > > > > Changed __submit_zone_reset_cmd to void return.
> > > > > Refactored the f2fs_wait_discard_bio function.
> > > > > Fixed code that was previously incorrectly merged.
> > > > >
> > > > > v1 -> v2
> > > > > Changed to apply the optional async reset write pointer by default.
> > > >
> > > > Don't add the history in the patch description.
> > >
> > > OK, I will do.
> > >
> > > > >
> > > > > This patch enables submit reset zone command asynchornously. It helps
> > > > > decrease average latency of write IOs in high utilization scenario by
> > > > > faster checkpointing.
> > > > >
> > > > > Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> > > > > ---
> > > > > fs/f2fs/f2fs.h | 1 +
> > > > > fs/f2fs/iostat.c | 1 +
> > > > > fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
> > > > > include/trace/events/f2fs.h | 24 +++++++++--
> > > > > 4 files changed, 104 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > > index d211ee89c158..51b68a629814 100644
> > > > > --- a/fs/f2fs/f2fs.h
> > > > > +++ b/fs/f2fs/f2fs.h
> > > > > @@ -1175,6 +1175,7 @@ enum iostat_type {
> > > > > /* other */
> > > > > FS_DISCARD_IO, /* discard */
> > > > > FS_FLUSH_IO, /* flush */
> > > > > + FS_ZONE_RESET_IO, /* zone reset */
> > > > > NR_IO_TYPE,
> > > > > };
> > > > >
> > > > > diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
> > > > > index 3d5bfb1ad585..f8703038e1d8 100644
> > > > > --- a/fs/f2fs/iostat.c
> > > > > +++ b/fs/f2fs/iostat.c
> > > > > @@ -80,6 +80,7 @@ int __maybe_unused iostat_info_seq_show(struct seq_file *seq, void *offset)
> > > > > seq_puts(seq, "[OTHER]\n");
> > > > > IOSTAT_INFO_SHOW("fs discard", FS_DISCARD_IO);
> > > > > IOSTAT_INFO_SHOW("fs flush", FS_FLUSH_IO);
> > > > > + IOSTAT_INFO_SHOW("fs zone reset", FS_ZONE_RESET_IO);
> > > > >
> > > > > return 0;
> > > > > }
> > > > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > > > > index 6db410f1bb8c..4802b05a795b 100644
> > > > > --- a/fs/f2fs/segment.c
> > > > > +++ b/fs/f2fs/segment.c
> > > > > @@ -1196,6 +1196,45 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
> > > > > static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > > > > struct block_device *bdev, block_t lstart,
> > > > > block_t start, block_t len);
> > > > > +
> > > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > > +static void __submit_zone_reset_cmd(struct f2fs_sb_info *sbi,
> > > >
> > > > Why can't we use __f2fs_issue_discard_zone()?
> > >
> > > In my understanding, __f2fs_issue_discard_zone is used to queue the zone reset command.
> > > Which point do you think is the issue?
> >
> > That actually calls blkdev_zone_mgmt() which is doing the same thing.
>
> f2fs_clear_prefree_segments -> f2fs_issue_discard -> __issue_discard_async -> __f2fs_issue_discard_zone
> In the case of discard in the existing code, discard can be queued by __queue_discard_cmd(). However the reset command is issued immediately.
>
> In this patch, I changed the zone reset command can be queued like discard command.
> Therefore, I changed __f2fs_issue_discard_zone() to queue the reset command, and to issue the reset command through __submit_discard_cmd() -> __submit_zone_reset_cmd().
>
> Also, in blkdev_zone_mgmt(), bi_end_io cannot be specified, so waiting in f2fs_wait_discard_bio() is not possible.
> That's why I created a new __submit_zone_reset_cmd().
Hmm, I was thinking whether there's any chance to reuse __queue_discard_cmd() in
__issue_discard_async() for the zone_reset command. Let me take a look at in
another view.
>
> >
> > >
> > > >
> > > > > + struct discard_cmd *dc, blk_opf_t flag,
> > > > > + struct list_head *wait_list,
> > > > > + unsigned int *issued)
> > > > > +{
> > > > > + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > > > > + struct block_device *bdev = dc->bdev;
> > > > > + struct bio *bio = bio_alloc(bdev, 0, REQ_OP_ZONE_RESET | flag, GFP_NOFS);
> > > > > + unsigned long flags;
> > > > > +
> > > > > + trace_f2fs_issue_reset_zone(bdev, dc->di.start);
> > > > > +
> > > > > + spin_lock_irqsave(&dc->lock, flags);
> > > > > + dc->state = D_SUBMIT;
> > > > > + dc->bio_ref++;
> > > > > + spin_unlock_irqrestore(&dc->lock, flags);
> > > > > +
> > > > > + if (issued)
> > > > > + (*issued)++;
> > > > > +
> > > > > + atomic_inc(&dcc->queued_discard);
> > > > > + dc->queued++;
> > > > > + list_move_tail(&dc->list, wait_list);
> > > > > +
> > > > > + /* sanity check on discard range */
> > > > > + __check_sit_bitmap(sbi, dc->di.lstart, dc->di.lstart + dc->di.len);
> > > > > +
> > > > > + bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(dc->di.start);
> > > > > + bio->bi_private = dc;
> > > > > + bio->bi_end_io = f2fs_submit_discard_endio;
> > > > > + submit_bio(bio);
> > > > > +
> > > > > + atomic_inc(&dcc->issued_discard);
> > > > > + f2fs_update_iostat(sbi, NULL, FS_ZONE_RESET_IO, dc->di.len * F2FS_BLKSIZE);
> > > > > +}
> > > > > +#endif
> > > > > +
> > > > > /* this function is copied from blkdev_issue_discard from block/blk-lib.c */
> > > > > static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > > > > struct discard_policy *dpolicy,
> > > > > @@ -1217,6 +1256,13 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > > > > if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> > > > > return 0;
> > > > >
> > > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > > + if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev)) {
> > > > > + __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> > > > > + return 0;
> > > > > + }
> > > > > +#endif
> > > > > +
> > > > > trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
> > > > >
> > > > > lstart = dc->di.lstart;
> > > > > @@ -1461,6 +1507,19 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > > > > }
> > > > > }
> > > > >
> > > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > > +static void __queue_zone_reset_cmd(struct f2fs_sb_info *sbi,
> > > > > + struct block_device *bdev, block_t blkstart, block_t lblkstart,
> > > > > + block_t blklen)
> > > > > +{
> > > > > + trace_f2fs_queue_reset_zone(bdev, blkstart);
> > > > > +
> > > > > + mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> > > > > + __insert_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
> > > > > + mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> > > > > +}
> > > > > +#endif
> > > > > +
> > > > > static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
> > > > > struct block_device *bdev, block_t blkstart, block_t blklen)
> > > > > {
> > > > > @@ -1724,6 +1783,19 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
> > > > >
> > > > > mutex_lock(&dcc->cmd_lock);
> > > > > dc = __lookup_discard_cmd(sbi, blkaddr);
> > > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > > + if (dc && f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
> > > > > + /* force submit zone reset */
> > > > > + if (dc->state == D_PREP)
> > > > > + __submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
> > > > > + &dcc->wait_list, NULL);
> > > > > + dc->ref++;
> > > > > + mutex_unlock(&dcc->cmd_lock);
> > > > > + /* wait zone reset */
> > > > > + __wait_one_discard_bio(sbi, dc);
> > > > > + return;
> > > > > + }
> > > > > +#endif
> > > > > if (dc) {
> > > > > if (dc->state == D_PREP) {
> > > > > __punch_discard_cmd(sbi, dc, blkaddr);
> > > > > @@ -1876,9 +1948,15 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
> > > > > blkstart, blklen);
> > > > > return -EIO;
> > > > > }
> > > > > - trace_f2fs_issue_reset_zone(bdev, blkstart);
> > > > > - return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > > > > - sector, nr_sects, GFP_NOFS);
> > > > > +
> > > > > + if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) {
> > > > > + trace_f2fs_issue_reset_zone(bdev, blkstart);
> > > > > + return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > > > > + sector, nr_sects, GFP_NOFS);
> > > > > + }
> > > > > +
> > > > > + __queue_zone_reset_cmd(sbi, bdev, blkstart, lblkstart, blklen);
> > > > > + return 0;
> > > > > }
> > > > >
> > > > > /* For conventional zones, use regular discard if supported */
> > > > > diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> > > > > index 99cbc5949e3c..793f82cc1515 100644
> > > > > --- a/include/trace/events/f2fs.h
> > > > > +++ b/include/trace/events/f2fs.h
> > > > > @@ -1512,7 +1512,7 @@ DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
> > > > > TP_ARGS(dev, blkstart, blklen)
> > > > > );
> > > > >
> > > > > -TRACE_EVENT(f2fs_issue_reset_zone,
> > > > > +DECLARE_EVENT_CLASS(f2fs_reset_zone,
> > > > >
> > > > > TP_PROTO(struct block_device *dev, block_t blkstart),
> > > > >
> > > > > @@ -1528,11 +1528,25 @@ TRACE_EVENT(f2fs_issue_reset_zone,
> > > > > __entry->blkstart = blkstart;
> > > > > ),
> > > > >
> > > > > - TP_printk("dev = (%d,%d), reset zone at block = 0x%llx",
> > > > > + TP_printk("dev = (%d,%d), zone at block = 0x%llx",
> > > > > show_dev(__entry->dev),
> > > > > (unsigned long long)__entry->blkstart)
> > > > > );
> > > > >
> > > > > +DEFINE_EVENT(f2fs_reset_zone, f2fs_queue_reset_zone,
> > > > > +
> > > > > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > > > > +
> > > > > + TP_ARGS(dev, blkstart)
> > > > > +);
> > > > > +
> > > > > +DEFINE_EVENT(f2fs_reset_zone, f2fs_issue_reset_zone,
> > > > > +
> > > > > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > > > > +
> > > > > + TP_ARGS(dev, blkstart)
> > > > > +);
> > > > > +
> > > > > TRACE_EVENT(f2fs_issue_flush,
> > > > >
> > > > > TP_PROTO(struct block_device *dev, unsigned int nobarrier,
> > > > > @@ -1979,6 +1993,7 @@ TRACE_EVENT(f2fs_iostat,
> > > > > __field(unsigned long long, fs_nrio)
> > > > > __field(unsigned long long, fs_mrio)
> > > > > __field(unsigned long long, fs_discard)
> > > > > + __field(unsigned long long, fs_reset_zone)
> > > > > ),
> > > > >
> > > > > TP_fast_assign(
> > > > > @@ -2010,12 +2025,14 @@ TRACE_EVENT(f2fs_iostat,
> > > > > __entry->fs_nrio = iostat[FS_NODE_READ_IO];
> > > > > __entry->fs_mrio = iostat[FS_META_READ_IO];
> > > > > __entry->fs_discard = iostat[FS_DISCARD_IO];
> > > > > + __entry->fs_reset_zone = iostat[FS_ZONE_RESET_IO];
> > > > > ),
> > > > >
> > > > > TP_printk("dev = (%d,%d), "
> > > > > "app [write=%llu (direct=%llu, buffered=%llu), mapped=%llu, "
> > > > > "compr(buffered=%llu, mapped=%llu)], "
> > > > > - "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu], "
> > > > > + "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu, "
> > > > > + "reset_zone=%llu], "
> > > > > "gc [data=%llu, node=%llu], "
> > > > > "cp [data=%llu, node=%llu, meta=%llu], "
> > > > > "app [read=%llu (direct=%llu, buffered=%llu), mapped=%llu], "
> > > > > @@ -2026,6 +2043,7 @@ TRACE_EVENT(f2fs_iostat,
> > > > > __entry->app_bio, __entry->app_mio, __entry->app_bcdio,
> > > > > __entry->app_mcdio, __entry->fs_dio, __entry->fs_cdio,
> > > > > __entry->fs_nio, __entry->fs_mio, __entry->fs_discard,
> > > > > + __entry->fs_reset_zone,
> > > > > __entry->fs_gc_dio, __entry->fs_gc_nio, __entry->fs_cp_dio,
> > > > > __entry->fs_cp_nio, __entry->fs_cp_mio,
> > > > > __entry->app_rio, __entry->app_drio, __entry->app_brio,
> > > > > --
> > > > > 2.25.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] (2) (2) [PATCH v6] f2fs: add async reset zone command support
@ 2023-05-24 1:19 ` Jaegeuk Kim
0 siblings, 0 replies; 18+ messages in thread
From: Jaegeuk Kim @ 2023-05-24 1:19 UTC (permalink / raw)
To: Daejun Park
Cc: linux-kernel, rostedt, linux-f2fs-devel, mhiramat, Seokhwan Kim,
beomsu kim, linux-trace-kernel
On 05/18, Daejun Park wrote:
> > On 05/11, Daejun Park wrote:
> > > > Sender : Jaegeuk Kim <jaegeuk@kernel.org>
> > > > Date : 2023-05-10 03:53 (GMT+9)
> > > > Title : Re: [PATCH v6] f2fs: add async reset zone command support
> > > > To : 박대준<daejun7.park@samsung.com>
> > > > CC : chao@kernel.org<chao@kernel.org>, rostedt@goodmis.org<rostedt@goodmis.org>, mhiramat@kernel.org<mhiramat@kernel.org>, linux-f2fs-devel@lists.sourceforge.net<linux-f2fs-devel@lists.sourceforge.net>, linux-kernel@vger.kernel.org<linux-kernel@vger.kernel.org>, linux-trace-kernel@vger.kernel.org<linux-trace-kernel@vger.kernel.org>, 김석환<sukka.kim@samsung.com>, 송용길<yonggil.song@samsung.com>, 김범수<beomsu7.kim@samsung.com>
> > > >
> > > > On 05/08, Daejun Park wrote:
> > > > > v5 -> v6
> > > > > Added trace_f2fs_iostat support for zone reset command.
> > > > >
> > > > > v4 -> v5
> > > > > Added f2fs iostat for zone reset command.
> > > > >
> > > > > v3 -> v4
> > > > > Fixed build error caused by unused function.
> > > > >
> > > > > v2 -> v3
> > > > > Modified arguments to be correct for ftrace parameter.
> > > > > Changed __submit_zone_reset_cmd to void return.
> > > > > Refactored the f2fs_wait_discard_bio function.
> > > > > Fixed code that was previously incorrectly merged.
> > > > >
> > > > > v1 -> v2
> > > > > Changed to apply the optional async reset write pointer by default.
> > > >
> > > > Don't add the history in the patch description.
> > >
> > > OK, I will do.
> > >
> > > > >
> > > > > This patch enables submit reset zone command asynchornously. It helps
> > > > > decrease average latency of write IOs in high utilization scenario by
> > > > > faster checkpointing.
> > > > >
> > > > > Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> > > > > ---
> > > > > fs/f2fs/f2fs.h | 1 +
> > > > > fs/f2fs/iostat.c | 1 +
> > > > > fs/f2fs/segment.c | 84 +++++++++++++++++++++++++++++++++++--
> > > > > include/trace/events/f2fs.h | 24 +++++++++--
> > > > > 4 files changed, 104 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > > index d211ee89c158..51b68a629814 100644
> > > > > --- a/fs/f2fs/f2fs.h
> > > > > +++ b/fs/f2fs/f2fs.h
> > > > > @@ -1175,6 +1175,7 @@ enum iostat_type {
> > > > > /* other */
> > > > > FS_DISCARD_IO, /* discard */
> > > > > FS_FLUSH_IO, /* flush */
> > > > > + FS_ZONE_RESET_IO, /* zone reset */
> > > > > NR_IO_TYPE,
> > > > > };
> > > > >
> > > > > diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
> > > > > index 3d5bfb1ad585..f8703038e1d8 100644
> > > > > --- a/fs/f2fs/iostat.c
> > > > > +++ b/fs/f2fs/iostat.c
> > > > > @@ -80,6 +80,7 @@ int __maybe_unused iostat_info_seq_show(struct seq_file *seq, void *offset)
> > > > > seq_puts(seq, "[OTHER]\n");
> > > > > IOSTAT_INFO_SHOW("fs discard", FS_DISCARD_IO);
> > > > > IOSTAT_INFO_SHOW("fs flush", FS_FLUSH_IO);
> > > > > + IOSTAT_INFO_SHOW("fs zone reset", FS_ZONE_RESET_IO);
> > > > >
> > > > > return 0;
> > > > > }
> > > > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > > > > index 6db410f1bb8c..4802b05a795b 100644
> > > > > --- a/fs/f2fs/segment.c
> > > > > +++ b/fs/f2fs/segment.c
> > > > > @@ -1196,6 +1196,45 @@ static void __init_discard_policy(struct f2fs_sb_info *sbi,
> > > > > static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > > > > struct block_device *bdev, block_t lstart,
> > > > > block_t start, block_t len);
> > > > > +
> > > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > > +static void __submit_zone_reset_cmd(struct f2fs_sb_info *sbi,
> > > >
> > > > Why can't we use __f2fs_issue_discard_zone()?
> > >
> > > In my understanding, __f2fs_issue_discard_zone is used to queue the zone reset command.
> > > Which point do you think is the issue?
> >
> > That actually calls blkdev_zone_mgmt() which is doing the same thing.
>
> f2fs_clear_prefree_segments -> f2fs_issue_discard -> __issue_discard_async -> __f2fs_issue_discard_zone
> In the case of discard in the existing code, discard can be queued by __queue_discard_cmd(). However the reset command is issued immediately.
>
> In this patch, I changed the zone reset command can be queued like discard command.
> Therefore, I changed __f2fs_issue_discard_zone() to queue the reset command, and to issue the reset command through __submit_discard_cmd() -> __submit_zone_reset_cmd().
>
> Also, in blkdev_zone_mgmt(), bi_end_io cannot be specified, so waiting in f2fs_wait_discard_bio() is not possible.
> That's why I created a new __submit_zone_reset_cmd().
Hmm, I was thinking whether there's any chance to reuse __queue_discard_cmd() in
__issue_discard_async() for the zone_reset command. Let me take a look at in
another view.
>
> >
> > >
> > > >
> > > > > + struct discard_cmd *dc, blk_opf_t flag,
> > > > > + struct list_head *wait_list,
> > > > > + unsigned int *issued)
> > > > > +{
> > > > > + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > > > > + struct block_device *bdev = dc->bdev;
> > > > > + struct bio *bio = bio_alloc(bdev, 0, REQ_OP_ZONE_RESET | flag, GFP_NOFS);
> > > > > + unsigned long flags;
> > > > > +
> > > > > + trace_f2fs_issue_reset_zone(bdev, dc->di.start);
> > > > > +
> > > > > + spin_lock_irqsave(&dc->lock, flags);
> > > > > + dc->state = D_SUBMIT;
> > > > > + dc->bio_ref++;
> > > > > + spin_unlock_irqrestore(&dc->lock, flags);
> > > > > +
> > > > > + if (issued)
> > > > > + (*issued)++;
> > > > > +
> > > > > + atomic_inc(&dcc->queued_discard);
> > > > > + dc->queued++;
> > > > > + list_move_tail(&dc->list, wait_list);
> > > > > +
> > > > > + /* sanity check on discard range */
> > > > > + __check_sit_bitmap(sbi, dc->di.lstart, dc->di.lstart + dc->di.len);
> > > > > +
> > > > > + bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(dc->di.start);
> > > > > + bio->bi_private = dc;
> > > > > + bio->bi_end_io = f2fs_submit_discard_endio;
> > > > > + submit_bio(bio);
> > > > > +
> > > > > + atomic_inc(&dcc->issued_discard);
> > > > > + f2fs_update_iostat(sbi, NULL, FS_ZONE_RESET_IO, dc->di.len * F2FS_BLKSIZE);
> > > > > +}
> > > > > +#endif
> > > > > +
> > > > > /* this function is copied from blkdev_issue_discard from block/blk-lib.c */
> > > > > static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > > > > struct discard_policy *dpolicy,
> > > > > @@ -1217,6 +1256,13 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> > > > > if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> > > > > return 0;
> > > > >
> > > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > > + if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev)) {
> > > > > + __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> > > > > + return 0;
> > > > > + }
> > > > > +#endif
> > > > > +
> > > > > trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
> > > > >
> > > > > lstart = dc->di.lstart;
> > > > > @@ -1461,6 +1507,19 @@ static void __update_discard_tree_range(struct f2fs_sb_info *sbi,
> > > > > }
> > > > > }
> > > > >
> > > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > > +static void __queue_zone_reset_cmd(struct f2fs_sb_info *sbi,
> > > > > + struct block_device *bdev, block_t blkstart, block_t lblkstart,
> > > > > + block_t blklen)
> > > > > +{
> > > > > + trace_f2fs_queue_reset_zone(bdev, blkstart);
> > > > > +
> > > > > + mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> > > > > + __insert_discard_cmd(sbi, bdev, lblkstart, blkstart, blklen);
> > > > > + mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> > > > > +}
> > > > > +#endif
> > > > > +
> > > > > static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
> > > > > struct block_device *bdev, block_t blkstart, block_t blklen)
> > > > > {
> > > > > @@ -1724,6 +1783,19 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
> > > > >
> > > > > mutex_lock(&dcc->cmd_lock);
> > > > > dc = __lookup_discard_cmd(sbi, blkaddr);
> > > > > +#ifdef CONFIG_BLK_DEV_ZONED
> > > > > + if (dc && f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
> > > > > + /* force submit zone reset */
> > > > > + if (dc->state == D_PREP)
> > > > > + __submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
> > > > > + &dcc->wait_list, NULL);
> > > > > + dc->ref++;
> > > > > + mutex_unlock(&dcc->cmd_lock);
> > > > > + /* wait zone reset */
> > > > > + __wait_one_discard_bio(sbi, dc);
> > > > > + return;
> > > > > + }
> > > > > +#endif
> > > > > if (dc) {
> > > > > if (dc->state == D_PREP) {
> > > > > __punch_discard_cmd(sbi, dc, blkaddr);
> > > > > @@ -1876,9 +1948,15 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi,
> > > > > blkstart, blklen);
> > > > > return -EIO;
> > > > > }
> > > > > - trace_f2fs_issue_reset_zone(bdev, blkstart);
> > > > > - return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > > > > - sector, nr_sects, GFP_NOFS);
> > > > > +
> > > > > + if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) {
> > > > > + trace_f2fs_issue_reset_zone(bdev, blkstart);
> > > > > + return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
> > > > > + sector, nr_sects, GFP_NOFS);
> > > > > + }
> > > > > +
> > > > > + __queue_zone_reset_cmd(sbi, bdev, blkstart, lblkstart, blklen);
> > > > > + return 0;
> > > > > }
> > > > >
> > > > > /* For conventional zones, use regular discard if supported */
> > > > > diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> > > > > index 99cbc5949e3c..793f82cc1515 100644
> > > > > --- a/include/trace/events/f2fs.h
> > > > > +++ b/include/trace/events/f2fs.h
> > > > > @@ -1512,7 +1512,7 @@ DEFINE_EVENT(f2fs_discard, f2fs_remove_discard,
> > > > > TP_ARGS(dev, blkstart, blklen)
> > > > > );
> > > > >
> > > > > -TRACE_EVENT(f2fs_issue_reset_zone,
> > > > > +DECLARE_EVENT_CLASS(f2fs_reset_zone,
> > > > >
> > > > > TP_PROTO(struct block_device *dev, block_t blkstart),
> > > > >
> > > > > @@ -1528,11 +1528,25 @@ TRACE_EVENT(f2fs_issue_reset_zone,
> > > > > __entry->blkstart = blkstart;
> > > > > ),
> > > > >
> > > > > - TP_printk("dev = (%d,%d), reset zone at block = 0x%llx",
> > > > > + TP_printk("dev = (%d,%d), zone at block = 0x%llx",
> > > > > show_dev(__entry->dev),
> > > > > (unsigned long long)__entry->blkstart)
> > > > > );
> > > > >
> > > > > +DEFINE_EVENT(f2fs_reset_zone, f2fs_queue_reset_zone,
> > > > > +
> > > > > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > > > > +
> > > > > + TP_ARGS(dev, blkstart)
> > > > > +);
> > > > > +
> > > > > +DEFINE_EVENT(f2fs_reset_zone, f2fs_issue_reset_zone,
> > > > > +
> > > > > + TP_PROTO(struct block_device *dev, block_t blkstart),
> > > > > +
> > > > > + TP_ARGS(dev, blkstart)
> > > > > +);
> > > > > +
> > > > > TRACE_EVENT(f2fs_issue_flush,
> > > > >
> > > > > TP_PROTO(struct block_device *dev, unsigned int nobarrier,
> > > > > @@ -1979,6 +1993,7 @@ TRACE_EVENT(f2fs_iostat,
> > > > > __field(unsigned long long, fs_nrio)
> > > > > __field(unsigned long long, fs_mrio)
> > > > > __field(unsigned long long, fs_discard)
> > > > > + __field(unsigned long long, fs_reset_zone)
> > > > > ),
> > > > >
> > > > > TP_fast_assign(
> > > > > @@ -2010,12 +2025,14 @@ TRACE_EVENT(f2fs_iostat,
> > > > > __entry->fs_nrio = iostat[FS_NODE_READ_IO];
> > > > > __entry->fs_mrio = iostat[FS_META_READ_IO];
> > > > > __entry->fs_discard = iostat[FS_DISCARD_IO];
> > > > > + __entry->fs_reset_zone = iostat[FS_ZONE_RESET_IO];
> > > > > ),
> > > > >
> > > > > TP_printk("dev = (%d,%d), "
> > > > > "app [write=%llu (direct=%llu, buffered=%llu), mapped=%llu, "
> > > > > "compr(buffered=%llu, mapped=%llu)], "
> > > > > - "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu], "
> > > > > + "fs [data=%llu, cdata=%llu, node=%llu, meta=%llu, discard=%llu, "
> > > > > + "reset_zone=%llu], "
> > > > > "gc [data=%llu, node=%llu], "
> > > > > "cp [data=%llu, node=%llu, meta=%llu], "
> > > > > "app [read=%llu (direct=%llu, buffered=%llu), mapped=%llu], "
> > > > > @@ -2026,6 +2043,7 @@ TRACE_EVENT(f2fs_iostat,
> > > > > __entry->app_bio, __entry->app_mio, __entry->app_bcdio,
> > > > > __entry->app_mcdio, __entry->fs_dio, __entry->fs_cdio,
> > > > > __entry->fs_nio, __entry->fs_mio, __entry->fs_discard,
> > > > > + __entry->fs_reset_zone,
> > > > > __entry->fs_gc_dio, __entry->fs_gc_nio, __entry->fs_cp_dio,
> > > > > __entry->fs_cp_nio, __entry->fs_cp_mio,
> > > > > __entry->app_rio, __entry->app_drio, __entry->app_brio,
> > > > > --
> > > > > 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 [flat|nested] 18+ messages in thread
* Re: [PATCH v6] f2fs: add async reset zone command support
2023-05-09 18:53 ` [f2fs-dev] " Jaegeuk Kim
@ 2023-05-25 1:20 ` Chao Yu
-1 siblings, 0 replies; 18+ messages in thread
From: Chao Yu @ 2023-05-25 1:20 UTC (permalink / raw)
To: Jaegeuk Kim, Daejun Park
Cc: rostedt, mhiramat, linux-f2fs-devel, linux-kernel,
linux-trace-kernel, Seokhwan Kim, Yonggil Song, beomsu kim
On 2023/5/10 2:53, Jaegeuk Kim wrote:
> On 05/08, Daejun Park wrote:
>> v5 -> v6
>> Added trace_f2fs_iostat support for zone reset command.
>>
>> v4 -> v5
>> Added f2fs iostat for zone reset command.
>>
>> v3 -> v4
>> Fixed build error caused by unused function.
>>
>> v2 -> v3
>> Modified arguments to be correct for ftrace parameter.
>> Changed __submit_zone_reset_cmd to void return.
>> Refactored the f2fs_wait_discard_bio function.
>> Fixed code that was previously incorrectly merged.
>>
>> v1 -> v2
>> Changed to apply the optional async reset write pointer by default.
>
> Don't add the history in the patch description.
>
>>
>> This patch enables submit reset zone command asynchornously. It helps
>> decrease average latency of write IOs in high utilization scenario by
>> faster checkpointing.
>>
>> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Except the change log, it looks good to me.
Reviewed-by: Chao Yu <chao@kernel.org>
Thanks,
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH v6] f2fs: add async reset zone command support
@ 2023-05-25 1:20 ` Chao Yu
0 siblings, 0 replies; 18+ messages in thread
From: Chao Yu @ 2023-05-25 1:20 UTC (permalink / raw)
To: Jaegeuk Kim, Daejun Park
Cc: linux-kernel, rostedt, linux-f2fs-devel, mhiramat, Seokhwan Kim,
beomsu kim, linux-trace-kernel
On 2023/5/10 2:53, Jaegeuk Kim wrote:
> On 05/08, Daejun Park wrote:
>> v5 -> v6
>> Added trace_f2fs_iostat support for zone reset command.
>>
>> v4 -> v5
>> Added f2fs iostat for zone reset command.
>>
>> v3 -> v4
>> Fixed build error caused by unused function.
>>
>> v2 -> v3
>> Modified arguments to be correct for ftrace parameter.
>> Changed __submit_zone_reset_cmd to void return.
>> Refactored the f2fs_wait_discard_bio function.
>> Fixed code that was previously incorrectly merged.
>>
>> v1 -> v2
>> Changed to apply the optional async reset write pointer by default.
>
> Don't add the history in the patch description.
>
>>
>> This patch enables submit reset zone command asynchornously. It helps
>> decrease average latency of write IOs in high utilization scenario by
>> faster checkpointing.
>>
>> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Except the change log, it looks good to me.
Reviewed-by: Chao Yu <chao@kernel.org>
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] 18+ messages in thread
* Re: [f2fs-dev] [PATCH v6] f2fs: add async reset zone command support
2023-05-08 8:10 ` [f2fs-dev] " Daejun Park
@ 2023-05-30 23:40 ` patchwork-bot+f2fs
-1 siblings, 0 replies; 18+ messages in thread
From: patchwork-bot+f2fs @ 2023-05-30 23:40 UTC (permalink / raw)
To: Daejun Park
Cc: jaegeuk, chao, rostedt, mhiramat, linux-f2fs-devel, linux-kernel,
linux-trace-kernel, sukka.kim, beomsu7.kim
Hello:
This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:
On Mon, 08 May 2023 17:10:42 +0900 you wrote:
> v5 -> v6
> Added trace_f2fs_iostat support for zone reset command.
>
> v4 -> v5
> Added f2fs iostat for zone reset command.
>
> v3 -> v4
> Fixed build error caused by unused function.
>
> [...]
Here is the summary with links:
- [f2fs-dev,v6] f2fs: add async reset zone command support
https://git.kernel.org/jaegeuk/f2fs/c/7aa6ef440cd7
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH v6] f2fs: add async reset zone command support
@ 2023-05-30 23:40 ` patchwork-bot+f2fs
0 siblings, 0 replies; 18+ messages in thread
From: patchwork-bot+f2fs @ 2023-05-30 23:40 UTC (permalink / raw)
To: Daejun Park
Cc: linux-kernel, rostedt, linux-f2fs-devel, mhiramat, sukka.kim,
jaegeuk, beomsu7.kim, linux-trace-kernel
Hello:
This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:
On Mon, 08 May 2023 17:10:42 +0900 you wrote:
> v5 -> v6
> Added trace_f2fs_iostat support for zone reset command.
>
> v4 -> v5
> Added f2fs iostat for zone reset command.
>
> v3 -> v4
> Fixed build error caused by unused function.
>
> [...]
Here is the summary with links:
- [f2fs-dev,v6] f2fs: add async reset zone command support
https://git.kernel.org/jaegeuk/f2fs/c/7aa6ef440cd7
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
_______________________________________________
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] 18+ messages in thread
end of thread, other threads:[~2023-05-30 23:40 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20230508081042epcms2p8a637deae7de1829f54614e09d5fde5e5@epcms2p8>
2023-05-08 8:10 ` [PATCH v6] f2fs: add async reset zone command support Daejun Park
2023-05-08 8:10 ` [f2fs-dev] " Daejun Park
2023-05-09 18:53 ` Jaegeuk Kim
2023-05-09 18:53 ` [f2fs-dev] " Jaegeuk Kim
2023-05-23 14:43 ` Steven Rostedt
2023-05-23 14:43 ` [f2fs-dev] " Steven Rostedt
2023-05-25 1:20 ` Chao Yu
2023-05-25 1:20 ` [f2fs-dev] " Chao Yu
[not found] ` <CGME20230508081042epcms2p8a637deae7de1829f54614e09d5fde5e5@epcms2p6>
2023-05-11 5:24 ` Daejun Park
2023-05-11 5:24 ` [f2fs-dev] (2) " Daejun Park
2023-05-18 1:05 ` Jaegeuk Kim
2023-05-18 1:05 ` [f2fs-dev] " Jaegeuk Kim
[not found] ` <CGME20230508081042epcms2p8a637deae7de1829f54614e09d5fde5e5@epcms2p7>
2023-05-18 2:08 ` Daejun Park
2023-05-18 2:08 ` [f2fs-dev] (2) " Daejun Park
2023-05-24 1:19 ` Jaegeuk Kim
2023-05-24 1:19 ` [f2fs-dev] " Jaegeuk Kim
2023-05-30 23:40 ` [f2fs-dev] " patchwork-bot+f2fs
2023-05-30 23:40 ` patchwork-bot+f2fs
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.