Re: [f2fs-dev] [PATCH] f2fs: accurately adjust free_sections during free_segment_range

[Chao Yu <chao@kernel.org>]

On 8/25/26 09:21, Daeho Jeong wrote:
> On Mon, Aug 24, 2026 at 6:05 PM Chao Yu <chao@kernel.org> wrote:
>>
>> On 8/19/26 01:14, Daeho Jeong wrote:
>>> From: Daeho Jeong <daehojeong@google.com>
>>>
>>> In free_segment_range(), MAIN_SECS(sbi) is temporarily reduced by `secs`
>>> while valid blocks in the truncated range are evacuated by GC.
>>>
>>> However, if any sections within the truncated range were already free,
>>> failing to deduct them from FREE_I(sbi)->free_sections leads to an
>>> over-estimation of available space in the reduced main area, causing
>>> inconsistent free section accounting.
>>>
>>> Fix this by calculating the number of already-free sections in the
>>> truncated range under segmap_lock, deducting them from free_sections upon
>>> entering free_segment_range(), and restoring them under segmap_lock on exit.
>>
>> I think we need to change commit message a bit to reflect real issues?
>>
>> Fixes and Cc stable line here.
>>
>>>
>>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
>>> Signed-off-by: Sunmin Jeong <s_min.jeong@samsung.com>
>>> ---
>>>    fs/f2fs/gc.c | 15 ++++++++++++++-
>>>    1 file changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>> index 787133ee2eb2..f3a6fc6d08ae 100644
>>> --- a/fs/f2fs/gc.c
>>> +++ b/fs/f2fs/gc.c
>>> @@ -2200,8 +2200,9 @@ int f2fs_gc_range(struct f2fs_sb_info *sbi,
>>>    static int free_segment_range(struct f2fs_sb_info *sbi,
>>>                                unsigned int secs, bool dry_run)
>>>    {
>>> -     unsigned int next_inuse, start, end;
>>> +     unsigned int secno, next_inuse, start, end, end_secno;
>>>        struct cp_control cpc = { CP_RESIZE, 0, 0, 0 };
>>> +     unsigned int freed_secs = 0;
>>>        int gc_mode, gc_type;
>>>        int err = 0;
>>>        int type;
>>> @@ -2210,6 +2211,7 @@ static int free_segment_range(struct f2fs_sb_info *sbi,
>>>        MAIN_SECS(sbi) -= secs;
>>>        start = MAIN_SECS(sbi) * SEGS_PER_SEC(sbi);
>>>        end = MAIN_SEGS(sbi) - 1;
>>> +     end_secno = GET_SEC_FROM_SEG(sbi, end);
>>>
>>>        mutex_lock(&DIRTY_I(sbi)->seglist_lock);
>>>        for (gc_mode = 0; gc_mode < MAX_GC_POLICY; gc_mode++)
>>> @@ -2221,6 +2223,14 @@ static int free_segment_range(struct f2fs_sb_info *sbi,
>>>                        sbi->next_victim_seg[gc_type] = NULL_SEGNO;
>>>        mutex_unlock(&DIRTY_I(sbi)->seglist_lock);
>>>
>>> +     spin_lock(&FREE_I(sbi)->segmap_lock);
>>> +     for (secno = MAIN_SECS(sbi); secno <= end_secno; secno++) {
>>> +             if (!test_bit(secno, FREE_I(sbi)->free_secmap))
>>> +                     freed_secs++;
>>> +     }
>>> +     FREE_I(sbi)->free_sections -= freed_secs;
>>> +     spin_unlock(&FREE_I(sbi)->segmap_lock);
>>> +
>>
>> Before f2fs_gc_range
>> 0..89 have 10 dirty/full sections, 80 free sections.
>> 90..99 have 5 dirty sections, 5 free sections.
>>
>> FREE_I(sbi)->free_sections -= freed_secs;
>> free_sections = 85 - 5 = 80
>>
>> After f2fs_gc_range
>> 0..89 have 13 dirty/full sections, 77 free sections.
>> 90..99 have 10 free sections.
>> free_sections = 77 (.free_sections) + 10 (secs) = 87
>>
>>>        /* Move out cursegs from the target range */
>>>        for (type = CURSEG_HOT_DATA; type < NR_CURSEG_TYPE; type++) {
>>>                err = f2fs_allocate_segment_for_resize(sbi, type, start, end);
>>> @@ -2245,6 +2255,9 @@ static int free_segment_range(struct f2fs_sb_info *sbi,
>>>                f2fs_bug_on(sbi, 1);
>>>        }
>>>    out:
>>> +     spin_lock(&FREE_I(sbi)->segmap_lock);
>>> +     FREE_I(sbi)->free_sections += freed_secs;
>>
>> So, shouldn't be?
>> FREE_I(sbi)->free_sections += secs;
> 
> During f2fs_gc_range(), when the dirty sections in 90..99 are cleaned and
> freed, `__set_test_and_free()` is invoked for those sections, which
> automatically increments `FREE_I(sbi)->free_sections` for each newly
> freed section?

Ah, I see, thanks for correcting me. :)

Thanks,

> 
>>
>> Thanks,
>>
>>> +     spin_unlock(&FREE_I(sbi)->segmap_lock);
>>>        MAIN_SECS(sbi) += secs;
>>>        return err;
>>>    }
>>