From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 06/14] f2fs: trace elapsed time for gc_lock lock
Date: Sun, 4 Jan 2026 05:35:04 +0000 [thread overview]
Message-ID: <aVn8CHlqx9YxpijL@google.com> (raw)
In-Reply-To: <20260104020729.1064529-6-chao@kernel.org>
On 01/04, Chao Yu wrote:
> Use f2fs_{down,up}_write_trace for gc_lock to trace lock elapsed time.
>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> fs/f2fs/checkpoint.c | 10 ++++++----
> fs/f2fs/f2fs.h | 22 ++++++++++++----------
> fs/f2fs/file.c | 13 +++++++------
> fs/f2fs/gc.c | 23 +++++++++++++----------
> fs/f2fs/segment.c | 11 ++++++-----
> fs/f2fs/super.c | 15 +++++++++------
> include/trace/events/f2fs.h | 3 ++-
> 7 files changed, 55 insertions(+), 42 deletions(-)
>
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index dfd54cba1b35..da7bcfa2a178 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -1930,11 +1930,12 @@ void f2fs_destroy_checkpoint_caches(void)
> static int __write_checkpoint_sync(struct f2fs_sb_info *sbi)
> {
> struct cp_control cpc = { .reason = CP_SYNC, };
> + struct f2fs_lock_context lc;
> int err;
>
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &lc);
> err = f2fs_write_checkpoint(sbi, &cpc);
> - f2fs_up_write(&sbi->gc_lock);
> + f2fs_up_write_trace(&sbi->gc_lock, &lc);
>
> return err;
> }
> @@ -2022,11 +2023,12 @@ int f2fs_issue_checkpoint(struct f2fs_sb_info *sbi)
> cpc.reason = __get_cp_reason(sbi);
> if (!test_opt(sbi, MERGE_CHECKPOINT) || cpc.reason != CP_SYNC ||
> sbi->umount_lock_holder == current) {
> + struct f2fs_lock_context lc;
> int ret;
>
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &lc);
> ret = f2fs_write_checkpoint(sbi, &cpc);
> - f2fs_up_write(&sbi->gc_lock);
> + f2fs_up_write_trace(&sbi->gc_lock, &lc);
>
> return ret;
> }
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 3f6278ba620d..5b6e632b37a9 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -178,6 +178,7 @@ enum f2fs_lock_name {
> LOCK_NAME_CP_RWSEM,
> LOCK_NAME_NODE_CHANGE,
> LOCK_NAME_NODE_WRITE,
> + LOCK_NAME_GC_LOCK,
> };
>
> /*
> @@ -1408,16 +1409,6 @@ struct atgc_management {
> unsigned long long age_threshold; /* age threshold */
> };
>
> -struct f2fs_gc_control {
> - unsigned int victim_segno; /* target victim segment number */
> - int init_gc_type; /* FG_GC or BG_GC */
> - bool no_bg_gc; /* check the space and stop bg_gc */
> - bool should_migrate_blocks; /* should migrate blocks */
> - bool err_gc_skipped; /* return EAGAIN if GC skipped */
> - bool one_time; /* require one time GC in one migration unit */
> - unsigned int nr_free_secs; /* # of free sections to do GC */
> -};
> -
> struct f2fs_time_stat {
> unsigned long long total_time; /* total wall clock time */
> #ifdef CONFIG_64BIT
> @@ -1436,6 +1427,17 @@ struct f2fs_lock_context {
> bool lock_trace;
> };
>
> +struct f2fs_gc_control {
> + unsigned int victim_segno; /* target victim segment number */
> + int init_gc_type; /* FG_GC or BG_GC */
> + bool no_bg_gc; /* check the space and stop bg_gc */
> + bool should_migrate_blocks; /* should migrate blocks */
> + bool err_gc_skipped; /* return EAGAIN if GC skipped */
> + bool one_time; /* require one time GC in one migration unit */
> + unsigned int nr_free_secs; /* # of free sections to do GC */
> + struct f2fs_lock_context lc; /* lock context for gc_lock */
> +};
> +
> /*
> * For s_flag in struct f2fs_sb_info
> * Modification on enum should be synchronized with s_flag array
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 1cdbbc2e1005..ce291f152bc3 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1928,7 +1928,7 @@ static int f2fs_expand_inode_data(struct inode *inode, loff_t offset,
>
> if (has_not_enough_free_secs(sbi, 0,
> sbi->reserved_pin_section)) {
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &gc_control.lc);
> stat_inc_gc_call_count(sbi, FOREGROUND);
> err = f2fs_gc(sbi, &gc_control);
> if (err && err != -ENODATA) {
> @@ -2779,12 +2779,13 @@ static int f2fs_ioc_gc(struct file *filp, unsigned long arg)
> return ret;
>
> if (!sync) {
> - if (!f2fs_down_write_trylock(&sbi->gc_lock)) {
> + if (!f2fs_down_write_trylock_trace(&sbi->gc_lock,
> + &gc_control.lc)) {
> ret = -EBUSY;
> goto out;
> }
> } else {
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &gc_control.lc);
> }
>
> gc_control.init_gc_type = sync ? FG_GC : BG_GC;
> @@ -2824,12 +2825,12 @@ static int __f2fs_ioc_gc_range(struct file *filp, struct f2fs_gc_range *range)
>
> do_more:
> if (!range->sync) {
> - if (!f2fs_down_write_trylock(&sbi->gc_lock)) {
> + if (!f2fs_down_write_trylock_trace(&sbi->gc_lock, &gc_control.lc)) {
> ret = -EBUSY;
> goto out;
> }
> } else {
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &gc_control.lc);
> }
>
> gc_control.victim_segno = GET_SEGNO(sbi, range->start);
> @@ -3320,7 +3321,7 @@ static int f2fs_ioc_flush_device(struct file *filp, unsigned long arg)
> end_segno = min(start_segno + range.segments, dev_end_segno);
>
> while (start_segno < end_segno) {
> - if (!f2fs_down_write_trylock(&sbi->gc_lock)) {
> + if (!f2fs_down_write_trylock_trace(&sbi->gc_lock, &gc_control.lc)) {
> ret = -EBUSY;
> goto out;
> }
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 8999829a9559..391e66064c7e 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -102,21 +102,22 @@ static int gc_thread_func(void *data)
> if (sbi->gc_mode == GC_URGENT_HIGH ||
> sbi->gc_mode == GC_URGENT_MID) {
> wait_ms = gc_th->urgent_sleep_time;
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &gc_control.lc);
> goto do_gc;
> }
>
> if (foreground) {
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &gc_control.lc);
> goto do_gc;
> - } else if (!f2fs_down_write_trylock(&sbi->gc_lock)) {
> + } else if (!f2fs_down_write_trylock_trace(&sbi->gc_lock,
> + &gc_control.lc)) {
> stat_other_skip_bggc_count(sbi);
> goto next;
> }
>
> if (!is_idle(sbi, GC_TIME)) {
> increase_sleep_time(gc_th, &wait_ms);
> - f2fs_up_write(&sbi->gc_lock);
> + f2fs_up_write_trace(&sbi->gc_lock, &gc_control.lc);
> stat_io_skip_bggc_count(sbi);
> goto next;
> }
> @@ -125,7 +126,8 @@ static int gc_thread_func(void *data)
> if (has_enough_free_blocks(sbi,
> gc_th->no_zoned_gc_percent)) {
> wait_ms = gc_th->no_gc_sleep_time;
> - f2fs_up_write(&sbi->gc_lock);
> + f2fs_up_write_trace(&sbi->gc_lock,
> + &gc_control.lc);
> goto next;
> }
> if (wait_ms == gc_th->no_gc_sleep_time)
> @@ -2046,7 +2048,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control)
> reserved_segments(sbi),
> prefree_segments(sbi));
>
> - f2fs_up_write(&sbi->gc_lock);
> + f2fs_up_write_trace(&sbi->gc_lock, &gc_control->lc);
>
> put_gc_inode(&gc_list);
>
> @@ -2264,6 +2266,7 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count)
> __u64 old_block_count, shrunk_blocks;
> struct cp_control cpc = { CP_RESIZE, 0, 0, 0 };
> struct f2fs_lock_context lc;
> + struct f2fs_lock_context glc;
> unsigned int secs;
> int err = 0;
> __u32 rem;
> @@ -2307,7 +2310,7 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count)
> secs = div_u64(shrunk_blocks, BLKS_PER_SEC(sbi));
>
> /* stop other GC */
> - if (!f2fs_down_write_trylock(&sbi->gc_lock)) {
> + if (!f2fs_down_write_trylock_trace(&sbi->gc_lock, &glc)) {
> err = -EAGAIN;
> goto out_drop_write;
> }
> @@ -2329,7 +2332,7 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count)
>
> out_unlock:
> f2fs_unlock_op(sbi, &lc);
> - f2fs_up_write(&sbi->gc_lock);
> + f2fs_up_write_trace(&sbi->gc_lock, &glc);
> out_drop_write:
> mnt_drop_write_file(filp);
> if (err)
> @@ -2346,7 +2349,7 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count)
> return -EROFS;
> }
>
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &glc);
> f2fs_down_write(&sbi->cp_global_sem);
>
> spin_lock(&sbi->stat_lock);
> @@ -2396,7 +2399,7 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count)
> }
> out_err:
> f2fs_up_write(&sbi->cp_global_sem);
> - f2fs_up_write(&sbi->gc_lock);
> + f2fs_up_write_trace(&sbi->gc_lock, &glc);
> thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL);
> return err;
> }
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index e4a8daf433a8..776b0df828ed 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -462,7 +462,7 @@ void f2fs_balance_fs(struct f2fs_sb_info *sbi, bool need)
> .should_migrate_blocks = false,
> .err_gc_skipped = false,
> .nr_free_secs = 1 };
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &gc_control.lc);
> stat_inc_gc_call_count(sbi, FOREGROUND);
> f2fs_gc(sbi, &gc_control);
> }
> @@ -3373,10 +3373,10 @@ int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi)
> f2fs_unlock_op(sbi, &lc);
>
> if (f2fs_sb_has_blkzoned(sbi) && err == -EAGAIN && gc_required) {
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &lc);
> err = f2fs_gc_range(sbi, 0, sbi->first_seq_zone_segno - 1,
> true, ZONED_PIN_SEC_REQUIRED_COUNT);
> - f2fs_up_write(&sbi->gc_lock);
> + f2fs_up_write_trace(&sbi->gc_lock, &lc);
>
> gc_required = false;
> if (!err)
> @@ -3496,6 +3496,7 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
> block_t start_block, end_block;
> struct cp_control cpc;
> struct discard_policy dpolicy;
> + struct f2fs_lock_context lc;
> unsigned long long trimmed = 0;
> int err = 0;
> bool need_align = f2fs_lfs_mode(sbi) && __is_large_section(sbi);
> @@ -3528,10 +3529,10 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
> if (sbi->discard_blks == 0)
> goto out;
>
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &lc);
> stat_inc_cp_call_count(sbi, TOTAL_CALL);
> err = f2fs_write_checkpoint(sbi, &cpc);
> - f2fs_up_write(&sbi->gc_lock);
> + f2fs_up_write_trace(&sbi->gc_lock, &lc);
> if (err)
> goto out;
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index d8e5e8652d97..abb468eb4394 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -2563,6 +2563,7 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
> int err = 0;
> int ret;
> block_t unusable;
> + struct f2fs_lock_context lc;
>
> if (s_flags & SB_RDONLY) {
> f2fs_err(sbi, "checkpoint=disable on readonly fs");
> @@ -2588,9 +2589,10 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
> .no_bg_gc = true,
> .nr_free_secs = 1 };
>
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &lc);
> stat_inc_gc_call_count(sbi, FOREGROUND);
> err = f2fs_gc(sbi, &gc_control);
> + f2fs_up_write_trace(&sbi->gc_lock, &lc);
^--- this looks wrong?
> if (err == -ENODATA) {
> err = 0;
> break;
> @@ -2612,7 +2614,7 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
> }
>
> skip_gc:
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &lc);
> cpc.reason = CP_PAUSE;
> set_sbi_flag(sbi, SBI_CP_DISABLED);
> stat_inc_cp_call_count(sbi, TOTAL_CALL);
> @@ -2625,7 +2627,7 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
> spin_unlock(&sbi->stat_lock);
>
> out_unlock:
> - f2fs_up_write(&sbi->gc_lock);
> + f2fs_up_write_trace(&sbi->gc_lock, &lc);
> restore_flag:
> sbi->gc_mode = gc_mode;
> sbi->sb->s_flags = s_flags; /* Restore SB_RDONLY status */
> @@ -2638,6 +2640,7 @@ static int f2fs_enable_checkpoint(struct f2fs_sb_info *sbi)
> unsigned int nr_pages = get_pages(sbi, F2FS_DIRTY_DATA) / 16;
> long long start, writeback, lock, sync_inode, end;
> int ret;
> + struct f2fs_lock_context lc;
>
> f2fs_info(sbi, "%s start, meta: %lld, node: %lld, data: %lld",
> __func__,
> @@ -2672,12 +2675,12 @@ static int f2fs_enable_checkpoint(struct f2fs_sb_info *sbi)
>
> sync_inode = ktime_get();
>
> - f2fs_down_write(&sbi->gc_lock);
> + f2fs_down_write_trace(&sbi->gc_lock, &lc);
> f2fs_dirty_to_prefree(sbi);
>
> clear_sbi_flag(sbi, SBI_CP_DISABLED);
> set_sbi_flag(sbi, SBI_IS_DIRTY);
> - f2fs_up_write(&sbi->gc_lock);
> + f2fs_up_write_trace(&sbi->gc_lock, &lc);
>
> f2fs_info(sbi, "%s sync_fs, meta: %lld, imeta: %lld, node: %lld, dents: %lld, qdata: %lld",
> __func__,
> @@ -4893,7 +4896,7 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc)
> sbi->sb = sb;
>
> /* initialize locks within allocated memory */
> - init_f2fs_rwsem(&sbi->gc_lock);
> + init_f2fs_rwsem_trace(&sbi->gc_lock, sbi, LOCK_NAME_GC_LOCK);
> mutex_init(&sbi->writepages);
> init_f2fs_rwsem(&sbi->cp_global_sem);
> init_f2fs_rwsem_trace(&sbi->node_write, sbi, LOCK_NAME_NODE_WRITE);
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index e5cfb8ad0d5e..bf353e7e024d 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -188,7 +188,8 @@ TRACE_DEFINE_ENUM(CP_PHASE_FINISH_CHECKPOINT);
> __print_symbolic(lock, \
> { LOCK_NAME_CP_RWSEM, "cp_rwsem" }, \
> { LOCK_NAME_NODE_CHANGE, "node_change" }, \
> - { LOCK_NAME_NODE_WRITE, "node_write" })
> + { LOCK_NAME_NODE_WRITE, "node_write" }, \
> + { LOCK_NAME_GC_LOCK, "gc_lock" })
>
> struct f2fs_sb_info;
> struct f2fs_io_info;
> --
> 2.49.0
next prev parent reply other threads:[~2026-01-04 5:35 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-04 2:07 [PATCH 01/14] f2fs: add lock elapsed time trace facility for f2fs rwsemphore Chao Yu
2026-01-04 2:07 ` [PATCH 02/14] f2fs: sysfs: introduce max_lock_elapsed_time Chao Yu
2026-01-04 2:07 ` [PATCH 03/14] f2fs: trace elapsed time for cp_rwsem lock Chao Yu
2026-01-04 2:07 ` [PATCH 04/14] f2fs: trace elapsed time for node_change lock Chao Yu
2026-01-04 2:07 ` [PATCH 05/14] f2fs: trace elapsed time for node_write lock Chao Yu
2026-01-04 2:07 ` [PATCH 06/14] f2fs: trace elapsed time for gc_lock lock Chao Yu
2026-01-04 5:35 ` Jaegeuk Kim [this message]
2026-01-04 5:42 ` Jaegeuk Kim
2026-01-04 6:27 ` Chao Yu
2026-01-04 2:07 ` [PATCH 07/14] f2fs: trace elapsed time for cp_global_sem lock Chao Yu
2026-01-04 2:07 ` [PATCH 08/14] f2fs: trace elapsed time for io_rwsem lock Chao Yu
2026-01-04 2:07 ` [PATCH 09/14] f2fs: clean up w/ __f2fs_schedule_timeout() Chao Yu
2026-01-04 2:07 ` [PATCH 10/14] f2fs: fix to use jiffies based precision for DEFAULT_SCHEDULE_TIMEOUT Chao Yu
2026-01-04 2:07 ` [PATCH 11/14] f2fs: fix timeout precision of f2fs_io_schedule_timeout_killable() Chao Yu
2026-01-04 2:07 ` [PATCH 12/14] f2fs: rename FAULT_TIMEOUT to FAULT_ATOMIC_TIMEOUT Chao Yu
2026-01-04 2:07 ` [PATCH 13/14] f2fs: introduce FAULT_LOCK_TIMEOUT Chao Yu
2026-01-04 2:07 ` [PATCH 14/14] f2fs: sysfs: introduce inject_lock_timeout Chao Yu
2026-01-04 5:28 ` [PATCH 01/14] f2fs: add lock elapsed time trace facility for f2fs rwsemphore Jaegeuk Kim
2026-01-07 3:30 ` [f2fs-dev] " patchwork-bot+f2fs
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=aVn8CHlqx9YxpijL@google.com \
--to=jaegeuk@kernel.org \
--cc=chao@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.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