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

[Chao Yu <chao@kernel.org>]

On 1/3/25 15:00, Chunhai Guo wrote:
> 在 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.

fstrim flow doesn't depend on dirty status of segments, right?

It will add discard range w/ below code?

	if (cpc->reason & CP_DISCARD) {
		__u64 trim_start = cpc->trim_start;

		for (; cpc->trim_start <= cpc->trim_end; cpc->trim_start++)
			add_discard_addrs(sbi, cpc, false);

		cpc->trim_start = trim_start;
	}

Thanks,

> 
> 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
>              ...
>          }
>      }
> 
> 
> 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);
> 
>