* [PATCH v2] f2fs: add async reset zone command support
[not found] <CGME20230425081018epcms2p8fefa94810f0b341d2cbd70ba587e9e8c@epcms2p8>
@ 2023-04-25 8:10 ` Daejun Park
0 siblings, 0 replies; 6+ messages in thread
From: Daejun Park @ 2023-04-25 8:10 UTC (permalink / raw
To: jaegeuk@kernel.org, chao@kernel.org, rostedt@goodmis.org,
mhiramat@kernel.org, linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Cc: Daejun Park, Seokhwan Kim, Yonggil Song, beomsu kim
Changelog:
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/segment.c | 95 ++++++++++++++++++++++++++++++++++---
include/trace/events/f2fs.h | 18 ++++++-
2 files changed, 105 insertions(+), 8 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 82430f80c5da..27074a6eaf20 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1195,6 +1195,46 @@ 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 int __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, SECTOR_FROM_BLOCK(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);
+
+ return 0;
+}
+#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,
@@ -1216,6 +1256,11 @@ 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))
+ return __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
+#endif
+
trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
lstart = dc->di.lstart;
@@ -1460,21 +1505,42 @@ 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 blklen)
+{
+ block_t lblkstart = blkstart;
+
+ if (f2fs_is_multi_device(sbi)) {
+ int devi = f2fs_target_device_index(sbi, blkstart);
+
+ blkstart -= FDEV(devi).start_blk;
+ }
+
+ 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)
{
block_t lblkstart = blkstart;
+ trace_f2fs_queue_discard(bdev, blkstart, blklen);
+
if (!f2fs_bdev_support_discard(bdev))
return;
- trace_f2fs_queue_discard(bdev, blkstart, blklen);
-
if (f2fs_is_multi_device(sbi)) {
int devi = f2fs_target_device_index(sbi, blkstart);
blkstart -= FDEV(devi).start_blk;
}
+
mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
__update_discard_tree_range(sbi, bdev, lblkstart, blkstart, blklen);
mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
@@ -1725,7 +1791,18 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
dc = __lookup_discard_cmd(sbi, blkaddr);
if (dc) {
if (dc->state == D_PREP) {
- __punch_discard_cmd(sbi, dc, blkaddr);
+ if (f2fs_sb_has_blkzoned(sbi) &&
+ bdev_is_zoned(dc->bdev)) {
+ struct discard_policy dpolicy;
+ /* force submit zone reset */
+ __init_discard_policy(sbi, &dpolicy,
+ DPOLICY_FORCE, 1);
+ __submit_discard_cmd(sbi, &dpolicy, dc, NULL);
+ dc->ref++;
+ need_wait = true;
+ } else {
+ __punch_discard_cmd(sbi, dc, blkaddr);
+ }
} else {
dc->ref++;
need_wait = true;
@@ -1875,9 +1952,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, 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..ee1477de8324 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,
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [f2fs-dev] [PATCH v2] f2fs: add async reset zone command support
@ 2023-04-25 8:10 ` Daejun Park
0 siblings, 0 replies; 6+ messages in thread
From: Daejun Park @ 2023-04-25 8:10 UTC (permalink / raw
To: jaegeuk@kernel.org, chao@kernel.org, rostedt@goodmis.org,
mhiramat@kernel.org, linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Cc: Seokhwan Kim, beomsu kim
Changelog:
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/segment.c | 95 ++++++++++++++++++++++++++++++++++---
include/trace/events/f2fs.h | 18 ++++++-
2 files changed, 105 insertions(+), 8 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 82430f80c5da..27074a6eaf20 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1195,6 +1195,46 @@ 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 int __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, SECTOR_FROM_BLOCK(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);
+
+ return 0;
+}
+#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,
@@ -1216,6 +1256,11 @@ 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))
+ return __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
+#endif
+
trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
lstart = dc->di.lstart;
@@ -1460,21 +1505,42 @@ 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 blklen)
+{
+ block_t lblkstart = blkstart;
+
+ if (f2fs_is_multi_device(sbi)) {
+ int devi = f2fs_target_device_index(sbi, blkstart);
+
+ blkstart -= FDEV(devi).start_blk;
+ }
+
+ 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)
{
block_t lblkstart = blkstart;
+ trace_f2fs_queue_discard(bdev, blkstart, blklen);
+
if (!f2fs_bdev_support_discard(bdev))
return;
- trace_f2fs_queue_discard(bdev, blkstart, blklen);
-
if (f2fs_is_multi_device(sbi)) {
int devi = f2fs_target_device_index(sbi, blkstart);
blkstart -= FDEV(devi).start_blk;
}
+
mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
__update_discard_tree_range(sbi, bdev, lblkstart, blkstart, blklen);
mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
@@ -1725,7 +1791,18 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
dc = __lookup_discard_cmd(sbi, blkaddr);
if (dc) {
if (dc->state == D_PREP) {
- __punch_discard_cmd(sbi, dc, blkaddr);
+ if (f2fs_sb_has_blkzoned(sbi) &&
+ bdev_is_zoned(dc->bdev)) {
+ struct discard_policy dpolicy;
+ /* force submit zone reset */
+ __init_discard_policy(sbi, &dpolicy,
+ DPOLICY_FORCE, 1);
+ __submit_discard_cmd(sbi, &dpolicy, dc, NULL);
+ dc->ref++;
+ need_wait = true;
+ } else {
+ __punch_discard_cmd(sbi, dc, blkaddr);
+ }
} else {
dc->ref++;
need_wait = true;
@@ -1875,9 +1952,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, 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..ee1477de8324 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,
--
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] 6+ messages in thread
* Re: [PATCH v2] f2fs: add async reset zone command support
2023-04-25 8:10 ` [f2fs-dev] " Daejun Park
@ 2023-04-25 15:04 ` Chao Yu
-1 siblings, 0 replies; 6+ messages in thread
From: Chao Yu @ 2023-04-25 15:04 UTC (permalink / raw
To: daejun7.park, jaegeuk@kernel.org, rostedt@goodmis.org,
mhiramat@kernel.org, linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Cc: Seokhwan Kim, Yonggil Song, beomsu kim
On 2023/4/25 16:10, Daejun Park wrote:
> Changelog:
>
> 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/segment.c | 95 ++++++++++++++++++++++++++++++++++---
> include/trace/events/f2fs.h | 18 ++++++-
> 2 files changed, 105 insertions(+), 8 deletions(-)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 82430f80c5da..27074a6eaf20 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1195,6 +1195,46 @@ 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 int __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, SECTOR_FROM_BLOCK(dc->di.start));
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);
> +
> + return 0;
Unneeded return value?
> +}
> +#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,
> @@ -1216,6 +1256,11 @@ 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))
> + return __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> +#endif
> +
> trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
>
> lstart = dc->di.lstart;
> @@ -1460,21 +1505,42 @@ 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 blklen)
block_t lblkstart,
> +{
> + block_t lblkstart = blkstart;
block_t blkstart = lblkstart;
> +
> + if (f2fs_is_multi_device(sbi)) {
> + int devi = f2fs_target_device_index(sbi, blkstart);
> +
> + blkstart -= FDEV(devi).start_blk;
> + }
> +
> + 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)
> {
> block_t lblkstart = blkstart;
>
> + trace_f2fs_queue_discard(bdev, blkstart, blklen);
If device doesn't support discard, it's better not show above tracepoint?
> +
> if (!f2fs_bdev_support_discard(bdev))
> return;
>
> - trace_f2fs_queue_discard(bdev, blkstart, blklen);
> -
> if (f2fs_is_multi_device(sbi)) {
> int devi = f2fs_target_device_index(sbi, blkstart);
>
> blkstart -= FDEV(devi).start_blk;
> }
> +
> mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> __update_discard_tree_range(sbi, bdev, lblkstart, blkstart, blklen);
> mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> @@ -1725,7 +1791,18 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
> dc = __lookup_discard_cmd(sbi, blkaddr);
> if (dc) {
> if (dc->state == D_PREP) {
> - __punch_discard_cmd(sbi, dc, blkaddr);
> + if (f2fs_sb_has_blkzoned(sbi) &&
> + bdev_is_zoned(dc->bdev)) {
> + struct discard_policy dpolicy;
> + /* force submit zone reset */
> + __init_discard_policy(sbi, &dpolicy,
> + DPOLICY_FORCE, 1);
> + __submit_discard_cmd(sbi, &dpolicy, dc, NULL);
Can we call __submit_zone_reset_cmd() directly?
Thanks,
> + dc->ref++;
> + need_wait = true;
> + } else {
> + __punch_discard_cmd(sbi, dc, blkaddr);
> + }
> } else {
> dc->ref++;
> need_wait = true;
> @@ -1875,9 +1952,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, 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..ee1477de8324 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,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: add async reset zone command support
@ 2023-04-25 15:04 ` Chao Yu
0 siblings, 0 replies; 6+ messages in thread
From: Chao Yu @ 2023-04-25 15:04 UTC (permalink / raw
To: daejun7.park, jaegeuk@kernel.org, rostedt@goodmis.org,
mhiramat@kernel.org, linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Cc: Seokhwan Kim, beomsu kim
On 2023/4/25 16:10, Daejun Park wrote:
> Changelog:
>
> 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/segment.c | 95 ++++++++++++++++++++++++++++++++++---
> include/trace/events/f2fs.h | 18 ++++++-
> 2 files changed, 105 insertions(+), 8 deletions(-)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 82430f80c5da..27074a6eaf20 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1195,6 +1195,46 @@ 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 int __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, SECTOR_FROM_BLOCK(dc->di.start));
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);
> +
> + return 0;
Unneeded return value?
> +}
> +#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,
> @@ -1216,6 +1256,11 @@ 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))
> + return __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> +#endif
> +
> trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
>
> lstart = dc->di.lstart;
> @@ -1460,21 +1505,42 @@ 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 blklen)
block_t lblkstart,
> +{
> + block_t lblkstart = blkstart;
block_t blkstart = lblkstart;
> +
> + if (f2fs_is_multi_device(sbi)) {
> + int devi = f2fs_target_device_index(sbi, blkstart);
> +
> + blkstart -= FDEV(devi).start_blk;
> + }
> +
> + 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)
> {
> block_t lblkstart = blkstart;
>
> + trace_f2fs_queue_discard(bdev, blkstart, blklen);
If device doesn't support discard, it's better not show above tracepoint?
> +
> if (!f2fs_bdev_support_discard(bdev))
> return;
>
> - trace_f2fs_queue_discard(bdev, blkstart, blklen);
> -
> if (f2fs_is_multi_device(sbi)) {
> int devi = f2fs_target_device_index(sbi, blkstart);
>
> blkstart -= FDEV(devi).start_blk;
> }
> +
> mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> __update_discard_tree_range(sbi, bdev, lblkstart, blkstart, blklen);
> mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> @@ -1725,7 +1791,18 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
> dc = __lookup_discard_cmd(sbi, blkaddr);
> if (dc) {
> if (dc->state == D_PREP) {
> - __punch_discard_cmd(sbi, dc, blkaddr);
> + if (f2fs_sb_has_blkzoned(sbi) &&
> + bdev_is_zoned(dc->bdev)) {
> + struct discard_policy dpolicy;
> + /* force submit zone reset */
> + __init_discard_policy(sbi, &dpolicy,
> + DPOLICY_FORCE, 1);
> + __submit_discard_cmd(sbi, &dpolicy, dc, NULL);
Can we call __submit_zone_reset_cmd() directly?
Thanks,
> + dc->ref++;
> + need_wait = true;
> + } else {
> + __punch_discard_cmd(sbi, dc, blkaddr);
> + }
> } else {
> dc->ref++;
> need_wait = true;
> @@ -1875,9 +1952,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, 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..ee1477de8324 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,
_______________________________________________
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] 6+ messages in thread
* Re: [f2fs-dev] (2) [PATCH v2] f2fs: add async reset zone command support
[not found] ` <CGME20230425081018epcms2p8fefa94810f0b341d2cbd70ba587e9e8c@epcms2p7>
@ 2023-04-26 7:01 ` Daejun Park
0 siblings, 0 replies; 6+ messages in thread
From: Daejun Park @ 2023-04-26 7:01 UTC (permalink / raw
To: Chao Yu, Daejun Park, jaegeuk@kernel.org, rostedt@goodmis.org,
mhiramat@kernel.org, linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Cc: Seokhwan Kim, beomsu kim
Thank you for your review. I have responded in the comments below.
> On 2023/4/25 16:10, Daejun Park wrote:
> > Changelog:
> >
> > 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/segment.c | 95 ++++++++++++++++++++++++++++++++++---
> > include/trace/events/f2fs.h | 18 ++++++-
> > 2 files changed, 105 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 82430f80c5da..27074a6eaf20 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -1195,6 +1195,46 @@ 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 int __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, SECTOR_FROM_BLOCK(dc->di.start));
>
> trace_f2fs_issue_reset_zone(bdev, dc->di.start); ?
I will change it.
>
> > +
> > + 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);
> > +
> > + return 0;
>
> Unneeded return value?
I will change return type of this function as void.
> > +}
> > +#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,
> > @@ -1216,6 +1256,11 @@ 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))
> > + return __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> > +#endif
> > +
> > trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
> >
> > lstart = dc->di.lstart;
> > @@ -1460,21 +1505,42 @@ 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 blklen)
>
> block_t lblkstart,
>
> > +{
> > + block_t lblkstart = blkstart;
>
> block_t blkstart = lblkstart;
I refactored this function as follows.
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);
}
>
> > +
> > + if (f2fs_is_multi_device(sbi)) {
> > + int devi = f2fs_target_device_index(sbi, blkstart);
> > +
> > + blkstart -= FDEV(devi).start_blk;
> > + }
> > +
> > + 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)
> > {
> > block_t lblkstart = blkstart;
> >
> > + trace_f2fs_queue_discard(bdev, blkstart, blklen);
>
> If device doesn't support discard, it's better not show above tracepoint?
This issue was caused by a mistake during the merge process. I have fixed it.
> > +
> > if (!f2fs_bdev_support_discard(bdev))
> > return;
> >
> > - trace_f2fs_queue_discard(bdev, blkstart, blklen);
> > -
> > if (f2fs_is_multi_device(sbi)) {
> > int devi = f2fs_target_device_index(sbi, blkstart);
> >
> > blkstart -= FDEV(devi).start_blk;
> > }
> > +
> > mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> > __update_discard_tree_range(sbi, bdev, lblkstart, blkstart, blklen);
> > mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> > @@ -1725,7 +1791,18 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
> > dc = __lookup_discard_cmd(sbi, blkaddr);
> > if (dc) {
> > if (dc->state == D_PREP) {
> > - __punch_discard_cmd(sbi, dc, blkaddr);
> > + if (f2fs_sb_has_blkzoned(sbi) &&
> > + bdev_is_zoned(dc->bdev)) {
> > + struct discard_policy dpolicy;
> > + /* force submit zone reset */
> > + __init_discard_policy(sbi, &dpolicy,
> > + DPOLICY_FORCE, 1);
> > + __submit_discard_cmd(sbi, &dpolicy, dc, NULL);
>
> Can we call __submit_zone_reset_cmd() directly?
Sure. I refactored as follows.
if (dc) {
if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
if (dc->state == D_PREP)
__submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
&dcc->wait_list, NULL);
dc->ref++;
need_wait = true;
} else {
if (dc->state == D_PREP) {
__punch_discard_cmd(sbi, dc, blkaddr);
} else {
dc->ref++;
need_wait = true;
}
}
}
Thanks,
>
> Thanks,
>
> > + dc->ref++;
> > + need_wait = true;
> > + } else {
> > + __punch_discard_cmd(sbi, dc, blkaddr);
> > + }
> > } else {
> > dc->ref++;
> > need_wait = true;
> > @@ -1875,9 +1952,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, 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..ee1477de8324 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,
_______________________________________________
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] 6+ messages in thread
* RE:(2) [PATCH v2] f2fs: add async reset zone command support
@ 2023-04-26 7:01 ` Daejun Park
0 siblings, 0 replies; 6+ messages in thread
From: Daejun Park @ 2023-04-26 7:01 UTC (permalink / raw
To: Chao Yu, Daejun Park, jaegeuk@kernel.org, rostedt@goodmis.org,
mhiramat@kernel.org, linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Cc: Seokhwan Kim, Yonggil Song, beomsu kim
Thank you for your review. I have responded in the comments below.
> On 2023/4/25 16:10, Daejun Park wrote:
> > Changelog:
> >
> > 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/segment.c | 95 ++++++++++++++++++++++++++++++++++---
> > include/trace/events/f2fs.h | 18 ++++++-
> > 2 files changed, 105 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 82430f80c5da..27074a6eaf20 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -1195,6 +1195,46 @@ 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 int __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, SECTOR_FROM_BLOCK(dc->di.start));
>
> trace_f2fs_issue_reset_zone(bdev, dc->di.start); ?
I will change it.
>
> > +
> > + 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);
> > +
> > + return 0;
>
> Unneeded return value?
I will change return type of this function as void.
> > +}
> > +#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,
> > @@ -1216,6 +1256,11 @@ 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))
> > + return __submit_zone_reset_cmd(sbi, dc, flag, wait_list, issued);
> > +#endif
> > +
> > trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
> >
> > lstart = dc->di.lstart;
> > @@ -1460,21 +1505,42 @@ 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 blklen)
>
> block_t lblkstart,
>
> > +{
> > + block_t lblkstart = blkstart;
>
> block_t blkstart = lblkstart;
I refactored this function as follows.
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);
}
>
> > +
> > + if (f2fs_is_multi_device(sbi)) {
> > + int devi = f2fs_target_device_index(sbi, blkstart);
> > +
> > + blkstart -= FDEV(devi).start_blk;
> > + }
> > +
> > + 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)
> > {
> > block_t lblkstart = blkstart;
> >
> > + trace_f2fs_queue_discard(bdev, blkstart, blklen);
>
> If device doesn't support discard, it's better not show above tracepoint?
This issue was caused by a mistake during the merge process. I have fixed it.
> > +
> > if (!f2fs_bdev_support_discard(bdev))
> > return;
> >
> > - trace_f2fs_queue_discard(bdev, blkstart, blklen);
> > -
> > if (f2fs_is_multi_device(sbi)) {
> > int devi = f2fs_target_device_index(sbi, blkstart);
> >
> > blkstart -= FDEV(devi).start_blk;
> > }
> > +
> > mutex_lock(&SM_I(sbi)->dcc_info->cmd_lock);
> > __update_discard_tree_range(sbi, bdev, lblkstart, blkstart, blklen);
> > mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> > @@ -1725,7 +1791,18 @@ static void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
> > dc = __lookup_discard_cmd(sbi, blkaddr);
> > if (dc) {
> > if (dc->state == D_PREP) {
> > - __punch_discard_cmd(sbi, dc, blkaddr);
> > + if (f2fs_sb_has_blkzoned(sbi) &&
> > + bdev_is_zoned(dc->bdev)) {
> > + struct discard_policy dpolicy;
> > + /* force submit zone reset */
> > + __init_discard_policy(sbi, &dpolicy,
> > + DPOLICY_FORCE, 1);
> > + __submit_discard_cmd(sbi, &dpolicy, dc, NULL);
>
> Can we call __submit_zone_reset_cmd() directly?
Sure. I refactored as follows.
if (dc) {
if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(dc->bdev)) {
if (dc->state == D_PREP)
__submit_zone_reset_cmd(sbi, dc, REQ_SYNC,
&dcc->wait_list, NULL);
dc->ref++;
need_wait = true;
} else {
if (dc->state == D_PREP) {
__punch_discard_cmd(sbi, dc, blkaddr);
} else {
dc->ref++;
need_wait = true;
}
}
}
Thanks,
>
> Thanks,
>
> > + dc->ref++;
> > + need_wait = true;
> > + } else {
> > + __punch_discard_cmd(sbi, dc, blkaddr);
> > + }
> > } else {
> > dc->ref++;
> > need_wait = true;
> > @@ -1875,9 +1952,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, 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..ee1477de8324 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,
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-04-26 7:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20230425081018epcms2p8fefa94810f0b341d2cbd70ba587e9e8c@epcms2p8>
2023-04-25 8:10 ` [PATCH v2] f2fs: add async reset zone command support Daejun Park
2023-04-25 8:10 ` [f2fs-dev] " Daejun Park
2023-04-25 15:04 ` Chao Yu
2023-04-25 15:04 ` [f2fs-dev] " Chao Yu
[not found] ` <CGME20230425081018epcms2p8fefa94810f0b341d2cbd70ba587e9e8c@epcms2p7>
2023-04-26 7:01 ` [f2fs-dev] (2) " Daejun Park
2023-04-26 7:01 ` Daejun Park
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.