Re: [f2fs-dev] [PATCH] f2fs: accurately adjust free_sections during free_segment_range
"Yeongjin Gil" <[email protected]>
| Newsgroups | net.sourceforge.lists.linux-f2fs-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
> On Thu, Aug 20, 2026 at 6:19 AM Chao Yu <[email protected]> wrote: > > > > On 8/20/26 02:26, Daeho Jeong wrote: > > > On Tue, Aug 18, 2026 at 8:57 PM Chao Yu <[email protected]> wrote: > > >> > > >> On 8/19/26 01:14, Daeho Jeong wrote: > > >>> From: Daeho Jeong <[email protected]> > > >>> > > >>> 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. > > >> > > >> Can you please show me an example for above case? I didn't get it. > > > > > > Here is a concrete example explaining why this adjustment is needed: > > > > Thanks for the detailed explanation. > > > > > > > > Suppose: > > > - Total main sections: MAIN_SECS = 100 (sections 0 .. 99) > > > - Total free sections: free_sections = 30 > > > - We want to shrink the filesystem by 10 sections (secs = 10, range > 90 .. 99). > > > - Within the truncated range (sections 90 .. 99): > > > * 6 sections are already free (free_secmap bit is 0) > > > * 4 sections are in-use with valid blocks that need to be migrated > by GC. > > > When free_segment_range() enters: > > > 1. MAIN_SECS is temporarily reduced from 100 to 90 so that new block > > > allocations are constrained to sections 0 .. 89. > > > 2. The actual number of free sections available in the reduced range > > > (0 .. 89) is only 24 (30 - 6 = 24). > > > 3. Without this patch: > > > - free_sections remains 30 while MAIN_SECS is 90. > > > - During the subsequent GC migrations, free section checks (such as > > > has_not_enough_free_secs()) will over-estimate available space > > > by 6 > > > > But free_segment_range() won't call into has_not_enough_free_secs(), > > if I'm not missing anything. > > Oh, right. I missed it's FG_GC. > I think we can drop this patch. > > Thanks. > I wonder if the free-section adjustment could still be relevant to the SSR/LFS allocation decision during resize. Could you please also check whether free_sections may affect allocation through the following paths? free_segment_range() -> f2fs_allocate_segment_for_resize() -> f2fs_need_SSR() -> new_curseg() -> get_new_segment() Also, when a current segment becomes full during block migration: free_segment_range() -> f2fs_gc_range() -> do_garbage_collect() -> f2fs_allocate_data_block() -> need_new_seg() -> f2fs_need_SSR() -> new_curseg() -> get_new_segment() If free_sections still include free sections in the range being removed, f2fs_need_SSR() may choose LFS allocation instead of SSR. Since get_new_segment() searches only within the temporarily reduced MAIN_SECS range, it may fail to find a free section. Could you please confirm whether this case also needs to be handled? Thanks, > > > > Not sure, maybe you mean other threads will call > > has_not_enough_free_secs(), are you worried about that we may miss > chances to call fggc earlier in below cases: > > > > f2fs_balance_fs -> has_enough_free_secs -> f2fs_gc? I guess it will > > blocked on gc_lock. > > > > > > sections in the active 0 .. 89 range. > > > - If free_segment_range() fails midway (e.g. -EAGAIN), > free_sections > > > accounting becomes inconsistent. > > > > Why free_sections accounting becomes inconsistent if > > free_segment_range() fails midway? > > > > Thanks, > > > > > With this patch: > > > - We count the 6 already-free sections in the truncated range (90 .. > 99) and > > > deduct them from free_sections upon entering (30 - 6 = 24), > perfectly > > > matching the actual free sections in the active range 0 .. 89. > > > - On exit, the deducted amount is restored, keeping free_sections > consistent > > > throughout the entire resize lifecycle. > > > > > > Hope this clarifies the scenario! > > > > > > Thanks, > > > > > >> > > >> BTW, it needs to rebase this patch on dev-test branch. > > >> > > >> Thanks, > > >> > > >>> > > >>> 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. > > >>> > > >>> Signed-off-by: Daeho Jeong <[email protected]> > > >>> Signed-off-by: Sunmin Jeong <[email protected]> > > >>> --- > > >>> 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); > > >>> + > > >>> /* 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; > > >>> + spin_unlock(&FREE_I(sbi)->segmap_lock); > > >>> MAIN_SECS(sbi) += secs; > > >>> return err; > > >>> } > > >> > > > > > _______________________________________________ > Linux-f2fs-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel