* [PATCH v1] f2fs: fix to ensure queued discard commands are properly issued @ 2025-02-04 14:53 Yohan Joung 2025-02-17 12:38 ` Chao Yu 0 siblings, 1 reply; 4+ messages in thread From: Yohan Joung @ 2025-02-04 14:53 UTC (permalink / raw) To: jaegeuk, chao, daeho43; +Cc: linux-f2fs-devel, linux-kernel, Yohan Joung F2FS zone storage requires discard and reset zone for each conventional, zoned device. In the current configuration, Discard granularity is set to the zone size but queuing is inserted into the pend list with a maximum size of the segment size As a result queued commands cannot be issued. so we are restorting discard granularity to its original state Signed-off-by: Yohan Joung <yohan.joung@sk.com> --- fs/f2fs/segment.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index c282e8a0a2ec..938bf5144ae8 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -2320,10 +2320,6 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi) dcc->discard_granularity = DEFAULT_DISCARD_GRANULARITY; dcc->max_ordered_discard = DEFAULT_MAX_ORDERED_DISCARD_GRANULARITY; dcc->discard_io_aware = DPOLICY_IO_AWARE_ENABLE; - if (F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SEGMENT) - dcc->discard_granularity = BLKS_PER_SEG(sbi); - else if (F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SECTION) - dcc->discard_granularity = BLKS_PER_SEC(sbi); INIT_LIST_HEAD(&dcc->entry_list); for (i = 0; i < MAX_PLIST_NUM; i++) -- 2.25.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] f2fs: fix to ensure queued discard commands are properly issued 2025-02-04 14:53 [PATCH v1] f2fs: fix to ensure queued discard commands are properly issued Yohan Joung @ 2025-02-17 12:38 ` Chao Yu 2025-02-21 15:56 ` Daeho Jeong 0 siblings, 1 reply; 4+ messages in thread From: Chao Yu @ 2025-02-17 12:38 UTC (permalink / raw) To: Yohan Joung, jaegeuk, daeho43 Cc: chao, linux-f2fs-devel, linux-kernel, Yohan Joung On 2/4/25 22:53, Yohan Joung wrote: > F2FS zone storage requires discard and reset zone for each conventional, > zoned device. > In the current configuration, Discard granularity is set to the zone > size but queuing is inserted into the pend list with a maximum size of the > segment size As a result queued commands cannot be issued. > so we are restorting discard granularity to its original state It seems commit 4f993264fe29 ("f2fs: introduce discard_unit mount option") introduced a bug: when we enable discard_unit=section option, it will set .discard_granularity to BLKS_PER_SEC(), however discard granularity only supports [1, 512], once section size is not equal to segment size, it will cause bug. blkzoned feature became the victim since it use discard_unit=section option by default. What: /sys/fs/f2fs/<disk>/discard_granularity Date: July 2017 Contact: "Chao Yu" <yuchao0@huawei.com> Description: Controls discard granularity of inner discard thread. Inner thread will not issue discards with size that is smaller than granularity. The unit size is one block(4KB), now only support configuring in range of [1, 512]. Default value is 16. For small devices, default value is 1. What about this? Subject: [PATCH] f2fs: fix to set .discard_granularity correctly commit 4f993264fe29 ("f2fs: introduce discard_unit mount option") introduced a bug, when we enable discard_unit=section option, it will set .discard_granularity to BLKS_PER_SEC(), however discard granularity only supports [1, 512], once section size is not equal to segment size, it will cause issue_discard_thread() in DPOLICY_BG mode will not select discard entry w/ any granularity to issue. Fixes: 4f993264fe29 ("f2fs: introduce discard_unit mount option") Signed-off-by: Yohan Joung <yohan.joung@sk.com> Signed-off-by: Chao Yu <chao@kernel.org> --- fs/f2fs/segment.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 6ebe25eafafa..2b415926641f 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -2320,10 +2320,9 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi) dcc->discard_granularity = DEFAULT_DISCARD_GRANULARITY; dcc->max_ordered_discard = DEFAULT_MAX_ORDERED_DISCARD_GRANULARITY; dcc->discard_io_aware = DPOLICY_IO_AWARE_ENABLE; - if (F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SEGMENT) + if (F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SEGMENT || + F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SECTION) dcc->discard_granularity = BLKS_PER_SEG(sbi); - else if (F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SECTION) - dcc->discard_granularity = BLKS_PER_SEC(sbi); INIT_LIST_HEAD(&dcc->entry_list); for (i = 0; i < MAX_PLIST_NUM; i++) -- 2.48.1.601.g30ceb7b040-goog ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] f2fs: fix to ensure queued discard commands are properly issued 2025-02-17 12:38 ` Chao Yu @ 2025-02-21 15:56 ` Daeho Jeong 2025-02-24 6:11 ` Chao Yu 0 siblings, 1 reply; 4+ messages in thread From: Daeho Jeong @ 2025-02-21 15:56 UTC (permalink / raw) To: Chao Yu; +Cc: Yohan Joung, jaegeuk, linux-f2fs-devel, linux-kernel, Yohan Joung On Mon, Feb 17, 2025 at 4:38 AM Chao Yu <chao@kernel.org> wrote: > > On 2/4/25 22:53, Yohan Joung wrote: > > F2FS zone storage requires discard and reset zone for each conventional, > > zoned device. > > In the current configuration, Discard granularity is set to the zone > > size but queuing is inserted into the pend list with a maximum size of the > > segment size As a result queued commands cannot be issued. > > so we are restorting discard granularity to its original state > > It seems commit 4f993264fe29 ("f2fs: introduce discard_unit mount option") > introduced a bug: when we enable discard_unit=section option, it will set > .discard_granularity to BLKS_PER_SEC(), however discard granularity only > supports [1, 512], once section size is not equal to segment size, it will > cause bug. blkzoned feature became the victim since it use > discard_unit=section option by default. > > What: /sys/fs/f2fs/<disk>/discard_granularity > Date: July 2017 > Contact: "Chao Yu" <yuchao0@huawei.com> > Description: Controls discard granularity of inner discard thread. Inner thread > will not issue discards with size that is smaller than granularity. > The unit size is one block(4KB), now only support configuring > in range of [1, 512]. Default value is 16. > For small devices, default value is 1. > > What about this? > > Subject: [PATCH] f2fs: fix to set .discard_granularity correctly > > commit 4f993264fe29 ("f2fs: introduce discard_unit mount option") introduced > a bug, when we enable discard_unit=section option, it will set > .discard_granularity to BLKS_PER_SEC(), however discard granularity only > supports [1, 512], once section size is not equal to segment size, it will > cause issue_discard_thread() in DPOLICY_BG mode will not select discard entry > w/ any granularity to issue. > > Fixes: 4f993264fe29 ("f2fs: introduce discard_unit mount option") > Signed-off-by: Yohan Joung <yohan.joung@sk.com> > Signed-off-by: Chao Yu <chao@kernel.org> > --- > fs/f2fs/segment.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 6ebe25eafafa..2b415926641f 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -2320,10 +2320,9 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi) > dcc->discard_granularity = DEFAULT_DISCARD_GRANULARITY; > dcc->max_ordered_discard = DEFAULT_MAX_ORDERED_DISCARD_GRANULARITY; > dcc->discard_io_aware = DPOLICY_IO_AWARE_ENABLE; > - if (F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SEGMENT) > + if (F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SEGMENT || > + F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SECTION) > dcc->discard_granularity = BLKS_PER_SEG(sbi); > - else if (F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SECTION) > - dcc->discard_granularity = BLKS_PER_SEC(sbi); > > INIT_LIST_HEAD(&dcc->entry_list); > for (i = 0; i < MAX_PLIST_NUM; i++) > -- > 2.48.1.601.g30ceb7b040-goog > Reviewed-by: Daeho Jeong <daehojeong@google.com> > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] f2fs: fix to ensure queued discard commands are properly issued 2025-02-21 15:56 ` Daeho Jeong @ 2025-02-24 6:11 ` Chao Yu 0 siblings, 0 replies; 4+ messages in thread From: Chao Yu @ 2025-02-24 6:11 UTC (permalink / raw) To: Daeho Jeong Cc: Yohan Joung, jaegeuk, linux-f2fs-devel, linux-kernel, Yohan Joung On 2/21/25 23:56, Daeho Jeong wrote: > On Mon, Feb 17, 2025 at 4:38 AM Chao Yu <chao@kernel.org> wrote: >> >> On 2/4/25 22:53, Yohan Joung wrote: >>> F2FS zone storage requires discard and reset zone for each conventional, >>> zoned device. >>> In the current configuration, Discard granularity is set to the zone >>> size but queuing is inserted into the pend list with a maximum size of the >>> segment size As a result queued commands cannot be issued. >>> so we are restorting discard granularity to its original state >> >> It seems commit 4f993264fe29 ("f2fs: introduce discard_unit mount option") >> introduced a bug: when we enable discard_unit=section option, it will set >> .discard_granularity to BLKS_PER_SEC(), however discard granularity only >> supports [1, 512], once section size is not equal to segment size, it will >> cause bug. blkzoned feature became the victim since it use >> discard_unit=section option by default. >> >> What: /sys/fs/f2fs/<disk>/discard_granularity >> Date: July 2017 >> Contact: "Chao Yu" <yuchao0@huawei.com> >> Description: Controls discard granularity of inner discard thread. Inner thread >> will not issue discards with size that is smaller than granularity. >> The unit size is one block(4KB), now only support configuring >> in range of [1, 512]. Default value is 16. >> For small devices, default value is 1. >> >> What about this? >> >> Subject: [PATCH] f2fs: fix to set .discard_granularity correctly >> >> commit 4f993264fe29 ("f2fs: introduce discard_unit mount option") introduced >> a bug, when we enable discard_unit=section option, it will set >> .discard_granularity to BLKS_PER_SEC(), however discard granularity only >> supports [1, 512], once section size is not equal to segment size, it will >> cause issue_discard_thread() in DPOLICY_BG mode will not select discard entry >> w/ any granularity to issue. >> >> Fixes: 4f993264fe29 ("f2fs: introduce discard_unit mount option") >> Signed-off-by: Yohan Joung <yohan.joung@sk.com> >> Signed-off-by: Chao Yu <chao@kernel.org> >> --- >> fs/f2fs/segment.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c >> index 6ebe25eafafa..2b415926641f 100644 >> --- a/fs/f2fs/segment.c >> +++ b/fs/f2fs/segment.c >> @@ -2320,10 +2320,9 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi) >> dcc->discard_granularity = DEFAULT_DISCARD_GRANULARITY; >> dcc->max_ordered_discard = DEFAULT_MAX_ORDERED_DISCARD_GRANULARITY; >> dcc->discard_io_aware = DPOLICY_IO_AWARE_ENABLE; >> - if (F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SEGMENT) >> + if (F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SEGMENT || >> + F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SECTION) >> dcc->discard_granularity = BLKS_PER_SEG(sbi); >> - else if (F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SECTION) >> - dcc->discard_granularity = BLKS_PER_SEC(sbi); >> >> INIT_LIST_HEAD(&dcc->entry_list); >> for (i = 0; i < MAX_PLIST_NUM; i++) >> -- >> 2.48.1.601.g30ceb7b040-goog >> > > Reviewed-by: Daeho Jeong <daehojeong@google.com> Thanks for the review, let me send a formal patch. Thanks, > >> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-02-24 6:11 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-02-04 14:53 [PATCH v1] f2fs: fix to ensure queued discard commands are properly issued Yohan Joung 2025-02-17 12:38 ` Chao Yu 2025-02-21 15:56 ` Daeho Jeong 2025-02-24 6:11 ` Chao Yu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®