linux-kernel - Re: [PATCH v1] f2fs: fix to ensure queued discard commands are properly issued

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <105491ee-17a1-4f6d-aa0c-5cb5864d185c@kernel.org>
Date: Mon, 24 Feb 2025 14:11:38 +0800
From: Chao Yu <chao@...nel.org>
To: Daeho Jeong <daeho43@...il.com>
Cc: Yohan Joung <jyh429@...il.com>, jaegeuk@...nel.org,
 linux-f2fs-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
 Yohan Joung <yohan.joung@...com>
Subject: Re: [PATCH v1] f2fs: fix to ensure queued discard commands are
 properly issued

On 2/21/25 23:56, Daeho Jeong wrote:
> On Mon, Feb 17, 2025 at 4:38 AM Chao Yu <chao@...nel.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@...wei.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@...com>
>> Signed-off-by: Chao Yu <chao@...nel.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@...gle.com>

Thanks for the review, let me send a formal patch.

Thanks,

> 
>>