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 <[email protected]>
> > ---
> >  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


_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to