From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7C6E02D836D for ; Sun, 4 Jan 2026 05:42:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767505340; cv=none; b=Suw5FiaFC3R8/lWvWun3R7a5wxH200mJ553sLAxKkH64CSmQrV2tFh2zlGDi1P9SRi/62c5kmj+X0dqG0h3V17UDjhKlhoIcOomabCf+fvsA84SGH2HIPXSAk9J7JUCqcrLdYGoqUUHeHLOkVlh9eHKVd/FZuJCtV+AgUjRomuU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767505340; c=relaxed/simple; bh=dbYYlVRSYummE9dNEWBwEZzzqa3bv/tOU8J+QFEAUFs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=L1PQnpIFH3YxlFWxDfwQvf/Yx1Ah1Nb0KI705y2v7oSiX1dOJZm3xH8O5QTR6XVwmtshXw08QSvmSmrSvjXT1BUJ6DY1RUDWv6n/ctd5+jKkt/11cmEZnByTDuDeyc0hX20ruqtnOu330dhlen4TY13Uv4I9NmOYtPkYPiD94Sg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=N2I4BBM7; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="N2I4BBM7" Received: by smtp.kernel.org (Postfix) with ESMTPSA id D4C9AC4CEF7; Sun, 4 Jan 2026 05:42:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1767505340; bh=dbYYlVRSYummE9dNEWBwEZzzqa3bv/tOU8J+QFEAUFs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=N2I4BBM7t8mPhD19ELi1jSZPlCDWQiDn8HfRjFlZRdvsC5/5DLoQJ3gDf2ykxYg+A 1z1IJRXisw0Hk/05BE7wPPId9BeGwEQgoA6VJPo1Eue0LrU0cBR6YnHey4Z/2U9aJ+ 32ghv3XRdExg2LQI3VFoZWhC7ro+Va61TdSsxu/C7Ci6/WnEmdyqCdhHEarBIRAW/x ECMuJaMdE5unmkjyHdHNtq6LF7jdvaUYLkndaGtIOgeDH0yi0aJMihH6BWGmcGoky/ i9874wscMuyj0iuLqRdy6n0rhWYDwz80E2S3BZwkgco89Fdc9vd2Mpk2uHab2F2hZ6 pS0Zigw+K15Pw== Date: Sun, 4 Jan 2026 05:42:18 +0000 From: Jaegeuk Kim To: Chao Yu 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 Message-ID: References: <20260104020729.1064529-1-chao@kernel.org> <20260104020729.1064529-6-chao@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Should be like this? --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -2639,12 +2639,12 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi) .should_migrate_blocks = false, .err_gc_skipped = true, .no_bg_gc = true, - .nr_free_secs = 1 }; + .nr_free_secs = 1, + .lc }; - f2fs_down_write_trace(&sbi->gc_lock, &lc); + f2fs_down_write_trace(&sbi->gc_lock, &gc_control.lc); stat_inc_gc_call_count(sbi, FOREGROUND); err = f2fs_gc(sbi, &gc_control); - f2fs_up_write_trace(&sbi->gc_lock, &lc); if (err == -ENODATA) { err = 0; break; On 01/04, Jaegeuk Kim wrote: > 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 > > --- > > 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