On Sun, Feb 25, 2024 at 6:42 PM Chao Yu <c...@kernel.org> wrote:
>
> On 2024/2/24 4:55, Jaegeuk Kim wrote:
> > In cfd66bb715fd ("f2fs: fix deadloop in foreground GC"), we needed to check
> > the number of blocks in a section instead of the segment.
> >
> > In addtion, let's check the entire node sections when checking the avaiable
> > node block space. It does not match one to one per temperature, but
> > currently
>
> I tested this patch w/ testcase in [1], it doesn't complain.
>
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=215914
>
> > we don't have exact dirty page count per temperature. Hence, use a rough
> > estimation.
>
> Do we need more accurate per-temperature dirty node count for extreme
> corner case?
>
> e.g. node_blocks is counted from hot dirty nodes, and current hot_node
> log has no enough free space for it.
>
> Thanks,
>
> >
> > Fixes: cfd66bb715fd ("f2fs: fix deadloop in foreground GC")
> > Signed-off-by: Jaegeuk Kim <jaeg...@kernel.org>
> > ---
> > fs/f2fs/segment.h | 22 +++++++++++-----------
> > 1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> > index 3edd3809b6b5..15bf5edd9b3c 100644
> > --- a/fs/f2fs/segment.h
> > +++ b/fs/f2fs/segment.h
> > @@ -561,23 +561,23 @@ static inline bool has_curseg_enough_space(struct
> > f2fs_sb_info *sbi,
> > unsigned int node_blocks, unsigned int dent_blocks)
> > {
> >
> > - unsigned int segno, left_blocks;
> > + unsigned segno, left_blocks;
> > int i;
> >
> > - /* check current node segment */
> > + /* check current node sections, which counts very roughly */
> > + left_blocks = 0;
> > for (i = CURSEG_HOT_NODE; i <= CURSEG_COLD_NODE; i++) {
> > segno = CURSEG_I(sbi, i)->segno;
> > - left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
> > - get_seg_entry(sbi, segno)->ckpt_valid_blocks;
> > -
> > - if (node_blocks > left_blocks)
> > - return false;
> > + left_blocks += CAP_BLKS_PER_SEC(sbi) -
> > + get_ckpt_valid_blocks(sbi, segno, true);
> > }
> > + if (node_blocks > left_blocks)
> > + return false;
I am not sure this is okay. The current implementation of f2fs may not
be optimal when the HOT_NODE section's space requirements exceed its
current capacity. In such cases, f2fs necessitates the creation of a
new free section to accommodate the overflow, even though there might
be free space available in other NODE sections. To address this issue,
it may be necessary to implement a more accurate accounting system for
NODE sections based on their temperature levels.
> >
> > - /* check current data segment */
> > + /* check current data section for dentry blocks. */
> > segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno;
> > - left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
> > - get_seg_entry(sbi, segno)->ckpt_valid_blocks;
> > + left_blocks = CAP_BLKS_PER_SEC(sbi) -
> > + get_ckpt_valid_blocks(sbi, segno, true);
> > if (dent_blocks > left_blocks)
> > return false;
> > return true;
> > @@ -626,7 +626,7 @@ static inline bool has_not_enough_free_secs(struct
> > f2fs_sb_info *sbi,
> >
> > if (free_secs > upper_secs)
> > return false;
> > - else if (free_secs <= lower_secs)
> > + if (free_secs <= lower_secs)
> > return true;
> > return !curseg_space;
> > }
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> 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
