mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Chao Yu <chao@kernel.org>
To: Xiaole He <hexiaole1994@126.com>, linux-f2fs-devel@lists.sourceforge.net
Cc: chao@kernel.org, jaegeuk@kernel.org,
	linux-kernel@vger.kernel.org, stable@kernel.org
Subject: Re: [PATCH v2] f2fs: fix has_curseg_enough_space to check all data segments for dentry blocks
Date: Tue, 11 Nov 2025 15:17:50 +0800	[thread overview]
Message-ID: <2eb3b1e7-931a-4a2c-92ac-4267a6a654ad@kernel.org> (raw)
In-Reply-To: <20251111061051.337547-1-hexiaole1994@126.com>

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,

  parent reply	other threads:[~2025-11-11  7:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-10 13:26 [PATCH v1] " 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 [this message]
2025-11-24 18:50       ` patchwork-bot+f2fs
2025-12-02 18:00       ` Jaegeuk Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2eb3b1e7-931a-4a2c-92ac-4267a6a654ad@kernel.org \
    --to=chao@kernel.org \
    --cc=hexiaole1994@126.com \
    --cc=jaegeuk@kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome