* [PATCH v1] f2fs: fix has_curseg_enough_space to check all data segments for dentry blocks
@ 2025-11-10 13:26 Xiaole He
2025-11-11 2:44 ` [f2fs-dev] " Yongpeng Yang
0 siblings, 1 reply; 8+ messages in thread
From: Xiaole He @ 2025-11-10 13:26 UTC (permalink / raw)
To: linux-f2fs-devel; +Cc: jaegeuk, chao, linux-kernel, Xiaole He, stable
When active_logs == 6, dentry blocks can be allocated to HOT, WARM, or
COLD segments based on various conditions in __get_segment_type_6():
- age extent cache (if enabled)
- FI_HOT_DATA flag (set when dirty_pages <= min_hot_blocks)
- rw_hint (defaults to WARM via f2fs_rw_hint_to_seg_type)
- file_is_hot(), FI_NEED_IPU, f2fs_is_cow_file(), etc.
However, has_curseg_enough_space() only checked CURSEG_HOT_DATA segment
for dentry blocks, which could lead to incorrect space calculation when
dentry blocks are actually allocated to WARM or COLD segments.
Reproducer:
Note: This reproducer requires adding a tracepoint to observe segment
type. Add the following tracepoint to include/trace/events/f2fs.h:
TRACE_EVENT(f2fs_allocate_data_block,
TP_PROTO(struct f2fs_sb_info *sbi, struct inode *inode,
enum log_type type, block_t blkaddr),
TP_ARGS(sbi, inode, type, blkaddr),
TP_STRUCT__entry(
__field(dev_t, dev)
__field(ino_t, ino)
__field(int, type)
__field(block_t, blkaddr)
__field(int, is_dir)
),
TP_fast_assign(
__entry->dev = sbi->sb->s_dev;
__entry->ino = inode ? inode->i_ino : 0;
__entry->type = type;
__entry->blkaddr = blkaddr;
__entry->is_dir = inode ? S_ISDIR(inode->i_mode) : 0;
),
TP_printk("dev = (%d,%d), ino = %lu, %s, blkaddr = %u, is_dir = %d",
show_dev(__entry->dev),
(unsigned long)__entry->ino,
show_data_type(__entry->type),
__entry->blkaddr,
__entry->is_dir)
);
And add the tracepoint call in fs/f2fs/segment.c in
f2fs_allocate_data_block() function. Find the location after
locate_dirty_segment() calls and before IS_DATASEG() check:
locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr));
locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr));
trace_f2fs_allocate_data_block(sbi, folio ? folio->mapping->host : NULL,
type, *new_blkaddr);
if (IS_DATASEG(curseg->seg_type))
1. Mount F2FS with active_logs=6 and age extent cache disabled:
# mkfs.f2fs -f /dev/sdb1
# mount -t f2fs -o active_logs=6 /dev/sdb1 /mnt/f2fs-test
2. Enable tracing and f2fs_allocate_data_block tracepoint:
# echo 1 > /sys/kernel/debug/tracing/events/f2fs/f2fs_allocate_data_block/enable
# echo 1 > /sys/kernel/debug/tracing/tracing_on
# echo > /sys/kernel/debug/tracing/trace
3. Create a directory and write enough files to trigger dirty_pages >
min_hot_blocks (default 16), which will clear FI_HOT_DATA flag:
# mkdir /mnt/f2fs-test/testdir
# cd /mnt/f2fs-test/testdir
# seq 1 8192 | xargs touch
# sync
4. Observe dentry block allocation:
# cat /sys/kernel/debug/tracing/trace
The trace output shows dentry blocks (is_dir = 1) allocated to WARM
segment because FI_HOT_DATA is cleared when dirty_pages >
min_hot_blocks (default 16). However, has_curseg_enough_space() only
checked HOT_DATA segment space.
Fix by checking all three data segments (HOT, WARM, COLD) when
active_logs == 6, similar to how __get_segment_type_6() can return
any of these segment types for dentry blocks.
Fixes: ef095d19e82f ("f2fs: write small sized IO to hot log")
Cc: stable@kernel.org
Signed-off-by: Xiaole He <hexiaole1994@126.com>
---
fs/f2fs/segment.h | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 1ce2c8abaf48..c13400a53013 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -632,15 +632,34 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
}
/* check current data section for dentry blocks. */
- segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno;
+ if (F2FS_OPTION(sbi).active_logs == 6) {
+ /*
+ * With active_logs == 6, dentry blocks can be allocated to
+ * HOT, WARM, or COLD segments based on age extent cache,
+ * FI_HOT_DATA flag, rw_hint, etc. Check all three.
+ */
+ for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++) {
+ segno = CURSEG_I(sbi, i)->segno;
+
+ if (unlikely(segno == NULL_SEGNO))
+ return false;
+
+ left_blocks = get_left_section_blocks(sbi, i, segno);
+
+ if (dent_blocks > left_blocks)
+ return false;
+ }
+ } else {
+ segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno;
- if (unlikely(segno == NULL_SEGNO))
- return false;
+ if (unlikely(segno == NULL_SEGNO))
+ return false;
- left_blocks = get_left_section_blocks(sbi, CURSEG_HOT_DATA, segno);
+ left_blocks = get_left_section_blocks(sbi, CURSEG_HOT_DATA, segno);
- if (dent_blocks > left_blocks)
- return false;
+ if (dent_blocks > left_blocks)
+ return false;
+ }
return true;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [f2fs-dev] [PATCH v1] f2fs: fix has_curseg_enough_space to check all data segments for dentry blocks 2025-11-10 13:26 [PATCH v1] f2fs: fix has_curseg_enough_space to check all data segments for dentry blocks Xiaole He @ 2025-11-11 2:44 ` Yongpeng Yang 2025-11-11 6:00 ` [PATCH v2] " Xiaole He 0 siblings, 1 reply; 8+ messages in thread From: Yongpeng Yang @ 2025-11-11 2:44 UTC (permalink / raw) To: Xiaole He, linux-f2fs-devel; +Cc: jaegeuk, linux-kernel, stable, Yongpeng Yang On 11/10/25 21:26, Xiaole He wrote: > When active_logs == 6, dentry blocks can be allocated to HOT, WARM, or > COLD segments based on various conditions in __get_segment_type_6(): > - age extent cache (if enabled) > - FI_HOT_DATA flag (set when dirty_pages <= min_hot_blocks) > - rw_hint (defaults to WARM via f2fs_rw_hint_to_seg_type) > - file_is_hot(), FI_NEED_IPU, f2fs_is_cow_file(), etc. > > However, has_curseg_enough_space() only checked CURSEG_HOT_DATA segment > for dentry blocks, which could lead to incorrect space calculation when > dentry blocks are actually allocated to WARM or COLD segments. > > Reproducer: > Note: This reproducer requires adding a tracepoint to observe segment > type. Add the following tracepoint to include/trace/events/f2fs.h: > > TRACE_EVENT(f2fs_allocate_data_block, > TP_PROTO(struct f2fs_sb_info *sbi, struct inode *inode, > enum log_type type, block_t blkaddr), > > TP_ARGS(sbi, inode, type, blkaddr), > > TP_STRUCT__entry( > __field(dev_t, dev) > __field(ino_t, ino) > __field(int, type) > __field(block_t, blkaddr) > __field(int, is_dir) > ), > > TP_fast_assign( > __entry->dev = sbi->sb->s_dev; > __entry->ino = inode ? inode->i_ino : 0; > __entry->type = type; > __entry->blkaddr = blkaddr; > __entry->is_dir = inode ? S_ISDIR(inode->i_mode) : 0; > ), > > TP_printk("dev = (%d,%d), ino = %lu, %s, blkaddr = %u, is_dir = %d", > show_dev(__entry->dev), > (unsigned long)__entry->ino, > show_data_type(__entry->type), > __entry->blkaddr, > __entry->is_dir) > ); > > And add the tracepoint call in fs/f2fs/segment.c in > f2fs_allocate_data_block() function. Find the location after > locate_dirty_segment() calls and before IS_DATASEG() check: > > locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr)); > locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr)); > > trace_f2fs_allocate_data_block(sbi, folio ? folio->mapping->host : NULL, > type, *new_blkaddr); > > if (IS_DATASEG(curseg->seg_type)) > > 1. Mount F2FS with active_logs=6 and age extent cache disabled: > # mkfs.f2fs -f /dev/sdb1 > # mount -t f2fs -o active_logs=6 /dev/sdb1 /mnt/f2fs-test > > 2. Enable tracing and f2fs_allocate_data_block tracepoint: > # echo 1 > /sys/kernel/debug/tracing/events/f2fs/f2fs_allocate_data_block/enable > # echo 1 > /sys/kernel/debug/tracing/tracing_on > # echo > /sys/kernel/debug/tracing/trace > > 3. Create a directory and write enough files to trigger dirty_pages > > min_hot_blocks (default 16), which will clear FI_HOT_DATA flag: > # mkdir /mnt/f2fs-test/testdir > # cd /mnt/f2fs-test/testdir > # seq 1 8192 | xargs touch > # sync > > 4. Observe dentry block allocation: > # cat /sys/kernel/debug/tracing/trace > > The trace output shows dentry blocks (is_dir = 1) allocated to WARM > segment because FI_HOT_DATA is cleared when dirty_pages > > min_hot_blocks (default 16). However, has_curseg_enough_space() only > checked HOT_DATA segment space. > > Fix by checking all three data segments (HOT, WARM, COLD) when > active_logs == 6, similar to how __get_segment_type_6() can return > any of these segment types for dentry blocks. > > Fixes: ef095d19e82f ("f2fs: write small sized IO to hot log") > Cc: stable@kernel.org > Signed-off-by: Xiaole He <hexiaole1994@126.com> > --- > fs/f2fs/segment.h | 31 +++++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 6 deletions(-) > > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h > index 1ce2c8abaf48..c13400a53013 100644 > --- a/fs/f2fs/segment.h > +++ b/fs/f2fs/segment.h > @@ -632,15 +632,34 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi, > } > > /* check current data section for dentry blocks. */ > - segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno; > + if (F2FS_OPTION(sbi).active_logs == 6) { > + /* > + * With active_logs == 6, dentry blocks can be allocated to > + * HOT, WARM, or COLD segments based on age extent cache, > + * FI_HOT_DATA flag, rw_hint, etc. Check all three. > + */ > + for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++) { > + segno = CURSEG_I(sbi, i)->segno; > + > + if (unlikely(segno == NULL_SEGNO)) > + return false; > + > + left_blocks = get_left_section_blocks(sbi, i, segno); > + > + if (dent_blocks > left_blocks) > + return false; > + } How about merging this for-loop with the one above to avoid duplicated code? Since both dent_blocks and data_blocks may write to data segments, should we also check whether data_blocks + dent_blocks > left_blocks? Yongpeng, > + } else { > + segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno; > > - if (unlikely(segno == NULL_SEGNO)) > - return false; > + if (unlikely(segno == NULL_SEGNO)) > + return false; > > - left_blocks = get_left_section_blocks(sbi, CURSEG_HOT_DATA, segno); > + left_blocks = get_left_section_blocks(sbi, CURSEG_HOT_DATA, segno); > > - if (dent_blocks > left_blocks) > - return false; > + if (dent_blocks > left_blocks) > + return false; > + } > return true; > } > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] f2fs: fix has_curseg_enough_space to check all data segments for dentry blocks 2025-11-11 2:44 ` [f2fs-dev] " Yongpeng Yang @ 2025-11-11 6:00 ` Xiaole He 2025-11-11 6:10 ` Xiaole He 0 siblings, 1 reply; 8+ messages in thread From: Xiaole He @ 2025-11-11 6:00 UTC (permalink / raw) To: linux-f2fs-devel; +Cc: jaegeuk, chao, linux-kernel Hi Yongpeng, Thanks for your feedback! I've updated the patch per your suggestions: - Merged the dentry block check into the main loop to avoid duplication - Check data_blocks + dent_blocks for data segments since both can write to the same segment Please see the v2 patch. Best regards, Xiaole ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] f2fs: fix has_curseg_enough_space to check all data segments for dentry blocks 2025-11-11 6:00 ` [PATCH v2] " Xiaole He @ 2025-11-11 6:10 ` Xiaole He 2025-11-11 6:29 ` [f2fs-dev] " Yongpeng Yang ` (3 more replies) 0 siblings, 4 replies; 8+ messages in thread From: Xiaole He @ 2025-11-11 6:10 UTC (permalink / raw) To: linux-f2fs-devel; +Cc: jaegeuk, chao, linux-kernel, Xiaole He, stable When active_logs == 6, dentry blocks can be allocated to HOT, WARM, or COLD segments based on various conditions in __get_segment_type_6(): - age extent cache (if enabled) - FI_HOT_DATA flag (set when dirty_pages <= min_hot_blocks) - rw_hint (defaults to WARM via f2fs_rw_hint_to_seg_type) - file_is_hot(), FI_NEED_IPU, f2fs_is_cow_file(), etc. However, has_curseg_enough_space() only checked CURSEG_HOT_DATA segment for dentry blocks, which could lead to incorrect space calculation when dentry blocks are actually allocated to WARM or COLD segments. Reproducer: Note: This reproducer requires adding a tracepoint to observe segment type. Add the following tracepoint to include/trace/events/f2fs.h: TRACE_EVENT(f2fs_allocate_data_block, TP_PROTO(struct f2fs_sb_info *sbi, struct inode *inode, enum log_type type, block_t blkaddr), TP_ARGS(sbi, inode, type, blkaddr), TP_STRUCT__entry( __field(dev_t, dev) __field(ino_t, ino) __field(int, type) __field(block_t, blkaddr) __field(int, is_dir) ), TP_fast_assign( __entry->dev = sbi->sb->s_dev; __entry->ino = inode ? inode->i_ino : 0; __entry->type = type; __entry->blkaddr = blkaddr; __entry->is_dir = inode ? S_ISDIR(inode->i_mode) : 0; ), TP_printk("dev = (%d,%d), ino = %lu, %s, blkaddr = %u, is_dir = %d", show_dev(__entry->dev), (unsigned long)__entry->ino, show_data_type(__entry->type), __entry->blkaddr, __entry->is_dir) ); And add the tracepoint call in fs/f2fs/segment.c in f2fs_allocate_data_block() function. Find the location after locate_dirty_segment() calls and before IS_DATASEG() check: locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr)); locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr)); trace_f2fs_allocate_data_block(sbi, folio ? folio->mapping->host : NULL, type, *new_blkaddr); if (IS_DATASEG(curseg->seg_type)) 1. Mount F2FS with active_logs=6 and age extent cache disabled: # mkfs.f2fs -f /dev/sdb1 # mount -t f2fs -o active_logs=6 /dev/sdb1 /mnt/f2fs-test 2. Enable tracing and f2fs_allocate_data_block tracepoint: # echo 1 > /sys/kernel/debug/tracing/events/f2fs/f2fs_allocate_data_block/enable # echo 1 > /sys/kernel/debug/tracing/tracing_on # echo > /sys/kernel/debug/tracing/trace 3. Create a directory and write enough files to trigger dirty_pages > min_hot_blocks (default 16), which will clear FI_HOT_DATA flag: # mkdir /mnt/f2fs-test/testdir # cd /mnt/f2fs-test/testdir # seq 1 8192 | xargs touch # sync 4. Observe dentry block allocation: # cat /sys/kernel/debug/tracing/trace The trace output shows dentry blocks (is_dir = 1) allocated to WARM segment because FI_HOT_DATA is cleared when dirty_pages > min_hot_blocks (default 16). However, has_curseg_enough_space() only checked HOT_DATA segment space. Fix by merging the dentry block check into the main data/node block check loop and checking data_blocks + dent_blocks for data segments, since both regular data blocks and dentry blocks can be written to the same segment. When active_logs == 6, dentry blocks can be allocated to any of the three data segments (HOT, WARM, COLD), so all three segments need to account for dentry blocks. When active_logs != 6, dentry blocks are always allocated to HOT_DATA segment only, so only HOT_DATA segment needs to account for dentry blocks, while WARM and COLD segments only check data_blocks. Fixes: ef095d19e82f ("f2fs: write small sized IO to hot log") Cc: stable@kernel.org Signed-off-by: Xiaole He <hexiaole1994@126.com> --- Changes in v2 (per Yongpeng's feedback): - Merged dentry block check into the main loop to avoid duplication - Check data_blocks + dent_blocks for data segments (both can write to same segment) --- fs/f2fs/segment.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h index 1ce2c8abaf48..acda720a54eb 100644 --- a/fs/f2fs/segment.h +++ b/fs/f2fs/segment.h @@ -626,21 +626,21 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi, left_blocks = get_left_section_blocks(sbi, i, segno); - blocks = i <= CURSEG_COLD_DATA ? data_blocks : node_blocks; + if (i <= CURSEG_COLD_DATA) { + blocks = data_blocks; + /* + * With active_logs == 6, dentry blocks can be allocated to + * any data segment. With active_logs != 6, dentry blocks + * are always allocated to HOT_DATA segment. + */ + if ((F2FS_OPTION(sbi).active_logs == 6) || (i == CURSEG_HOT_DATA)) + blocks += dent_blocks; + } else { + blocks = node_blocks; + } if (blocks > left_blocks) return false; } - - /* check current data section for dentry blocks. */ - segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno; - - if (unlikely(segno == NULL_SEGNO)) - return false; - - left_blocks = get_left_section_blocks(sbi, CURSEG_HOT_DATA, segno); - - if (dent_blocks > left_blocks) - return false; return true; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix has_curseg_enough_space to check all data segments for dentry blocks 2025-11-11 6:10 ` Xiaole He @ 2025-11-11 6:29 ` Yongpeng Yang 2025-11-11 7:17 ` Chao Yu ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Yongpeng Yang @ 2025-11-11 6:29 UTC (permalink / raw) To: Xiaole He, linux-f2fs-devel; +Cc: jaegeuk, linux-kernel, stable, Yongpeng Yang On 11/11/25 14:10, Xiaole He wrote: > When active_logs == 6, dentry blocks can be allocated to HOT, WARM, or > COLD segments based on various conditions in __get_segment_type_6(): > - age extent cache (if enabled) > - FI_HOT_DATA flag (set when dirty_pages <= min_hot_blocks) > - rw_hint (defaults to WARM via f2fs_rw_hint_to_seg_type) > - file_is_hot(), FI_NEED_IPU, f2fs_is_cow_file(), etc. > > However, has_curseg_enough_space() only checked CURSEG_HOT_DATA segment > for dentry blocks, which could lead to incorrect space calculation when > dentry blocks are actually allocated to WARM or COLD segments. > > Reproducer: > Note: This reproducer requires adding a tracepoint to observe segment > type. Add the following tracepoint to include/trace/events/f2fs.h: > > TRACE_EVENT(f2fs_allocate_data_block, > TP_PROTO(struct f2fs_sb_info *sbi, struct inode *inode, > enum log_type type, block_t blkaddr), > > TP_ARGS(sbi, inode, type, blkaddr), > > TP_STRUCT__entry( > __field(dev_t, dev) > __field(ino_t, ino) > __field(int, type) > __field(block_t, blkaddr) > __field(int, is_dir) > ), > > TP_fast_assign( > __entry->dev = sbi->sb->s_dev; > __entry->ino = inode ? inode->i_ino : 0; > __entry->type = type; > __entry->blkaddr = blkaddr; > __entry->is_dir = inode ? S_ISDIR(inode->i_mode) : 0; > ), > > TP_printk("dev = (%d,%d), ino = %lu, %s, blkaddr = %u, is_dir = %d", > show_dev(__entry->dev), > (unsigned long)__entry->ino, > show_data_type(__entry->type), > __entry->blkaddr, > __entry->is_dir) > ); > > And add the tracepoint call in fs/f2fs/segment.c in > f2fs_allocate_data_block() function. Find the location after > locate_dirty_segment() calls and before IS_DATASEG() check: > > locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr)); > locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr)); > > trace_f2fs_allocate_data_block(sbi, folio ? folio->mapping->host : NULL, > type, *new_blkaddr); > > if (IS_DATASEG(curseg->seg_type)) > > 1. Mount F2FS with active_logs=6 and age extent cache disabled: > # mkfs.f2fs -f /dev/sdb1 > # mount -t f2fs -o active_logs=6 /dev/sdb1 /mnt/f2fs-test > > 2. Enable tracing and f2fs_allocate_data_block tracepoint: > # echo 1 > /sys/kernel/debug/tracing/events/f2fs/f2fs_allocate_data_block/enable > # echo 1 > /sys/kernel/debug/tracing/tracing_on > # echo > /sys/kernel/debug/tracing/trace > > 3. Create a directory and write enough files to trigger dirty_pages > > min_hot_blocks (default 16), which will clear FI_HOT_DATA flag: > # mkdir /mnt/f2fs-test/testdir > # cd /mnt/f2fs-test/testdir > # seq 1 8192 | xargs touch > # sync > > 4. Observe dentry block allocation: > # cat /sys/kernel/debug/tracing/trace > > The trace output shows dentry blocks (is_dir = 1) allocated to WARM > segment because FI_HOT_DATA is cleared when dirty_pages > > min_hot_blocks (default 16). However, has_curseg_enough_space() only > checked HOT_DATA segment space. > > Fix by merging the dentry block check into the main data/node block > check loop and checking data_blocks + dent_blocks for data segments, > since both regular data blocks and dentry blocks can be written to the > same segment. When active_logs == 6, dentry blocks can be allocated to > any of the three data segments (HOT, WARM, COLD), so all three segments > need to account for dentry blocks. When active_logs != 6, dentry blocks > are always allocated to HOT_DATA segment only, so only HOT_DATA segment > needs to account for dentry blocks, while WARM and COLD segments only > check data_blocks. > > Fixes: ef095d19e82f ("f2fs: write small sized IO to hot log") > Cc: stable@kernel.org > Signed-off-by: Xiaole He <hexiaole1994@126.com> > --- > Changes in v2 (per Yongpeng's feedback): > - Merged dentry block check into the main loop to avoid duplication > - Check data_blocks + dent_blocks for data segments (both can write to same segment) > --- > fs/f2fs/segment.h | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h > index 1ce2c8abaf48..acda720a54eb 100644 > --- a/fs/f2fs/segment.h > +++ b/fs/f2fs/segment.h > @@ -626,21 +626,21 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi, > > left_blocks = get_left_section_blocks(sbi, i, segno); > > - blocks = i <= CURSEG_COLD_DATA ? data_blocks : node_blocks; > + if (i <= CURSEG_COLD_DATA) { > + blocks = data_blocks; > + /* > + * With active_logs == 6, dentry blocks can be allocated to > + * any data segment. With active_logs != 6, dentry blocks > + * are always allocated to HOT_DATA segment. > + */ > + if ((F2FS_OPTION(sbi).active_logs == 6) || (i == CURSEG_HOT_DATA)) > + blocks += dent_blocks; > + } else { > + blocks = node_blocks; > + } > if (blocks > left_blocks) > return false; > } > - > - /* check current data section for dentry blocks. */ > - segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno; > - > - if (unlikely(segno == NULL_SEGNO)) > - return false; > - > - left_blocks = get_left_section_blocks(sbi, CURSEG_HOT_DATA, segno); > - > - if (dent_blocks > left_blocks) > - return false; > return true; > } > Looks good to me, Reviewed-by: Yongpeng Yang <yangyongpeng@xiaomi.com> Thanks Yongpeng, ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] f2fs: fix has_curseg_enough_space to check all data segments for dentry blocks 2025-11-11 6:10 ` Xiaole He 2025-11-11 6:29 ` [f2fs-dev] " Yongpeng Yang @ 2025-11-11 7:17 ` Chao Yu 2025-11-24 18:50 ` [f2fs-dev] " patchwork-bot+f2fs 2025-12-02 18:00 ` Jaegeuk Kim 3 siblings, 0 replies; 8+ messages in thread From: Chao Yu @ 2025-11-11 7:17 UTC (permalink / raw) To: Xiaole He, linux-f2fs-devel; +Cc: chao, jaegeuk, linux-kernel, stable On 11/11/25 14:10, Xiaole He wrote: > When active_logs == 6, dentry blocks can be allocated to HOT, WARM, or > COLD segments based on various conditions in __get_segment_type_6(): > - age extent cache (if enabled) > - FI_HOT_DATA flag (set when dirty_pages <= min_hot_blocks) > - rw_hint (defaults to WARM via f2fs_rw_hint_to_seg_type) > - file_is_hot(), FI_NEED_IPU, f2fs_is_cow_file(), etc. > > However, has_curseg_enough_space() only checked CURSEG_HOT_DATA segment > for dentry blocks, which could lead to incorrect space calculation when > dentry blocks are actually allocated to WARM or COLD segments. > > Reproducer: > Note: This reproducer requires adding a tracepoint to observe segment > type. Add the following tracepoint to include/trace/events/f2fs.h: > > TRACE_EVENT(f2fs_allocate_data_block, > TP_PROTO(struct f2fs_sb_info *sbi, struct inode *inode, > enum log_type type, block_t blkaddr), > > TP_ARGS(sbi, inode, type, blkaddr), > > TP_STRUCT__entry( > __field(dev_t, dev) > __field(ino_t, ino) > __field(int, type) > __field(block_t, blkaddr) > __field(int, is_dir) > ), > > TP_fast_assign( > __entry->dev = sbi->sb->s_dev; > __entry->ino = inode ? inode->i_ino : 0; > __entry->type = type; > __entry->blkaddr = blkaddr; > __entry->is_dir = inode ? S_ISDIR(inode->i_mode) : 0; > ), > > TP_printk("dev = (%d,%d), ino = %lu, %s, blkaddr = %u, is_dir = %d", > show_dev(__entry->dev), > (unsigned long)__entry->ino, > show_data_type(__entry->type), > __entry->blkaddr, > __entry->is_dir) > ); > > And add the tracepoint call in fs/f2fs/segment.c in > f2fs_allocate_data_block() function. Find the location after > locate_dirty_segment() calls and before IS_DATASEG() check: > > locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr)); > locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr)); > > trace_f2fs_allocate_data_block(sbi, folio ? folio->mapping->host : NULL, > type, *new_blkaddr); > > if (IS_DATASEG(curseg->seg_type)) > > 1. Mount F2FS with active_logs=6 and age extent cache disabled: > # mkfs.f2fs -f /dev/sdb1 > # mount -t f2fs -o active_logs=6 /dev/sdb1 /mnt/f2fs-test > > 2. Enable tracing and f2fs_allocate_data_block tracepoint: > # echo 1 > /sys/kernel/debug/tracing/events/f2fs/f2fs_allocate_data_block/enable > # echo 1 > /sys/kernel/debug/tracing/tracing_on > # echo > /sys/kernel/debug/tracing/trace > > 3. Create a directory and write enough files to trigger dirty_pages > > min_hot_blocks (default 16), which will clear FI_HOT_DATA flag: > # mkdir /mnt/f2fs-test/testdir > # cd /mnt/f2fs-test/testdir > # seq 1 8192 | xargs touch > # sync > > 4. Observe dentry block allocation: > # cat /sys/kernel/debug/tracing/trace > > The trace output shows dentry blocks (is_dir = 1) allocated to WARM > segment because FI_HOT_DATA is cleared when dirty_pages > > min_hot_blocks (default 16). However, has_curseg_enough_space() only > checked HOT_DATA segment space. > > Fix by merging the dentry block check into the main data/node block > check loop and checking data_blocks + dent_blocks for data segments, > since both regular data blocks and dentry blocks can be written to the > same segment. When active_logs == 6, dentry blocks can be allocated to > any of the three data segments (HOT, WARM, COLD), so all three segments > need to account for dentry blocks. When active_logs != 6, dentry blocks > are always allocated to HOT_DATA segment only, so only HOT_DATA segment > needs to account for dentry blocks, while WARM and COLD segments only > check data_blocks. > > Fixes: ef095d19e82f ("f2fs: write small sized IO to hot log") > Cc: stable@kernel.org > Signed-off-by: Xiaole He <hexiaole1994@126.com> Reviewed-by: Chao Yu <chao@kernel.org> Thanks, ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix has_curseg_enough_space to check all data segments for dentry blocks 2025-11-11 6:10 ` Xiaole He 2025-11-11 6:29 ` [f2fs-dev] " Yongpeng Yang 2025-11-11 7:17 ` Chao Yu @ 2025-11-24 18:50 ` patchwork-bot+f2fs 2025-12-02 18:00 ` Jaegeuk Kim 3 siblings, 0 replies; 8+ messages in thread From: patchwork-bot+f2fs @ 2025-11-24 18:50 UTC (permalink / raw) To: Xiaole He; +Cc: linux-f2fs-devel, jaegeuk, linux-kernel, stable Hello: This patch was applied to jaegeuk/f2fs.git (dev) by Jaegeuk Kim <jaegeuk@kernel.org>: On Tue, 11 Nov 2025 14:10:51 +0800 you wrote: > When active_logs == 6, dentry blocks can be allocated to HOT, WARM, or > COLD segments based on various conditions in __get_segment_type_6(): > - age extent cache (if enabled) > - FI_HOT_DATA flag (set when dirty_pages <= min_hot_blocks) > - rw_hint (defaults to WARM via f2fs_rw_hint_to_seg_type) > - file_is_hot(), FI_NEED_IPU, f2fs_is_cow_file(), etc. > > [...] Here is the summary with links: - [f2fs-dev,v2] f2fs: fix has_curseg_enough_space to check all data segments for dentry blocks https://git.kernel.org/jaegeuk/f2fs/c/6d87364f7e94 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] f2fs: fix has_curseg_enough_space to check all data segments for dentry blocks 2025-11-11 6:10 ` Xiaole He ` (2 preceding siblings ...) 2025-11-24 18:50 ` [f2fs-dev] " patchwork-bot+f2fs @ 2025-12-02 18:00 ` Jaegeuk Kim 3 siblings, 0 replies; 8+ messages in thread From: Jaegeuk Kim @ 2025-12-02 18:00 UTC (permalink / raw) To: Xiaole He; +Cc: linux-f2fs-devel, chao, linux-kernel, stable I need to drop this patch, since this breaks xfstests such as f2fs/005. Please revisit the issue. On 11/11, Xiaole He wrote: > When active_logs == 6, dentry blocks can be allocated to HOT, WARM, or > COLD segments based on various conditions in __get_segment_type_6(): > - age extent cache (if enabled) > - FI_HOT_DATA flag (set when dirty_pages <= min_hot_blocks) > - rw_hint (defaults to WARM via f2fs_rw_hint_to_seg_type) > - file_is_hot(), FI_NEED_IPU, f2fs_is_cow_file(), etc. > > However, has_curseg_enough_space() only checked CURSEG_HOT_DATA segment > for dentry blocks, which could lead to incorrect space calculation when > dentry blocks are actually allocated to WARM or COLD segments. > > Reproducer: > Note: This reproducer requires adding a tracepoint to observe segment > type. Add the following tracepoint to include/trace/events/f2fs.h: > > TRACE_EVENT(f2fs_allocate_data_block, > TP_PROTO(struct f2fs_sb_info *sbi, struct inode *inode, > enum log_type type, block_t blkaddr), > > TP_ARGS(sbi, inode, type, blkaddr), > > TP_STRUCT__entry( > __field(dev_t, dev) > __field(ino_t, ino) > __field(int, type) > __field(block_t, blkaddr) > __field(int, is_dir) > ), > > TP_fast_assign( > __entry->dev = sbi->sb->s_dev; > __entry->ino = inode ? inode->i_ino : 0; > __entry->type = type; > __entry->blkaddr = blkaddr; > __entry->is_dir = inode ? S_ISDIR(inode->i_mode) : 0; > ), > > TP_printk("dev = (%d,%d), ino = %lu, %s, blkaddr = %u, is_dir = %d", > show_dev(__entry->dev), > (unsigned long)__entry->ino, > show_data_type(__entry->type), > __entry->blkaddr, > __entry->is_dir) > ); > > And add the tracepoint call in fs/f2fs/segment.c in > f2fs_allocate_data_block() function. Find the location after > locate_dirty_segment() calls and before IS_DATASEG() check: > > locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr)); > locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr)); > > trace_f2fs_allocate_data_block(sbi, folio ? folio->mapping->host : NULL, > type, *new_blkaddr); > > if (IS_DATASEG(curseg->seg_type)) > > 1. Mount F2FS with active_logs=6 and age extent cache disabled: > # mkfs.f2fs -f /dev/sdb1 > # mount -t f2fs -o active_logs=6 /dev/sdb1 /mnt/f2fs-test > > 2. Enable tracing and f2fs_allocate_data_block tracepoint: > # echo 1 > /sys/kernel/debug/tracing/events/f2fs/f2fs_allocate_data_block/enable > # echo 1 > /sys/kernel/debug/tracing/tracing_on > # echo > /sys/kernel/debug/tracing/trace > > 3. Create a directory and write enough files to trigger dirty_pages > > min_hot_blocks (default 16), which will clear FI_HOT_DATA flag: > # mkdir /mnt/f2fs-test/testdir > # cd /mnt/f2fs-test/testdir > # seq 1 8192 | xargs touch > # sync > > 4. Observe dentry block allocation: > # cat /sys/kernel/debug/tracing/trace > > The trace output shows dentry blocks (is_dir = 1) allocated to WARM > segment because FI_HOT_DATA is cleared when dirty_pages > > min_hot_blocks (default 16). However, has_curseg_enough_space() only > checked HOT_DATA segment space. > > Fix by merging the dentry block check into the main data/node block > check loop and checking data_blocks + dent_blocks for data segments, > since both regular data blocks and dentry blocks can be written to the > same segment. When active_logs == 6, dentry blocks can be allocated to > any of the three data segments (HOT, WARM, COLD), so all three segments > need to account for dentry blocks. When active_logs != 6, dentry blocks > are always allocated to HOT_DATA segment only, so only HOT_DATA segment > needs to account for dentry blocks, while WARM and COLD segments only > check data_blocks. > > Fixes: ef095d19e82f ("f2fs: write small sized IO to hot log") > Cc: stable@kernel.org > Signed-off-by: Xiaole He <hexiaole1994@126.com> > --- > Changes in v2 (per Yongpeng's feedback): > - Merged dentry block check into the main loop to avoid duplication > - Check data_blocks + dent_blocks for data segments (both can write to same segment) > --- > fs/f2fs/segment.h | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h > index 1ce2c8abaf48..acda720a54eb 100644 > --- a/fs/f2fs/segment.h > +++ b/fs/f2fs/segment.h > @@ -626,21 +626,21 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi, > > left_blocks = get_left_section_blocks(sbi, i, segno); > > - blocks = i <= CURSEG_COLD_DATA ? data_blocks : node_blocks; > + if (i <= CURSEG_COLD_DATA) { > + blocks = data_blocks; > + /* > + * With active_logs == 6, dentry blocks can be allocated to > + * any data segment. With active_logs != 6, dentry blocks > + * are always allocated to HOT_DATA segment. > + */ > + if ((F2FS_OPTION(sbi).active_logs == 6) || (i == CURSEG_HOT_DATA)) > + blocks += dent_blocks; > + } else { > + blocks = node_blocks; > + } > if (blocks > left_blocks) > return false; > } > - > - /* check current data section for dentry blocks. */ > - segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno; > - > - if (unlikely(segno == NULL_SEGNO)) > - return false; > - > - left_blocks = get_left_section_blocks(sbi, CURSEG_HOT_DATA, segno); > - > - if (dent_blocks > left_blocks) > - return false; > return true; > } > > -- > 2.43.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-12-02 18:00 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-10 13:26 [PATCH v1] f2fs: fix has_curseg_enough_space to check all data segments for dentry blocks Xiaole He 2025-11-11 2:44 ` [f2fs-dev] " Yongpeng Yang 2025-11-11 6:00 ` [PATCH v2] " Xiaole He 2025-11-11 6:10 ` Xiaole He 2025-11-11 6:29 ` [f2fs-dev] " Yongpeng Yang 2025-11-11 7:17 ` Chao Yu 2025-11-24 18:50 ` [f2fs-dev] " patchwork-bot+f2fs 2025-12-02 18:00 ` Jaegeuk Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox