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

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