diff for duplicates of <001401dd3147$17b1f300$4715d900$@samsung.com> diff --git a/a/1.txt b/N1/1.txt index d79bb4b..361ea14 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,199 +1,191 @@ -> On Thu, Aug 20, 2026 at 6:19 AM Chao Yu <chao@kernel.org> wrote: -> > -> > On 8/20/26 02:26, Daeho Jeong wrote: -> > > On Tue, Aug 18, 2026 at 8:57 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. -> > >> -> > >> 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 <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); -> > >>> + -> > >>> /* 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 -> Linux-f2fs-devel@lists.sourceforge.net +> On Thu, Aug 20, 2026 at 6:19 AM Chao Yu <chao@kernel.org> wrote:\r +> >\r +> > On 8/20/26 02:26, Daeho Jeong wrote:\r +> > > On Tue, Aug 18, 2026 at 8:57 PM Chao Yu <chao@kernel.org> wrote:\r +> > >>\r +> > >> On 8/19/26 01:14, Daeho Jeong wrote:\r +> > >>> From: Daeho Jeong <daehojeong@google.com>\r +> > >>>\r +> > >>> In free_segment_range(), MAIN_SECS(sbi) is temporarily reduced by\r +> > >>> `secs` while valid blocks in the truncated range are evacuated by GC.\r +> > >>>\r +> > >>> However, if any sections within the truncated range were already\r +> > >>> free, failing to deduct them from FREE_I(sbi)->free_sections leads\r +> > >>> to an over-estimation of available space in the reduced main area,\r +> > >>> causing inconsistent free section accounting.\r +> > >>\r +> > >> Can you please show me an example for above case? I didn't get it.\r +> > >\r +> > > Here is a concrete example explaining why this adjustment is needed:\r +> >\r +> > Thanks for the detailed explanation.\r +> >\r +> > >\r +> > > Suppose:\r +> > > - Total main sections: MAIN_SECS = 100 (sections 0 .. 99)\r +> > > - Total free sections: free_sections = 30\r +> > > - We want to shrink the filesystem by 10 sections (secs = 10, range\r +> 90 .. 99).\r +> > > - Within the truncated range (sections 90 .. 99):\r +> > > * 6 sections are already free (free_secmap bit is 0)\r +> > > * 4 sections are in-use with valid blocks that need to be migrated\r +> by GC.\r +> > > When free_segment_range() enters:\r +> > > 1. MAIN_SECS is temporarily reduced from 100 to 90 so that new block\r +> > > allocations are constrained to sections 0 .. 89.\r +> > > 2. The actual number of free sections available in the reduced range\r +> > > (0 .. 89) is only 24 (30 - 6 = 24).\r +> > > 3. Without this patch:\r +> > > - free_sections remains 30 while MAIN_SECS is 90.\r +> > > - During the subsequent GC migrations, free section checks (such as\r +> > > has_not_enough_free_secs()) will over-estimate available space\r +> > > by 6\r +> >\r +> > But free_segment_range() won't call into has_not_enough_free_secs(),\r +> > if I'm not missing anything.\r +> \r +> Oh, right. I missed it's FG_GC.\r +> I think we can drop this patch.\r +> \r +> Thanks.\r +>\r +I wonder if the free-section adjustment could still be relevant\r +to the SSR/LFS allocation decision during resize.\r +\r +Could you please also check whether free_sections may affect\r +allocation through the following paths?\r +\r + free_segment_range()\r + -> f2fs_allocate_segment_for_resize()\r + -> f2fs_need_SSR()\r + -> new_curseg()\r + -> get_new_segment()\r +\r +Also, when a current segment becomes full during block migration:\r +\r + free_segment_range()\r + -> f2fs_gc_range()\r + -> do_garbage_collect()\r + -> f2fs_allocate_data_block()\r + -> need_new_seg()\r + -> f2fs_need_SSR()\r + -> new_curseg()\r + -> get_new_segment()\r +\r +If free_sections still include free sections in the range being\r +removed, f2fs_need_SSR() may choose LFS allocation instead of SSR.\r +Since get_new_segment() searches only within the temporarily reduced\r +MAIN_SECS range, it may fail to find a free section.\r +\r +Could you please confirm whether this case also needs to be handled?\r +\r +Thanks,\r +> >\r +> > Not sure, maybe you mean other threads will call\r +> > has_not_enough_free_secs(), are you worried about that we may miss\r +> chances to call fggc earlier in below cases:\r +> >\r +> > f2fs_balance_fs -> has_enough_free_secs -> f2fs_gc? I guess it will\r +> > blocked on gc_lock.\r +> \r +> >\r +> > > sections in the active 0 .. 89 range.\r +> > > - If free_segment_range() fails midway (e.g. -EAGAIN),\r +> free_sections\r +> > > accounting becomes inconsistent.\r +> >\r +> > Why free_sections accounting becomes inconsistent if\r +> > free_segment_range() fails midway?\r +> >\r +> > Thanks,\r +> >\r +> > > With this patch:\r +> > > - We count the 6 already-free sections in the truncated range (90 ..\r +> 99) and\r +> > > deduct them from free_sections upon entering (30 - 6 = 24),\r +> perfectly\r +> > > matching the actual free sections in the active range 0 .. 89.\r +> > > - On exit, the deducted amount is restored, keeping free_sections\r +> consistent\r +> > > throughout the entire resize lifecycle.\r +> > >\r +> > > Hope this clarifies the scenario!\r +> > >\r +> > > Thanks,\r +> > >\r +> > >>\r +> > >> BTW, it needs to rebase this patch on dev-test branch.\r +> > >>\r +> > >> Thanks,\r +> > >>\r +> > >>>\r +> > >>> Fix this by calculating the number of already-free sections in the\r +> > >>> truncated range under segmap_lock, deducting them from\r +> > >>> free_sections upon entering free_segment_range(), and restoring them\r +> under segmap_lock on exit.\r +> > >>>\r +> > >>> Signed-off-by: Daeho Jeong <daehojeong@google.com>\r +> > >>> Signed-off-by: Sunmin Jeong <s_min.jeong@samsung.com>\r +> > >>> ---\r +> > >>> fs/f2fs/gc.c | 15 ++++++++++++++-\r +> > >>> 1 file changed, 14 insertions(+), 1 deletion(-)\r +> > >>>\r +> > >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index\r +> > >>> 787133ee2eb2..f3a6fc6d08ae 100644\r +> > >>> --- a/fs/f2fs/gc.c\r +> > >>> +++ b/fs/f2fs/gc.c\r +> > >>> @@ -2200,8 +2200,9 @@ int f2fs_gc_range(struct f2fs_sb_info *sbi,\r +> > >>> static int free_segment_range(struct f2fs_sb_info *sbi,\r +> > >>> unsigned int secs, bool dry_run)\r +> > >>> {\r +> > >>> - unsigned int next_inuse, start, end;\r +> > >>> + unsigned int secno, next_inuse, start, end, end_secno;\r +> > >>> struct cp_control cpc = { CP_RESIZE, 0, 0, 0 };\r +> > >>> + unsigned int freed_secs = 0;\r +> > >>> int gc_mode, gc_type;\r +> > >>> int err = 0;\r +> > >>> int type;\r +> > >>> @@ -2210,6 +2211,7 @@ static int free_segment_range(struct\r +> f2fs_sb_info *sbi,\r +> > >>> MAIN_SECS(sbi) -= secs;\r +> > >>> start = MAIN_SECS(sbi) * SEGS_PER_SEC(sbi);\r +> > >>> end = MAIN_SEGS(sbi) - 1;\r +> > >>> + end_secno = GET_SEC_FROM_SEG(sbi, end);\r +> > >>>\r +> > >>> mutex_lock(&DIRTY_I(sbi)->seglist_lock);\r +> > >>> for (gc_mode = 0; gc_mode < MAX_GC_POLICY; gc_mode++) @@\r +> > >>> -2221,6 +2223,14 @@ static int free_segment_range(struct\r +> f2fs_sb_info *sbi,\r +> > >>> sbi->next_victim_seg[gc_type] = NULL_SEGNO;\r +> > >>> mutex_unlock(&DIRTY_I(sbi)->seglist_lock);\r +> > >>>\r +> > >>> + spin_lock(&FREE_I(sbi)->segmap_lock);\r +> > >>> + for (secno = MAIN_SECS(sbi); secno <= end_secno; secno++) {\r +> > >>> + if (!test_bit(secno, FREE_I(sbi)->free_secmap))\r +> > >>> + freed_secs++;\r +> > >>> + }\r +> > >>> + FREE_I(sbi)->free_sections -= freed_secs;\r +> > >>> + spin_unlock(&FREE_I(sbi)->segmap_lock);\r +> > >>> +\r +> > >>> /* Move out cursegs from the target range */\r +> > >>> for (type = CURSEG_HOT_DATA; type < NR_CURSEG_TYPE; type++) {\r +> > >>> err = f2fs_allocate_segment_for_resize(sbi, type,\r +> > >>> start, end); @@ -2245,6 +2255,9 @@ static int\r +> free_segment_range(struct f2fs_sb_info *sbi,\r +> > >>> f2fs_bug_on(sbi, 1);\r +> > >>> }\r +> > >>> out:\r +> > >>> + spin_lock(&FREE_I(sbi)->segmap_lock);\r +> > >>> + FREE_I(sbi)->free_sections += freed_secs;\r +> > >>> + spin_unlock(&FREE_I(sbi)->segmap_lock);\r +> > >>> MAIN_SECS(sbi) += secs;\r +> > >>> return err;\r +> > >>> }\r +> > >>\r +> >\r +> \r +> \r +> _______________________________________________\r +> Linux-f2fs-devel mailing list\r +> Linux-f2fs-devel@lists.sourceforge.net\r > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel - - - - -_______________________________________________ -Linux-f2fs-devel mailing list -Linux-f2fs-devel@lists.sourceforge.net -https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel diff --git a/a/content_digest b/N1/content_digest index aa01515..0c159ce 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -5,214 +5,208 @@ "ref\0CGME20260820161308epcas1p4a14d03c7418eb82241f42d668a0e3d45@epcas1p4.samsung.com\0" "ref\0CACOAw_xdzvksT=C-ZgSEo1gXXyqYF_-CYXWbn-3ZRZkFYGJCKw@mail.gmail.com\0" "From\0Yeongjin Gil <youngjin.gil@samsung.com>\0" - "Subject\0Re: [f2fs-dev] [PATCH] f2fs: accurately adjust free_sections during free_segment_range\0" + "Subject\0RE: [f2fs-dev] [PATCH] f2fs: accurately adjust free_sections during free_segment_range\0" "Date\0Fri, 21 Aug 2026 17:28:49 +0900\0" "To\0'Daeho Jeong' <daeho43@gmail.com>" " 'Chao Yu' <chao@kernel.org>\0" "Cc\0'Daeho Jeong' <daehojeong@google.com>" + kernel-team@android.com linux-kernel@vger.kernel.org linux-f2fs-devel@lists.sourceforge.net - " kernel-team@android.com\0" + 'Yeongjin Gil' <youngjin.gil@samsung.com> + " '\354\240\225\354\204\240\353\257\274' <s_min.jeong@samsung.com>\0" "\00:1\0" "b\0" - "> On Thu, Aug 20, 2026 at 6:19\342\200\257AM Chao Yu <chao@kernel.org> wrote:\n" - "> >\n" - "> > On 8/20/26 02:26, Daeho Jeong wrote:\n" - "> > > On Tue, Aug 18, 2026 at 8:57\342\200\257PM Chao Yu <chao@kernel.org> wrote:\n" - "> > >>\n" - "> > >> On 8/19/26 01:14, Daeho Jeong wrote:\n" - "> > >>> From: Daeho Jeong <daehojeong@google.com>\n" - "> > >>>\n" - "> > >>> In free_segment_range(), MAIN_SECS(sbi) is temporarily reduced by\n" - "> > >>> `secs` while valid blocks in the truncated range are evacuated by GC.\n" - "> > >>>\n" - "> > >>> However, if any sections within the truncated range were already\n" - "> > >>> free, failing to deduct them from FREE_I(sbi)->free_sections leads\n" - "> > >>> to an over-estimation of available space in the reduced main area,\n" - "> > >>> causing inconsistent free section accounting.\n" - "> > >>\n" - "> > >> Can you please show me an example for above case? I didn't get it.\n" - "> > >\n" - "> > > Here is a concrete example explaining why this adjustment is needed:\n" - "> >\n" - "> > Thanks for the detailed explanation.\n" - "> >\n" - "> > >\n" - "> > > Suppose:\n" - "> > > - Total main sections: MAIN_SECS = 100 (sections 0 .. 99)\n" - "> > > - Total free sections: free_sections = 30\n" - "> > > - We want to shrink the filesystem by 10 sections (secs = 10, range\n" - "> 90 .. 99).\n" - "> > > - Within the truncated range (sections 90 .. 99):\n" - "> > > * 6 sections are already free (free_secmap bit is 0)\n" - "> > > * 4 sections are in-use with valid blocks that need to be migrated\n" - "> by GC.\n" - "> > > When free_segment_range() enters:\n" - "> > > 1. MAIN_SECS is temporarily reduced from 100 to 90 so that new block\n" - "> > > allocations are constrained to sections 0 .. 89.\n" - "> > > 2. The actual number of free sections available in the reduced range\n" - "> > > (0 .. 89) is only 24 (30 - 6 = 24).\n" - "> > > 3. Without this patch:\n" - "> > > - free_sections remains 30 while MAIN_SECS is 90.\n" - "> > > - During the subsequent GC migrations, free section checks (such as\n" - "> > > has_not_enough_free_secs()) will over-estimate available space\n" - "> > > by 6\n" - "> >\n" - "> > But free_segment_range() won't call into has_not_enough_free_secs(),\n" - "> > if I'm not missing anything.\n" - "> \n" - "> Oh, right. I missed it's FG_GC.\n" - "> I think we can drop this patch.\n" - "> \n" - "> Thanks.\n" - ">\n" - "I wonder if the free-section adjustment could still be relevant\n" - "to the SSR/LFS allocation decision during resize.\n" - "\n" - "Could you please also check whether free_sections may affect\n" - "allocation through the following paths?\n" - "\n" - " free_segment_range()\n" - " -> f2fs_allocate_segment_for_resize()\n" - " -> f2fs_need_SSR()\n" - " -> new_curseg()\n" - " -> get_new_segment()\n" - "\n" - "Also, when a current segment becomes full during block migration:\n" - "\n" - " free_segment_range()\n" - " -> f2fs_gc_range()\n" - " -> do_garbage_collect()\n" - " -> f2fs_allocate_data_block()\n" - " -> need_new_seg()\n" - " -> f2fs_need_SSR()\n" - " -> new_curseg()\n" - " -> get_new_segment()\n" - "\n" - "If free_sections still include free sections in the range being\n" - "removed, f2fs_need_SSR() may choose LFS allocation instead of SSR.\n" - "Since get_new_segment() searches only within the temporarily reduced\n" - "MAIN_SECS range, it may fail to find a free section.\n" - "\n" - "Could you please confirm whether this case also needs to be handled?\n" - "\n" - "Thanks,\n" - "> >\n" - "> > Not sure, maybe you mean other threads will call\n" - "> > has_not_enough_free_secs(), are you worried about that we may miss\n" - "> chances to call fggc earlier in below cases:\n" - "> >\n" - "> > f2fs_balance_fs -> has_enough_free_secs -> f2fs_gc? I guess it will\n" - "> > blocked on gc_lock.\n" - "> \n" - "> >\n" - "> > > sections in the active 0 .. 89 range.\n" - "> > > - If free_segment_range() fails midway (e.g. -EAGAIN),\n" - "> free_sections\n" - "> > > accounting becomes inconsistent.\n" - "> >\n" - "> > Why free_sections accounting becomes inconsistent if\n" - "> > free_segment_range() fails midway?\n" - "> >\n" - "> > Thanks,\n" - "> >\n" - "> > > With this patch:\n" - "> > > - We count the 6 already-free sections in the truncated range (90 ..\n" - "> 99) and\n" - "> > > deduct them from free_sections upon entering (30 - 6 = 24),\n" - "> perfectly\n" - "> > > matching the actual free sections in the active range 0 .. 89.\n" - "> > > - On exit, the deducted amount is restored, keeping free_sections\n" - "> consistent\n" - "> > > throughout the entire resize lifecycle.\n" - "> > >\n" - "> > > Hope this clarifies the scenario!\n" - "> > >\n" - "> > > Thanks,\n" - "> > >\n" - "> > >>\n" - "> > >> BTW, it needs to rebase this patch on dev-test branch.\n" - "> > >>\n" - "> > >> Thanks,\n" - "> > >>\n" - "> > >>>\n" - "> > >>> Fix this by calculating the number of already-free sections in the\n" - "> > >>> truncated range under segmap_lock, deducting them from\n" - "> > >>> free_sections upon entering free_segment_range(), and restoring them\n" - "> under segmap_lock on exit.\n" - "> > >>>\n" - "> > >>> Signed-off-by: Daeho Jeong <daehojeong@google.com>\n" - "> > >>> Signed-off-by: Sunmin Jeong <s_min.jeong@samsung.com>\n" - "> > >>> ---\n" - "> > >>> fs/f2fs/gc.c | 15 ++++++++++++++-\n" - "> > >>> 1 file changed, 14 insertions(+), 1 deletion(-)\n" - "> > >>>\n" - "> > >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index\n" - "> > >>> 787133ee2eb2..f3a6fc6d08ae 100644\n" - "> > >>> --- a/fs/f2fs/gc.c\n" - "> > >>> +++ b/fs/f2fs/gc.c\n" - "> > >>> @@ -2200,8 +2200,9 @@ int f2fs_gc_range(struct f2fs_sb_info *sbi,\n" - "> > >>> static int free_segment_range(struct f2fs_sb_info *sbi,\n" - "> > >>> unsigned int secs, bool dry_run)\n" - "> > >>> {\n" - "> > >>> - unsigned int next_inuse, start, end;\n" - "> > >>> + unsigned int secno, next_inuse, start, end, end_secno;\n" - "> > >>> struct cp_control cpc = { CP_RESIZE, 0, 0, 0 };\n" - "> > >>> + unsigned int freed_secs = 0;\n" - "> > >>> int gc_mode, gc_type;\n" - "> > >>> int err = 0;\n" - "> > >>> int type;\n" - "> > >>> @@ -2210,6 +2211,7 @@ static int free_segment_range(struct\n" - "> f2fs_sb_info *sbi,\n" - "> > >>> MAIN_SECS(sbi) -= secs;\n" - "> > >>> start = MAIN_SECS(sbi) * SEGS_PER_SEC(sbi);\n" - "> > >>> end = MAIN_SEGS(sbi) - 1;\n" - "> > >>> + end_secno = GET_SEC_FROM_SEG(sbi, end);\n" - "> > >>>\n" - "> > >>> mutex_lock(&DIRTY_I(sbi)->seglist_lock);\n" - "> > >>> for (gc_mode = 0; gc_mode < MAX_GC_POLICY; gc_mode++) @@\n" - "> > >>> -2221,6 +2223,14 @@ static int free_segment_range(struct\n" - "> f2fs_sb_info *sbi,\n" - "> > >>> sbi->next_victim_seg[gc_type] = NULL_SEGNO;\n" - "> > >>> mutex_unlock(&DIRTY_I(sbi)->seglist_lock);\n" - "> > >>>\n" - "> > >>> + spin_lock(&FREE_I(sbi)->segmap_lock);\n" - "> > >>> + for (secno = MAIN_SECS(sbi); secno <= end_secno; secno++) {\n" - "> > >>> + if (!test_bit(secno, FREE_I(sbi)->free_secmap))\n" - "> > >>> + freed_secs++;\n" - "> > >>> + }\n" - "> > >>> + FREE_I(sbi)->free_sections -= freed_secs;\n" - "> > >>> + spin_unlock(&FREE_I(sbi)->segmap_lock);\n" - "> > >>> +\n" - "> > >>> /* Move out cursegs from the target range */\n" - "> > >>> for (type = CURSEG_HOT_DATA; type < NR_CURSEG_TYPE; type++) {\n" - "> > >>> err = f2fs_allocate_segment_for_resize(sbi, type,\n" - "> > >>> start, end); @@ -2245,6 +2255,9 @@ static int\n" - "> free_segment_range(struct f2fs_sb_info *sbi,\n" - "> > >>> f2fs_bug_on(sbi, 1);\n" - "> > >>> }\n" - "> > >>> out:\n" - "> > >>> + spin_lock(&FREE_I(sbi)->segmap_lock);\n" - "> > >>> + FREE_I(sbi)->free_sections += freed_secs;\n" - "> > >>> + spin_unlock(&FREE_I(sbi)->segmap_lock);\n" - "> > >>> MAIN_SECS(sbi) += secs;\n" - "> > >>> return err;\n" - "> > >>> }\n" - "> > >>\n" - "> >\n" - "> \n" - "> \n" - "> _______________________________________________\n" - "> Linux-f2fs-devel mailing list\n" - "> Linux-f2fs-devel@lists.sourceforge.net\n" - "> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel\n" - "\n" - "\n" - "\n" - "\n" - "_______________________________________________\n" - "Linux-f2fs-devel mailing list\n" - "Linux-f2fs-devel@lists.sourceforge.net\n" - https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel + "> On Thu, Aug 20, 2026 at 6:19\342\200\257AM Chao Yu <chao@kernel.org> wrote:\r\n" + "> >\r\n" + "> > On 8/20/26 02:26, Daeho Jeong wrote:\r\n" + "> > > On Tue, Aug 18, 2026 at 8:57\342\200\257PM Chao Yu <chao@kernel.org> wrote:\r\n" + "> > >>\r\n" + "> > >> On 8/19/26 01:14, Daeho Jeong wrote:\r\n" + "> > >>> From: Daeho Jeong <daehojeong@google.com>\r\n" + "> > >>>\r\n" + "> > >>> In free_segment_range(), MAIN_SECS(sbi) is temporarily reduced by\r\n" + "> > >>> `secs` while valid blocks in the truncated range are evacuated by GC.\r\n" + "> > >>>\r\n" + "> > >>> However, if any sections within the truncated range were already\r\n" + "> > >>> free, failing to deduct them from FREE_I(sbi)->free_sections leads\r\n" + "> > >>> to an over-estimation of available space in the reduced main area,\r\n" + "> > >>> causing inconsistent free section accounting.\r\n" + "> > >>\r\n" + "> > >> Can you please show me an example for above case? I didn't get it.\r\n" + "> > >\r\n" + "> > > Here is a concrete example explaining why this adjustment is needed:\r\n" + "> >\r\n" + "> > Thanks for the detailed explanation.\r\n" + "> >\r\n" + "> > >\r\n" + "> > > Suppose:\r\n" + "> > > - Total main sections: MAIN_SECS = 100 (sections 0 .. 99)\r\n" + "> > > - Total free sections: free_sections = 30\r\n" + "> > > - We want to shrink the filesystem by 10 sections (secs = 10, range\r\n" + "> 90 .. 99).\r\n" + "> > > - Within the truncated range (sections 90 .. 99):\r\n" + "> > > * 6 sections are already free (free_secmap bit is 0)\r\n" + "> > > * 4 sections are in-use with valid blocks that need to be migrated\r\n" + "> by GC.\r\n" + "> > > When free_segment_range() enters:\r\n" + "> > > 1. MAIN_SECS is temporarily reduced from 100 to 90 so that new block\r\n" + "> > > allocations are constrained to sections 0 .. 89.\r\n" + "> > > 2. The actual number of free sections available in the reduced range\r\n" + "> > > (0 .. 89) is only 24 (30 - 6 = 24).\r\n" + "> > > 3. Without this patch:\r\n" + "> > > - free_sections remains 30 while MAIN_SECS is 90.\r\n" + "> > > - During the subsequent GC migrations, free section checks (such as\r\n" + "> > > has_not_enough_free_secs()) will over-estimate available space\r\n" + "> > > by 6\r\n" + "> >\r\n" + "> > But free_segment_range() won't call into has_not_enough_free_secs(),\r\n" + "> > if I'm not missing anything.\r\n" + "> \r\n" + "> Oh, right. I missed it's FG_GC.\r\n" + "> I think we can drop this patch.\r\n" + "> \r\n" + "> Thanks.\r\n" + ">\r\n" + "I wonder if the free-section adjustment could still be relevant\r\n" + "to the SSR/LFS allocation decision during resize.\r\n" + "\r\n" + "Could you please also check whether free_sections may affect\r\n" + "allocation through the following paths?\r\n" + "\r\n" + " free_segment_range()\r\n" + " -> f2fs_allocate_segment_for_resize()\r\n" + " -> f2fs_need_SSR()\r\n" + " -> new_curseg()\r\n" + " -> get_new_segment()\r\n" + "\r\n" + "Also, when a current segment becomes full during block migration:\r\n" + "\r\n" + " free_segment_range()\r\n" + " -> f2fs_gc_range()\r\n" + " -> do_garbage_collect()\r\n" + " -> f2fs_allocate_data_block()\r\n" + " -> need_new_seg()\r\n" + " -> f2fs_need_SSR()\r\n" + " -> new_curseg()\r\n" + " -> get_new_segment()\r\n" + "\r\n" + "If free_sections still include free sections in the range being\r\n" + "removed, f2fs_need_SSR() may choose LFS allocation instead of SSR.\r\n" + "Since get_new_segment() searches only within the temporarily reduced\r\n" + "MAIN_SECS range, it may fail to find a free section.\r\n" + "\r\n" + "Could you please confirm whether this case also needs to be handled?\r\n" + "\r\n" + "Thanks,\r\n" + "> >\r\n" + "> > Not sure, maybe you mean other threads will call\r\n" + "> > has_not_enough_free_secs(), are you worried about that we may miss\r\n" + "> chances to call fggc earlier in below cases:\r\n" + "> >\r\n" + "> > f2fs_balance_fs -> has_enough_free_secs -> f2fs_gc? I guess it will\r\n" + "> > blocked on gc_lock.\r\n" + "> \r\n" + "> >\r\n" + "> > > sections in the active 0 .. 89 range.\r\n" + "> > > - If free_segment_range() fails midway (e.g. -EAGAIN),\r\n" + "> free_sections\r\n" + "> > > accounting becomes inconsistent.\r\n" + "> >\r\n" + "> > Why free_sections accounting becomes inconsistent if\r\n" + "> > free_segment_range() fails midway?\r\n" + "> >\r\n" + "> > Thanks,\r\n" + "> >\r\n" + "> > > With this patch:\r\n" + "> > > - We count the 6 already-free sections in the truncated range (90 ..\r\n" + "> 99) and\r\n" + "> > > deduct them from free_sections upon entering (30 - 6 = 24),\r\n" + "> perfectly\r\n" + "> > > matching the actual free sections in the active range 0 .. 89.\r\n" + "> > > - On exit, the deducted amount is restored, keeping free_sections\r\n" + "> consistent\r\n" + "> > > throughout the entire resize lifecycle.\r\n" + "> > >\r\n" + "> > > Hope this clarifies the scenario!\r\n" + "> > >\r\n" + "> > > Thanks,\r\n" + "> > >\r\n" + "> > >>\r\n" + "> > >> BTW, it needs to rebase this patch on dev-test branch.\r\n" + "> > >>\r\n" + "> > >> Thanks,\r\n" + "> > >>\r\n" + "> > >>>\r\n" + "> > >>> Fix this by calculating the number of already-free sections in the\r\n" + "> > >>> truncated range under segmap_lock, deducting them from\r\n" + "> > >>> free_sections upon entering free_segment_range(), and restoring them\r\n" + "> under segmap_lock on exit.\r\n" + "> > >>>\r\n" + "> > >>> Signed-off-by: Daeho Jeong <daehojeong@google.com>\r\n" + "> > >>> Signed-off-by: Sunmin Jeong <s_min.jeong@samsung.com>\r\n" + "> > >>> ---\r\n" + "> > >>> fs/f2fs/gc.c | 15 ++++++++++++++-\r\n" + "> > >>> 1 file changed, 14 insertions(+), 1 deletion(-)\r\n" + "> > >>>\r\n" + "> > >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index\r\n" + "> > >>> 787133ee2eb2..f3a6fc6d08ae 100644\r\n" + "> > >>> --- a/fs/f2fs/gc.c\r\n" + "> > >>> +++ b/fs/f2fs/gc.c\r\n" + "> > >>> @@ -2200,8 +2200,9 @@ int f2fs_gc_range(struct f2fs_sb_info *sbi,\r\n" + "> > >>> static int free_segment_range(struct f2fs_sb_info *sbi,\r\n" + "> > >>> unsigned int secs, bool dry_run)\r\n" + "> > >>> {\r\n" + "> > >>> - unsigned int next_inuse, start, end;\r\n" + "> > >>> + unsigned int secno, next_inuse, start, end, end_secno;\r\n" + "> > >>> struct cp_control cpc = { CP_RESIZE, 0, 0, 0 };\r\n" + "> > >>> + unsigned int freed_secs = 0;\r\n" + "> > >>> int gc_mode, gc_type;\r\n" + "> > >>> int err = 0;\r\n" + "> > >>> int type;\r\n" + "> > >>> @@ -2210,6 +2211,7 @@ static int free_segment_range(struct\r\n" + "> f2fs_sb_info *sbi,\r\n" + "> > >>> MAIN_SECS(sbi) -= secs;\r\n" + "> > >>> start = MAIN_SECS(sbi) * SEGS_PER_SEC(sbi);\r\n" + "> > >>> end = MAIN_SEGS(sbi) - 1;\r\n" + "> > >>> + end_secno = GET_SEC_FROM_SEG(sbi, end);\r\n" + "> > >>>\r\n" + "> > >>> mutex_lock(&DIRTY_I(sbi)->seglist_lock);\r\n" + "> > >>> for (gc_mode = 0; gc_mode < MAX_GC_POLICY; gc_mode++) @@\r\n" + "> > >>> -2221,6 +2223,14 @@ static int free_segment_range(struct\r\n" + "> f2fs_sb_info *sbi,\r\n" + "> > >>> sbi->next_victim_seg[gc_type] = NULL_SEGNO;\r\n" + "> > >>> mutex_unlock(&DIRTY_I(sbi)->seglist_lock);\r\n" + "> > >>>\r\n" + "> > >>> + spin_lock(&FREE_I(sbi)->segmap_lock);\r\n" + "> > >>> + for (secno = MAIN_SECS(sbi); secno <= end_secno; secno++) {\r\n" + "> > >>> + if (!test_bit(secno, FREE_I(sbi)->free_secmap))\r\n" + "> > >>> + freed_secs++;\r\n" + "> > >>> + }\r\n" + "> > >>> + FREE_I(sbi)->free_sections -= freed_secs;\r\n" + "> > >>> + spin_unlock(&FREE_I(sbi)->segmap_lock);\r\n" + "> > >>> +\r\n" + "> > >>> /* Move out cursegs from the target range */\r\n" + "> > >>> for (type = CURSEG_HOT_DATA; type < NR_CURSEG_TYPE; type++) {\r\n" + "> > >>> err = f2fs_allocate_segment_for_resize(sbi, type,\r\n" + "> > >>> start, end); @@ -2245,6 +2255,9 @@ static int\r\n" + "> free_segment_range(struct f2fs_sb_info *sbi,\r\n" + "> > >>> f2fs_bug_on(sbi, 1);\r\n" + "> > >>> }\r\n" + "> > >>> out:\r\n" + "> > >>> + spin_lock(&FREE_I(sbi)->segmap_lock);\r\n" + "> > >>> + FREE_I(sbi)->free_sections += freed_secs;\r\n" + "> > >>> + spin_unlock(&FREE_I(sbi)->segmap_lock);\r\n" + "> > >>> MAIN_SECS(sbi) += secs;\r\n" + "> > >>> return err;\r\n" + "> > >>> }\r\n" + "> > >>\r\n" + "> >\r\n" + "> \r\n" + "> \r\n" + "> _______________________________________________\r\n" + "> Linux-f2fs-devel mailing list\r\n" + "> Linux-f2fs-devel@lists.sourceforge.net\r\n" + > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel -97fdced0505c0dcbd579be1cd17a58d479b41a89740e94785d1cb5ac4149b9c6 +4f079b1e01db22990547429f38b422cdc2761999ee7cc6f184a9fd60b6b9b591
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.