Messages in this thread Patch in this message | ![/](/images/icornerl.gif) | | Date | Sun, 7 Apr 2024 11:37:38 +0800 | Subject | Re: [PATCH v2] f2fs: zone: don't block IO if there is remained open zone | From | Chao Yu <> |
| |
On 2024/4/2 3:08, Daeho Jeong wrote: > On Thu, Mar 28, 2024 at 8:20 PM Chao Yu <chao@kernel.org> wrote: >> >> max open zone may be larger than log header number of f2fs, for >> such case, it doesn't need to wait last IO in previous zone, let's >> introduce available_open_zone semaphore, and reduce it once we >> submit first write IO in a zone, and increase it after completion >> of last IO in the zone. >> >> Cc: Daeho Jeong <daeho43@gmail.com> >> Signed-off-by: Chao Yu <chao@kernel.org> >> --- >> v2: >> - remove unneeded declaration. >> fs/f2fs/data.c | 80 +++++++++++++++++++++++++---------------------- >> fs/f2fs/f2fs.h | 33 ++++++++++++++++--- >> fs/f2fs/iostat.c | 7 +++++ >> fs/f2fs/iostat.h | 2 ++ >> fs/f2fs/segment.c | 58 ++++++++++++++++++++++++++++++---- >> fs/f2fs/segment.h | 12 ++++++- >> fs/f2fs/super.c | 2 ++ >> 7 files changed, 145 insertions(+), 49 deletions(-) >> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >> index 0d88649c60a5..132a3ede60b1 100644 >> --- a/fs/f2fs/data.c >> +++ b/fs/f2fs/data.c >> @@ -373,11 +373,10 @@ static void f2fs_write_end_io(struct bio *bio) >> #ifdef CONFIG_BLK_DEV_ZONED >> static void f2fs_zone_write_end_io(struct bio *bio) >> { >> - struct f2fs_bio_info *io = (struct f2fs_bio_info *)bio->bi_private; >> + struct f2fs_sb_info *sbi = iostat_get_bio_private(bio); >> >> - bio->bi_private = io->bi_private; >> - complete(&io->zone_wait); >> f2fs_write_end_io(bio); >> + up(&sbi->available_open_zones); >> } >> #endif >> >> @@ -531,6 +530,24 @@ static void __submit_merged_bio(struct f2fs_bio_info *io) >> if (!io->bio) >> return; >> >> +#ifdef CONFIG_BLK_DEV_ZONED >> + if (io->open_zone) { >> + /* >> + * if there is no open zone, it will wait for last IO in >> + * previous zone before submitting new IO. >> + */ >> + down(&fio->sbi->available_open_zones); >> + io->open_zone = false; >> + io->zone_openned = true; >> + } >> + >> + if (io->close_zone) { >> + io->bio->bi_end_io = f2fs_zone_write_end_io; >> + io->zone_openned = false; > > I am not sure this part isn't racy with the part in __allocate_new_segment().
I guess so, it seems we should append below diff for this patch.
--- fs/f2fs/segment.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 20534f512fb2..c1fdbb8362e6 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -3150,8 +3150,13 @@ allocate:
f2fs_down_write(&io->io_rwsem); if (io->zone_openned) { - up(&sbi->available_open_zones); - io->zone_openned = false; + if (io->bio) { + io->close_zone = true; + __submit_merged_bio(io); + } else if (io->zone_openned) { + up(&sbi->available_open_zones); + io->zone_openned = false; + } } f2fs_up_write(&io->io_rwsem); } -- Thanks,
> > Thanks, > >> + io->close_zone = false; >> + } >> +#endif >> + >> if (is_read_io(fio->op)) { >> trace_f2fs_prepare_read_bio(io->sbi->sb, fio->type, io->bio); >> f2fs_submit_read_bio(io->sbi, io->bio, fio->type); >> @@ -601,9 +618,9 @@ int f2fs_init_write_merge_io(struct f2fs_sb_info *sbi) >> INIT_LIST_HEAD(&sbi->write_io[i][j].bio_list); >> init_f2fs_rwsem(&sbi->write_io[i][j].bio_list_lock); >> #ifdef CONFIG_BLK_DEV_ZONED >> - init_completion(&sbi->write_io[i][j].zone_wait); >> - sbi->write_io[i][j].zone_pending_bio = NULL; >> - sbi->write_io[i][j].bi_private = NULL; >> + sbi->write_io[i][j].open_zone = false; >> + sbi->write_io[i][j].zone_openned = false; >> + sbi->write_io[i][j].close_zone = false; >> #endif >> } >> } >> @@ -918,22 +935,16 @@ int f2fs_merge_page_bio(struct f2fs_io_info *fio) >> } >> >> #ifdef CONFIG_BLK_DEV_ZONED >> -static bool is_end_zone_blkaddr(struct f2fs_sb_info *sbi, block_t blkaddr) >> +static bool is_blkaddr_zone_boundary(struct f2fs_sb_info *sbi, >> + block_t blkaddr, bool start) >> { >> - int devi = 0; >> + if (!f2fs_blkaddr_in_seqzone(sbi, blkaddr)) >> + return false; >> + >> + if (start) >> + return (blkaddr % sbi->blocks_per_blkz) == 0; >> + return (blkaddr % sbi->blocks_per_blkz == sbi->blocks_per_blkz - 1); >> >> - if (f2fs_is_multi_device(sbi)) { >> - devi = f2fs_target_device_index(sbi, blkaddr); >> - if (blkaddr < FDEV(devi).start_blk || >> - blkaddr > FDEV(devi).end_blk) { >> - f2fs_err(sbi, "Invalid block %x", blkaddr); >> - return false; >> - } >> - blkaddr -= FDEV(devi).start_blk; >> - } >> - return bdev_is_zoned(FDEV(devi).bdev) && >> - f2fs_blkz_is_seq(sbi, devi, blkaddr) && >> - (blkaddr % sbi->blocks_per_blkz == sbi->blocks_per_blkz - 1); >> } >> #endif >> >> @@ -944,20 +955,14 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio) >> struct f2fs_bio_info *io = sbi->write_io[btype] + fio->temp; >> struct page *bio_page; >> enum count_type type; >> +#ifdef CONFIG_BLK_DEV_ZONED >> + bool blkzoned = f2fs_sb_has_blkzoned(sbi) && btype < META; >> +#endif >> >> f2fs_bug_on(sbi, is_read_io(fio->op)); >> >> f2fs_down_write(&io->io_rwsem); >> next: >> -#ifdef CONFIG_BLK_DEV_ZONED >> - if (f2fs_sb_has_blkzoned(sbi) && btype < META && io->zone_pending_bio) { >> - wait_for_completion_io(&io->zone_wait); >> - bio_put(io->zone_pending_bio); >> - io->zone_pending_bio = NULL; >> - io->bi_private = NULL; >> - } >> -#endif >> - >> if (fio->in_list) { >> spin_lock(&io->io_lock); >> if (list_empty(&io->io_list)) { >> @@ -985,6 +990,11 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio) >> type = WB_DATA_TYPE(bio_page, fio->compressed_page); >> inc_page_count(sbi, type); >> >> +#ifdef CONFIG_BLK_DEV_ZONED >> + if (blkzoned && is_blkaddr_zone_boundary(sbi, fio->new_blkaddr, true)) >> + io->open_zone = true; >> +#endif >> + >> if (io->bio && >> (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio, >> fio->new_blkaddr) || >> @@ -1010,15 +1020,11 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio) >> io->last_block_in_bio = fio->new_blkaddr; >> >> trace_f2fs_submit_page_write(fio->page, fio); >> + >> #ifdef CONFIG_BLK_DEV_ZONED >> - if (f2fs_sb_has_blkzoned(sbi) && btype < META && >> - is_end_zone_blkaddr(sbi, fio->new_blkaddr)) { >> - bio_get(io->bio); >> - reinit_completion(&io->zone_wait); >> - io->bi_private = io->bio->bi_private; >> - io->bio->bi_private = io; >> - io->bio->bi_end_io = f2fs_zone_write_end_io; >> - io->zone_pending_bio = io->bio; >> + if (blkzoned && >> + is_blkaddr_zone_boundary(sbi, fio->new_blkaddr, false)) { >> + io->close_zone = true; >> __submit_merged_bio(io); >> } >> #endif >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index 694f8a52cb84..41a07bedf0a1 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -1234,16 +1234,16 @@ struct f2fs_bio_info { >> struct bio *bio; /* bios to merge */ >> sector_t last_block_in_bio; /* last block number */ >> struct f2fs_io_info fio; /* store buffered io info. */ >> -#ifdef CONFIG_BLK_DEV_ZONED >> - struct completion zone_wait; /* condition value for the previous open zone to close */ >> - struct bio *zone_pending_bio; /* pending bio for the previous zone */ >> - void *bi_private; /* previous bi_private for pending bio */ >> -#endif >> struct f2fs_rwsem io_rwsem; /* blocking op for bio */ >> spinlock_t io_lock; /* serialize DATA/NODE IOs */ >> struct list_head io_list; /* track fios */ >> struct list_head bio_list; /* bio entry list head */ >> struct f2fs_rwsem bio_list_lock; /* lock to protect bio entry list */ >> +#ifdef CONFIG_BLK_DEV_ZONED >> + bool open_zone; /* open a zone */ >> + bool zone_openned; /* zone has been openned */ >> + bool close_zone; /* close a zone */ >> +#endif >> }; >> >> #define FDEV(i) (sbi->devs[i]) >> @@ -1560,6 +1560,7 @@ struct f2fs_sb_info { >> #ifdef CONFIG_BLK_DEV_ZONED >> unsigned int blocks_per_blkz; /* F2FS blocks per zone */ >> unsigned int max_open_zones; /* max open zone resources of the zoned device */ >> + struct semaphore available_open_zones; /* available open zones */ >> #endif >> >> /* for node-related operations */ >> @@ -4469,6 +4470,28 @@ static inline bool f2fs_blkz_is_seq(struct f2fs_sb_info *sbi, int devi, >> >> return test_bit(zno, FDEV(devi).blkz_seq); >> } >> + >> +static inline bool f2fs_blkaddr_in_seqzone(struct f2fs_sb_info *sbi, >> + block_t blkaddr) >> +{ >> + int devi = 0; >> + >> + if (f2fs_is_multi_device(sbi)) { >> + devi = f2fs_target_device_index(sbi, blkaddr); >> + if (blkaddr < FDEV(devi).start_blk || >> + blkaddr > FDEV(devi).end_blk) { >> + f2fs_err(sbi, "Invalid block %x", blkaddr); >> + return false; >> + } >> + blkaddr -= FDEV(devi).start_blk; >> + } >> + >> + if (!bdev_is_zoned(FDEV(devi).bdev) || >> + !f2fs_blkz_is_seq(sbi, devi, blkaddr)) >> + return false; >> + >> + return true; >> +} >> #endif >> >> static inline int f2fs_bdev_index(struct f2fs_sb_info *sbi, >> diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c >> index f8703038e1d8..a8626e297876 100644 >> --- a/fs/f2fs/iostat.c >> +++ b/fs/f2fs/iostat.c >> @@ -237,6 +237,13 @@ static inline void __update_iostat_latency(struct bio_iostat_ctx *iostat_ctx, >> spin_unlock_irqrestore(&sbi->iostat_lat_lock, flags); >> } >> >> +void *iostat_get_bio_private(struct bio *bio) >> +{ >> + struct bio_iostat_ctx *iostat_ctx = bio->bi_private; >> + >> + return iostat_ctx->sbi; >> +} >> + >> void iostat_update_and_unbind_ctx(struct bio *bio) >> { >> struct bio_iostat_ctx *iostat_ctx = bio->bi_private; >> diff --git a/fs/f2fs/iostat.h b/fs/f2fs/iostat.h >> index eb99d05cf272..9006c3d41590 100644 >> --- a/fs/f2fs/iostat.h >> +++ b/fs/f2fs/iostat.h >> @@ -58,6 +58,7 @@ static inline struct bio_post_read_ctx *get_post_read_ctx(struct bio *bio) >> return iostat_ctx->post_read_ctx; >> } >> >> +extern void *iostat_get_bio_private(struct bio *bio); >> extern void iostat_update_and_unbind_ctx(struct bio *bio); >> extern void iostat_alloc_and_bind_ctx(struct f2fs_sb_info *sbi, >> struct bio *bio, struct bio_post_read_ctx *ctx); >> @@ -68,6 +69,7 @@ extern void f2fs_destroy_iostat(struct f2fs_sb_info *sbi); >> #else >> static inline void f2fs_update_iostat(struct f2fs_sb_info *sbi, struct inode *inode, >> enum iostat_type type, unsigned long long io_bytes) {} >> +static inline void *iostat_get_bio_private(struct bio *bio) { return bio->bi_private; } >> static inline void iostat_update_and_unbind_ctx(struct bio *bio) {} >> static inline void iostat_alloc_and_bind_ctx(struct f2fs_sb_info *sbi, >> struct bio *bio, struct bio_post_read_ctx *ctx) {} >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c >> index 4fd76e867e0a..9b11cd7888b9 100644 >> --- a/fs/f2fs/segment.c >> +++ b/fs/f2fs/segment.c >> @@ -3140,6 +3140,23 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type, >> return err; >> stat_inc_seg_type(sbi, curseg); >> locate_dirty_segment(sbi, old_segno); >> + >> + >> +#ifdef CONFIG_BLK_DEV_ZONED >> + if (f2fs_sb_has_blkzoned(sbi)) { >> + struct f2fs_bio_info *io; >> + >> + io = sbi->write_io[PAGE_TYPE(type)] + type_to_temp(type); >> + >> + f2fs_down_write(&io->io_rwsem); >> + if (io->zone_openned) { >> + up(&sbi->available_open_zones); >> + io->zone_openned = false; >> + } >> + f2fs_up_write(&io->io_rwsem); >> + } >> +#endif >> + >> return 0; >> } >> >> @@ -3461,12 +3478,7 @@ static int __get_segment_type(struct f2fs_io_info *fio) >> f2fs_bug_on(fio->sbi, true); >> } >> >> - if (IS_HOT(type)) >> - fio->temp = HOT; >> - else if (IS_WARM(type)) >> - fio->temp = WARM; >> - else >> - fio->temp = COLD; >> + fio->temp = type_to_temp(type); >> return type; >> } >> >> @@ -4132,6 +4144,27 @@ static int restore_curseg_summaries(struct f2fs_sb_info *sbi) >> return -EINVAL; >> } >> >> +#ifdef CONFIG_BLK_DEV_ZONED >> + if (f2fs_sb_has_blkzoned(sbi)) { >> + for (type = 0; type < NR_PERSISTENT_LOG; type++) { >> + struct curseg_info *curseg = CURSEG_I(sbi, type); >> + enum page_type ptype; >> + enum temp_type temp; >> + >> + if (!(curseg->next_blkoff % sbi->blocks_per_blkz)) >> + continue; >> + >> + if (!f2fs_blkaddr_in_seqzone(sbi, >> + START_BLOCK(sbi, curseg->segno))) >> + continue; >> + >> + ptype = PAGE_TYPE(type); >> + temp = type_to_temp(type); >> + down(&sbi->available_open_zones); >> + sbi->write_io[ptype][temp].zone_openned = true; >> + } >> + } >> +#endif >> return 0; >> } >> >> @@ -5451,6 +5484,19 @@ static void destroy_curseg(struct f2fs_sb_info *sbi) >> for (i = 0; i < NR_CURSEG_TYPE; i++) { >> kfree(array[i].sum_blk); >> kfree(array[i].journal); >> + >> +#ifdef CONFIG_BLK_DEV_ZONED >> + if (f2fs_sb_has_blkzoned(sbi)) { >> + enum page_type ptype = PAGE_TYPE(i); >> + enum temp_type temp = type_to_temp(i); >> + >> + if (sbi->write_io[ptype][temp].zone_openned) { >> + up(&sbi->available_open_zones); >> + sbi->write_io[ptype][temp].zone_openned = false; >> + } >> + } >> +#endif >> + >> } >> kfree(array); >> } >> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h >> index e1c0f418aa11..855978ca869f 100644 >> --- a/fs/f2fs/segment.h >> +++ b/fs/f2fs/segment.h >> @@ -24,7 +24,8 @@ >> >> #define IS_DATASEG(t) ((t) <= CURSEG_COLD_DATA) >> #define IS_NODESEG(t) ((t) >= CURSEG_HOT_NODE && (t) <= CURSEG_COLD_NODE) >> -#define SE_PAGETYPE(se) ((IS_NODESEG((se)->type) ? NODE : DATA)) >> +#define PAGE_TYPE(t) (IS_NODESEG(t) ? NODE : DATA) >> +#define SE_PAGETYPE(se) (PAGE_TYPE((se)->type)) >> >> static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi, >> unsigned short seg_type) >> @@ -965,3 +966,12 @@ static inline unsigned int first_zoned_segno(struct f2fs_sb_info *sbi) >> return GET_SEGNO(sbi, FDEV(devi).start_blk); >> return 0; >> } >> + >> +static inline enum temp_type type_to_temp(int type) >> +{ >> + if (IS_HOT(type)) >> + return HOT; >> + else if (IS_WARM(type)) >> + return WARM; >> + return COLD; >> +} >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >> index 642540782471..e398800a192d 100644 >> --- a/fs/f2fs/super.c >> +++ b/fs/f2fs/super.c >> @@ -3895,6 +3895,8 @@ static int init_blkz_info(struct f2fs_sb_info *sbi, int devi) >> sbi->max_open_zones, F2FS_OPTION(sbi).active_logs); >> return -EINVAL; >> } >> + >> + sema_init(&sbi->available_open_zones, sbi->max_open_zones); >> } >> >> zone_sectors = bdev_zone_sectors(bdev); >> -- >> 2.40.1 >>
| ![\](/images/icornerr.gif) |