| Message ID | 20240223205535.307307-1-jaegeuk@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [f2fs-dev,1/5] f2fs: check number of blocks in a current section | expand |
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 <jaegeuk@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; > > - /* 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; > }
On Sun, Feb 25, 2024 at 6:42 PM Chao Yu <chao@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 <jaegeuk@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
In cfd66bb715fd ("f2fs: fix deadloop in foreground GC"), we needed to check
the number of blocks in a section instead of the segment.
Fixes: cfd66bb715fd ("f2fs: fix deadloop in foreground GC")
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
from v1:
- check current node block space to deal with the worst case
- TODO: need to fine tuning on node temperature
fs/f2fs/segment.h | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 3edd3809b6b5..335fc6285fa5 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -561,23 +561,22 @@ 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 in the worst case. */
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;
-
+ left_blocks = CAP_BLKS_PER_SEC(sbi) -
+ get_ckpt_valid_blocks(sbi, segno, true);
if (node_blocks > left_blocks)
return false;
}
- /* 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 +625,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;
}
On 2024/2/27 7:14, 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. > > Fixes: cfd66bb715fd ("f2fs: fix deadloop in foreground GC") > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> Reviewed-by: Chao Yu <chao@kernel.org> Thanks,
Hello: This series was applied to jaegeuk/f2fs.git (dev) by Jaegeuk Kim <jaegeuk@kernel.org>: On Fri, 23 Feb 2024 12:55:31 -0800 you 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 > we don't have exact dirty page count per temperature. Hence, use a rough > estimation. > > [...] Here is the summary with links: - [f2fs-dev,1/5] f2fs: check number of blocks in a current section (no matching commit) - [f2fs-dev,2/5] f2fs: fix write pointers all the time (no matching commit) - [f2fs-dev,3/5] f2fs: print zone status in string and some log (no matching commit) - [f2fs-dev,4/5] f2fs: prevent an f2fs_gc loop during disable_checkpoint https://git.kernel.org/jaegeuk/f2fs/c/496b655d0460 - [f2fs-dev,5/5] f2fs: allow to mount if cap is 100 https://git.kernel.org/jaegeuk/f2fs/c/38fcb47642ae You are awesome, thank you!
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; - /* 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; }
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 we don't have exact dirty page count per temperature. Hence, use a rough estimation. Fixes: cfd66bb715fd ("f2fs: fix deadloop in foreground GC") Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> --- fs/f2fs/segment.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)