Re: [f2fs-dev] [PATCH] f2fs: adjust iostat tracepoints

   Hi,

   Thank you for the review. We have decided not to upstream this patch,

    as the functionality is now implemented through a vendor hook and no 
F2FS core changes are needed.

   I will withdraw this patch. Thanks again for your time.

   Best regards,
   liujinbao1

在 2026/8/3 17:19, Chao Yu 写道:
> On 7/31/26 19:29, liujinbao1 wrote:
>> From: liujinbao1 <[email protected]>
>>
>> Move trace_f2fs_submit_read_bio() and trace_f2fs_submit_write_bio()
>
> Why? can you please explain more?
>
>> after iostat context initialization, and add trace_f2fs_iostat_update()
>> so f2fs iostat can be adjusted through tracepoints.
>>
>> Use ktime_get_ns() for more accurate iostat accounting.
>>
>> Signed-off-by: liujinbao1 <[email protected]>
>> ---
>>   fs/f2fs/data.c              |  4 ++--
>>   fs/f2fs/iostat.c            |  5 ++++-
>>   fs/f2fs/iostat.h            |  2 +-
>>   include/trace/events/f2fs.h | 32 ++++++++++++++++++++++++++++++++
>>   4 files changed, 39 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index a765fda71536..0b6aab302f02 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -568,9 +568,9 @@ void f2fs_submit_read_bio(struct f2fs_sb_info 
>> *sbi, struct bio *bio,
>>           return;
>>         WARN_ON_ONCE(!is_read_io(bio_op(bio)));
>> -    trace_f2fs_submit_read_bio(sbi->sb, type, bio);
>>         iostat_update_submit_ctx(bio, type);
>> +    trace_f2fs_submit_read_bio(sbi->sb, type, bio);
>>       blk_crypto_submit_bio(bio);
>>   }
>>   @@ -578,8 +578,8 @@ static void f2fs_submit_write_bio(struct 
>> f2fs_sb_info *sbi, struct bio *bio,
>>                     enum page_type type)
>>   {
>>       WARN_ON_ONCE(is_read_io(bio_op(bio)));
>> -    trace_f2fs_submit_write_bio(sbi->sb, type, bio);
>>       iostat_update_submit_ctx(bio, type);
>> +    trace_f2fs_submit_write_bio(sbi->sb, type, bio);
>>       blk_crypto_submit_bio(bio);
>>   }
>>   diff --git a/fs/f2fs/iostat.c b/fs/f2fs/iostat.c
>> index ae265e3e9b2c..73d810b3b7ff 100644
>> --- a/fs/f2fs/iostat.c
>> +++ b/fs/f2fs/iostat.c
>> @@ -257,7 +257,7 @@ static inline void __update_iostat_latency(struct 
>> bio_iostat_ctx *iostat_ctx,
>>       if (!sbi->iostat_enable)
>>           return;
>>   -    ts_diff = jiffies - iostat_ctx->submit_ts;
>> +    ts_diff = ktime_get_ns() - iostat_ctx->submit_ts;
>
> It missed to adjust below codes:
>
>             iostat_lat[idx][io].peak_lat =
>                jiffies_to_msecs(io_lat->peak_lat[idx][io]);
>
>                            ^^^^^^^^^^^^^^^^
>
>             iostat_lat[idx][io].cnt = io_lat->bio_cnt[idx][io];
>             iostat_lat[idx][io].avg_lat = iostat_lat[idx][io].cnt ?
>                jiffies_to_msecs(io_lat->sum_lat[idx][io]) / 
> iostat_lat[idx][io].cnt : 0;
>
>                            ^^^^^^^^^^^^^^^^
>
>>       if (page_type == META_FLUSH) {
>>           page_type = META;
>>       } else if (page_type >= NR_PAGE_TYPE) {
>> @@ -278,6 +278,9 @@ void iostat_update_and_unbind_ctx(struct bio *bio)
>>       struct bio_iostat_ctx *iostat_ctx = bio->bi_private;
>>       enum iostat_lat_type lat_type;
>>   +    trace_f2fs_iostat_update(iostat_ctx->sbi->sb, bio,
>> +                 iostat_ctx->submit_ts);
>> +
>>       if (op_is_write(bio_op(bio))) {
>>           lat_type = bio->bi_opf & REQ_SYNC ?
>>                   WRITE_SYNC_IO : WRITE_ASYNC_IO;
>> diff --git a/fs/f2fs/iostat.h b/fs/f2fs/iostat.h
>> index 2025225b5bed..2e3e1e4ce8b3 100644
>> --- a/fs/f2fs/iostat.h
>> +++ b/fs/f2fs/iostat.h
>> @@ -49,7 +49,7 @@ static inline void iostat_update_submit_ctx(struct 
>> bio *bio,
>>   {
>>       struct bio_iostat_ctx *iostat_ctx = bio->bi_private;
>>   -    iostat_ctx->submit_ts = jiffies;
>> +    iostat_ctx->submit_ts = ktime_get_ns();
>
> Hmm, it's in I/O path, can you please check how will this affect 
> performance?
>
>>       iostat_ctx->type = type;
>>   }
>>   diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
>> index 270c1a2c24c4..b97e0740d7ed 100644
>> --- a/include/trace/events/f2fs.h
>> +++ b/include/trace/events/f2fs.h
>> @@ -2315,6 +2315,38 @@ TRACE_EVENT(f2fs_iostat_latency,
>>           __entry->n_wr_as_peak, __entry->n_wr_as_avg, 
>> __entry->n_wr_as_cnt,
>>           __entry->m_wr_as_peak, __entry->m_wr_as_avg, 
>> __entry->m_wr_as_cnt)
>>   );
>> +
>> +TRACE_EVENT(f2fs_iostat_update,
>> +
>> +    TP_PROTO(struct super_block *sb, struct bio *bio,
>> +        unsigned long submit_ts),
>> +
>> +    TP_ARGS(sb, bio, submit_ts),
>> +
>> +    TP_STRUCT__entry(
>> +        __field(dev_t, dev)
>> +        __field(enum req_op, op)
>> +        __field(blk_opf_t, op_flags)
>> +        __field(sector_t, sector)
>> +        __field(unsigned int, size)
>> +        __field(unsigned long, submit_ts)
>> +    ),
>> +
>> +    TP_fast_assign(
>> +        __entry->dev = sb->s_dev;
>> +        __entry->op = bio_op(bio);
>> +        __entry->op_flags = bio->bi_opf;
>> +        __entry->sector = bio->bi_iter.bi_sector;
>> +        __entry->size = bio->bi_iter.bi_size;
>> +        __entry->submit_ts = submit_ts;
>> +    ),
>> +
>> +    TP_printk("dev=(%d,%d), rw = %s(%s), sector = %llu, size = %u",
>> +        show_dev(__entry->dev),
>> +        show_bio_type(__entry->op, __entry->op_flags),
>> +        (unsigned long long)__entry->sector,
>> +        __entry->size)
>
> Missed to print submit_ts?
>
> Thanks,
>
>> +);
>>   #endif
>>     TRACE_EVENT(f2fs_bmap,
>


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