Re: [PATCH 2/2] f2fs: fix missing small discard in fstrim

[Chunhai Guo <guochunhai@vivo.com>]

在 1/3/2025 3:00 PM, Chunhai Guo 写道:
> 在 1/3/2025 11:36 AM, Chao Yu 写道:
>> On 2025/1/2 18:13, Chunhai Guo wrote:
>>> If userspace issues an fstrim with a range that does not include all
>>> segments with small discards, these segments will be reused without being
>> I didn't get it, if fstrim didn't cover those segments, why do we need to
>> issue small discard for out-of-range segments?
> Currently, all the dirty sentries in the dirty_sentries_bitmap are
> handled in the fstrim process regardless of whether they are within the
> fstrim range or not. Therefore, this patch is necessary to address the
> issue.
>
> f2fs_flush_sit_entries()
>       list_for_each_entry_safe(ses, tmp, head, set_list) {
>           for_each_set_bit_from(segno, bitmap, end) {
>               ...
>               __clear_bit(segno, bitmap); // segno is cleared regardless
> of whether or not it is within the fstrim range
>               ...
>           }
>       }


Hi Chao,


Could you please provide some advice on this issue when you have a moment?
Or do you have any questions about my explanation above?

Thanks,

>
> Thanks,
>
>> Thanks,
>>
>>> discarded. This patch fixes this issue.
>>> This patch is somewhat similar to commit 650d3c4e56e1 ("f2fs: fix a missing
>>> discard prefree segments").
>>>
>>> Fixes: d7bc2484b8d4 ("f2fs: fix small discards not to issue redundantly")
>>> Signed-off-by: Chunhai Guo <guochunhai@vivo.com>
>>> ---
>>>     fs/f2fs/segment.c | 10 +++++++---
>>>     1 file changed, 7 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 8fe9f794b581..af9a62591c49 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -4552,6 +4552,8 @@ void f2fs_flush_sit_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>     	struct list_head *head = &SM_I(sbi)->sit_entry_set;
>>>     	bool to_journal = !is_sbi_flag_set(sbi, SBI_IS_RESIZEFS);
>>>     	struct seg_entry *se;
>>> +	bool force = (cpc->reason & CP_DISCARD);
>>> +	__u64 trim_start = cpc->trim_start;
>>>     
>>>     	down_write(&sit_i->sentry_lock);
>>>     
>>> @@ -4609,7 +4611,9 @@ void f2fs_flush_sit_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>     #endif
>>>     
>>>     			/* add discard candidates */
>>> -			if (!(cpc->reason & CP_DISCARD)) {
>>> +			if (!force || (force &&
>>> +					(segno < trim_start ||
>>> +					 segno > cpc->trim_end))) {
>>>     				cpc->trim_start = segno;
>>>     				add_discard_addrs(sbi, cpc, false, false);
>>>     			}
>>> @@ -4649,8 +4653,8 @@ void f2fs_flush_sit_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>     	f2fs_bug_on(sbi, !list_empty(head));
>>>     	f2fs_bug_on(sbi, sit_i->dirty_sentries);
>>>     out:
>>> -	if (cpc->reason & CP_DISCARD) {
>>> -		__u64 trim_start = cpc->trim_start;
>>> +	if (force) {
>>> +		cpc->trim_start = trim_start;
>>>     
>>>     		for (; cpc->trim_start <= cpc->trim_end; cpc->trim_start++)
>>>     			add_discard_addrs(sbi, cpc, true, false);
>