* [PATCH] f2fs: add async reset zone command support [not found] <CGME20230414025921epcms2p5736ebf6a215201e0c2a2c1a3f73ee06a@epcms2p5> @ 2023-04-14 2:59 ` Daejun Park 2023-04-19 20:28 ` Jaegeuk Kim [not found] ` <CGME20230414025921epcms2p5736ebf6a215201e0c2a2c1a3f73ee06a@epcms2p2> 0 siblings, 2 replies; 3+ messages in thread From: Daejun Park @ 2023-04-14 2:59 UTC (permalink / raw) To: jaegeuk@kernel.org, chao@kernel.org, corbet@lwn.net, rostedt@goodmis.org, mhiramat@kernel.org Cc: linux-f2fs-devel@lists.sourceforge.net, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, Seokhwan Kim, beomsu kim, Yonggil Song, Daejun Park 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> --- Documentation/filesystems/f2fs.rst | 4 ++ fs/f2fs/f2fs.h | 1 + fs/f2fs/segment.c | 92 +++++++++++++++++++++++++++++- fs/f2fs/super.c | 8 +++ include/trace/events/f2fs.h | 18 +++++- 5 files changed, 119 insertions(+), 4 deletions(-) diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst index 2055e72871fe..4cfabf831a79 100644 --- a/Documentation/filesystems/f2fs.rst +++ b/Documentation/filesystems/f2fs.rst @@ -342,6 +342,10 @@ discard_unit=%s Control discard unit, the argument can be "block", "segment" default, it is helpful for large sized SMR or ZNS devices to reduce memory cost by getting rid of fs metadata supports small discard. +async_reset_zone Enable the RESET WRITE POINTER command to be submitted asynchronously. + With this option, the RESET WRITE POINTER command can be processed by + the discard thread like a discard command. It can reduce checkpoint + latency by asynchronously checking for completion of the RESET WRITE POINTER command. memory=%s Control memory mode. This supports "normal" and "low" modes. "low" mode is introduced to support low memory devices. Because of the nature of low memory devices, in this mode, f2fs diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 4e2596dacbf1..021e55c5d1a8 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -167,6 +167,7 @@ struct f2fs_mount_info { * be aligned to this unit: block, * segment or section */ + bool async_zone_reset; /* async zone reset */ struct fscrypt_dummy_policy dummy_enc_policy; /* test dummy encryption */ block_t unusable_cap_perc; /* percentage for cap */ block_t unusable_cap; /* Amount of space allowed to be diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 45128694eefa..60cfe97c9dbd 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -1189,6 +1189,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, @@ -1210,6 +1250,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; @@ -1454,21 +1499,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); @@ -1719,7 +1785,22 @@ 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) { +#ifdef CONFIG_BLK_DEV_ZONED + 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 __punch_discard_cmd(sbi, dc, blkaddr); +#endif } else { dc->ref++; need_wait = true; @@ -1869,6 +1950,13 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi, blkstart, blklen); return -EIO; } + + if (likely(!is_sbi_flag_set(sbi, SBI_POR_DOING)) && + F2FS_OPTION(sbi).async_zone_reset) { + __queue_zone_reset_cmd(sbi, bdev, lblkstart, blklen); + return 0; + } + trace_f2fs_issue_reset_zone(bdev, blkstart); return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET, sector, nr_sects, GFP_NOFS); diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 7d0202f7b317..48198112cbbc 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -162,6 +162,7 @@ enum { Opt_gc_merge, Opt_nogc_merge, Opt_discard_unit, + Opt_async_zone_reset, Opt_memory_mode, Opt_age_extent_cache, Opt_err, @@ -241,6 +242,7 @@ static match_table_t f2fs_tokens = { {Opt_gc_merge, "gc_merge"}, {Opt_nogc_merge, "nogc_merge"}, {Opt_discard_unit, "discard_unit=%s"}, + {Opt_async_zone_reset, "async_zone_reset"}, {Opt_memory_mode, "memory=%s"}, {Opt_age_extent_cache, "age_extent_cache"}, {Opt_err, NULL}, @@ -1249,6 +1251,9 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) } kfree(name); break; + case Opt_async_zone_reset: + F2FS_OPTION(sbi).async_zone_reset = true; + break; case Opt_memory_mode: name = match_strdup(&args[0]); if (!name) @@ -2047,6 +2052,9 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) if (test_opt(sbi, ATGC)) seq_puts(seq, ",atgc"); + if (F2FS_OPTION(sbi).async_zone_reset) + seq_puts(seq, ",async_zone_reset"); + if (F2FS_OPTION(sbi).memory_mode == MEMORY_MODE_NORMAL) seq_printf(seq, ",memory=%s", "normal"); else if (F2FS_OPTION(sbi).memory_mode == MEMORY_MODE_LOW) 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] 3+ messages in thread
* Re: [PATCH] f2fs: add async reset zone command support 2023-04-14 2:59 ` [PATCH] f2fs: add async reset zone command support Daejun Park @ 2023-04-19 20:28 ` Jaegeuk Kim [not found] ` <CGME20230414025921epcms2p5736ebf6a215201e0c2a2c1a3f73ee06a@epcms2p2> 1 sibling, 0 replies; 3+ messages in thread From: Jaegeuk Kim @ 2023-04-19 20:28 UTC (permalink / raw) To: Daejun Park Cc: chao@kernel.org, corbet@lwn.net, rostedt@goodmis.org, mhiramat@kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, Seokhwan Kim, beomsu kim, Yonggil Song On 04/14, Daejun Park wrote: > 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> > --- > Documentation/filesystems/f2fs.rst | 4 ++ > fs/f2fs/f2fs.h | 1 + > fs/f2fs/segment.c | 92 +++++++++++++++++++++++++++++- > fs/f2fs/super.c | 8 +++ > include/trace/events/f2fs.h | 18 +++++- > 5 files changed, 119 insertions(+), 4 deletions(-) > > diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst > index 2055e72871fe..4cfabf831a79 100644 > --- a/Documentation/filesystems/f2fs.rst > +++ b/Documentation/filesystems/f2fs.rst > @@ -342,6 +342,10 @@ discard_unit=%s Control discard unit, the argument can be "block", "segment" > default, it is helpful for large sized SMR or ZNS devices to > reduce memory cost by getting rid of fs metadata supports small > discard. > +async_reset_zone Enable the RESET WRITE POINTER command to be submitted asynchronously. > + With this option, the RESET WRITE POINTER command can be processed by > + the discard thread like a discard command. It can reduce checkpoint > + latency by asynchronously checking for completion of the RESET WRITE POINTER command. Do we need to set this by default instead of mount option? > memory=%s Control memory mode. This supports "normal" and "low" modes. > "low" mode is introduced to support low memory devices. > Because of the nature of low memory devices, in this mode, f2fs > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 4e2596dacbf1..021e55c5d1a8 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -167,6 +167,7 @@ struct f2fs_mount_info { > * be aligned to this unit: block, > * segment or section > */ > + bool async_zone_reset; /* async zone reset */ > struct fscrypt_dummy_policy dummy_enc_policy; /* test dummy encryption */ > block_t unusable_cap_perc; /* percentage for cap */ > block_t unusable_cap; /* Amount of space allowed to be > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 45128694eefa..60cfe97c9dbd 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -1189,6 +1189,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, > @@ -1210,6 +1250,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; > @@ -1454,21 +1499,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); > @@ -1719,7 +1785,22 @@ 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) { > +#ifdef CONFIG_BLK_DEV_ZONED > + 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); Can be consolidated in both cases below. > + } > +#else > __punch_discard_cmd(sbi, dc, blkaddr); > +#endif > } else { > dc->ref++; > need_wait = true; > @@ -1869,6 +1950,13 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi, > blkstart, blklen); > return -EIO; > } > + > + if (likely(!is_sbi_flag_set(sbi, SBI_POR_DOING)) && > + F2FS_OPTION(sbi).async_zone_reset) { > + __queue_zone_reset_cmd(sbi, bdev, lblkstart, blklen); > + return 0; > + } Can make a sigle case above and below, if we set this by default. > + > trace_f2fs_issue_reset_zone(bdev, blkstart); > return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET, > sector, nr_sects, GFP_NOFS); > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 7d0202f7b317..48198112cbbc 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -162,6 +162,7 @@ enum { > Opt_gc_merge, > Opt_nogc_merge, > Opt_discard_unit, > + Opt_async_zone_reset, > Opt_memory_mode, > Opt_age_extent_cache, > Opt_err, > @@ -241,6 +242,7 @@ static match_table_t f2fs_tokens = { > {Opt_gc_merge, "gc_merge"}, > {Opt_nogc_merge, "nogc_merge"}, > {Opt_discard_unit, "discard_unit=%s"}, > + {Opt_async_zone_reset, "async_zone_reset"}, > {Opt_memory_mode, "memory=%s"}, > {Opt_age_extent_cache, "age_extent_cache"}, > {Opt_err, NULL}, > @@ -1249,6 +1251,9 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) > } > kfree(name); > break; > + case Opt_async_zone_reset: > + F2FS_OPTION(sbi).async_zone_reset = true; > + break; > case Opt_memory_mode: > name = match_strdup(&args[0]); > if (!name) > @@ -2047,6 +2052,9 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) > if (test_opt(sbi, ATGC)) > seq_puts(seq, ",atgc"); > > + if (F2FS_OPTION(sbi).async_zone_reset) > + seq_puts(seq, ",async_zone_reset"); > + > if (F2FS_OPTION(sbi).memory_mode == MEMORY_MODE_NORMAL) > seq_printf(seq, ",memory=%s", "normal"); > else if (F2FS_OPTION(sbi).memory_mode == MEMORY_MODE_LOW) > 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 [flat|nested] 3+ messages in thread
[parent not found: <CGME20230414025921epcms2p5736ebf6a215201e0c2a2c1a3f73ee06a@epcms2p2>]
* RE:(2) [PATCH] f2fs: add async reset zone command support [not found] ` <CGME20230414025921epcms2p5736ebf6a215201e0c2a2c1a3f73ee06a@epcms2p2> @ 2023-04-20 7:52 ` Daejun Park 0 siblings, 0 replies; 3+ messages in thread From: Daejun Park @ 2023-04-20 7:52 UTC (permalink / raw) To: Jaegeuk Kim, Daejun Park Cc: chao@kernel.org, corbet@lwn.net, rostedt@goodmis.org, mhiramat@kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, Seokhwan Kim, beomsu kim, Yonggil Song Hi Jeageuk, Thanks for helpful review. > > On 04/14, Daejun Park wrote: > > 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> > > --- > > Documentation/filesystems/f2fs.rst | 4 ++ > > fs/f2fs/f2fs.h | 1 + > > fs/f2fs/segment.c | 92 +++++++++++++++++++++++++++++- > > fs/f2fs/super.c | 8 +++ > > include/trace/events/f2fs.h | 18 +++++- > > 5 files changed, 119 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst > > index 2055e72871fe..4cfabf831a79 100644 > > --- a/Documentation/filesystems/f2fs.rst > > +++ b/Documentation/filesystems/f2fs.rst > > @@ -342,6 +342,10 @@ discard_unit=%s Control discard unit, the argument can be "block", "segment" > > default, it is helpful for large sized SMR or ZNS devices to > > reduce memory cost by getting rid of fs metadata supports small > > discard. > > +async_reset_zone Enable the RESET WRITE POINTER command to be submitted asynchronously. > > + With this option, the RESET WRITE POINTER command can be processed by > > + the discard thread like a discard command. It can reduce checkpoint > > + latency by asynchronously checking for completion of the RESET WRITE POINTER command. > > Do we need to set this by default instead of mount option? I think it is good idea. I will make this feature by default. > > > memory=%s Control memory mode. This supports "normal" and "low" modes. > > "low" mode is introduced to support low memory devices. > > Because of the nature of low memory devices, in this mode, f2fs > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > index 4e2596dacbf1..021e55c5d1a8 100644 > > --- a/fs/f2fs/f2fs.h > > +++ b/fs/f2fs/f2fs.h > > @@ -167,6 +167,7 @@ struct f2fs_mount_info { > > * be aligned to this unit: block, > > * segment or section > > */ > > + bool async_zone_reset; /* async zone reset */ > > struct fscrypt_dummy_policy dummy_enc_policy; /* test dummy encryption */ > > block_t unusable_cap_perc; /* percentage for cap */ > > block_t unusable_cap; /* Amount of space allowed to be > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > > index 45128694eefa..60cfe97c9dbd 100644 > > --- a/fs/f2fs/segment.c > > +++ b/fs/f2fs/segment.c > > @@ -1189,6 +1189,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, > > @@ -1210,6 +1250,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; > > @@ -1454,21 +1499,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); > > @@ -1719,7 +1785,22 @@ 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) { > > +#ifdef CONFIG_BLK_DEV_ZONED > > + 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); > > Can be consolidated in both cases below. How do you think this modification as below. if (dc) { if (dc->state == D_PREP) { 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; } } > > > + } > > +#else > > __punch_discard_cmd(sbi, dc, blkaddr); > > +#endif > > } else { > > dc->ref++; > > need_wait = true; > > @@ -1869,6 +1950,13 @@ static int __f2fs_issue_discard_zone(struct f2fs_sb_info *sbi, > > blkstart, blklen); > > return -EIO; > > } > > + > > + if (likely(!is_sbi_flag_set(sbi, SBI_POR_DOING)) && > > + F2FS_OPTION(sbi).async_zone_reset) { > > + __queue_zone_reset_cmd(sbi, bdev, lblkstart, blklen); > > + return 0; > > + } > > Can make a sigle case above and below, if we set this by default. I can remove "F2FS_OPTION(sbi).async_zone_reset", but I think the checking "SBI_POR_DOING" statement can not be removed. Because check_zone_write_pointer() and fix_curseg_write_pointer() use this __f2fs_issue_discard_zone() for sending reset write pointer command synchronously. > > + > > trace_f2fs_issue_reset_zone(bdev, blkstart); > > return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET, > > sector, nr_sects, GFP_NOFS); > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > > index 7d0202f7b317..48198112cbbc 100644 > > --- a/fs/f2fs/super.c > > +++ b/fs/f2fs/super.c > > @@ -162,6 +162,7 @@ enum { > > Opt_gc_merge, > > Opt_nogc_merge, > > Opt_discard_unit, > > + Opt_async_zone_reset, > > Opt_memory_mode, > > Opt_age_extent_cache, > > Opt_err, > > @@ -241,6 +242,7 @@ static match_table_t f2fs_tokens = { > > {Opt_gc_merge, "gc_merge"}, > > {Opt_nogc_merge, "nogc_merge"}, > > {Opt_discard_unit, "discard_unit=%s"}, > > + {Opt_async_zone_reset, "async_zone_reset"}, > > {Opt_memory_mode, "memory=%s"}, > > {Opt_age_extent_cache, "age_extent_cache"}, > > {Opt_err, NULL}, > > @@ -1249,6 +1251,9 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) > > } > > kfree(name); > > break; > > + case Opt_async_zone_reset: > > + F2FS_OPTION(sbi).async_zone_reset = true; > > + break; > > case Opt_memory_mode: > > name = match_strdup(&args[0]); > > if (!name) > > @@ -2047,6 +2052,9 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) > > if (test_opt(sbi, ATGC)) > > seq_puts(seq, ",atgc"); > > > > + if (F2FS_OPTION(sbi).async_zone_reset) > > + seq_puts(seq, ",async_zone_reset"); > > + > > if (F2FS_OPTION(sbi).memory_mode == MEMORY_MODE_NORMAL) > > seq_printf(seq, ",memory=%s", "normal"); > > else if (F2FS_OPTION(sbi).memory_mode == MEMORY_MODE_LOW) > > 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 > Thanks, Daejun ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-04-20 7:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20230414025921epcms2p5736ebf6a215201e0c2a2c1a3f73ee06a@epcms2p5>
2023-04-14 2:59 ` [PATCH] f2fs: add async reset zone command support Daejun Park
2023-04-19 20:28 ` Jaegeuk Kim
[not found] ` <CGME20230414025921epcms2p5736ebf6a215201e0c2a2c1a3f73ee06a@epcms2p2>
2023-04-20 7:52 ` Daejun Park
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).