Re: [RFC PATCH v7 3/4] blk-iocost: add iocost_ioc_tick tracepoint for per-period device summary

[bot+bpf-ci]

> diff --git a/block/blk-iocost.c b/block/blk-iocost.c
> index 21e4f8cbd9f2..a48751b6dbdd 100644
> --- a/block/blk-iocost.c
> +++ b/block/blk-iocost.c

[ ... ]

> diff --git a/include/trace/events/iocost.h b/include/trace/events/iocost.h
> index e772b1bc60d6..32f19861a78f 100644
> --- a/include/trace/events/iocost.h
> +++ b/include/trace/events/iocost.h
> @@ -178,6 +178,51 @@ TRACE_EVENT(iocost_ioc_vrate_adj,
>  	)
>  );
>
> +/*
> + * Periodic per-device summary, emitted once per period from the tail of
> + * ioc_timer_fn().  Unlike the state-change events above, this fires every
> + * period the controller is running, including steady states, and carries
> + * the overall controller state so basic monitoring doesn't require drgn.
> + */
> +TRACE_EVENT(iocost_ioc_tick,
> +
> +	TP_PROTO(struct ioc *ioc, int nr_active, u64 usage_us_sum,
> +		 u32 tick_period_us, u64 tick_vrate,
> +		 int tick_busy, int tick_running, u64 tick_dur),
> +
> +	TP_ARGS(ioc, nr_active, usage_us_sum, tick_period_us, tick_vrate,
> +		tick_busy, tick_running, tick_dur),
> +
> +	TP_STRUCT__entry (
> +		__string(devname, ioc_name(ioc))
> +		__field(u64, cur_period)
> +		__field(u32, period_us)
> +		__field(u64, vrate)
> +		__field(int, busy_level)
> +		__field(int, nr_active)
> +		__field(u32, usage_pct)
> +		__field(int, running)
> +	),
> +
> +	TP_fast_assign(
> +		__assign_str(devname);
> +		__entry->cur_period = atomic64_read(&ioc->cur_period);
> +		__entry->period_us = tick_period_us;
> +		__entry->vrate = tick_vrate;
> +		__entry->busy_level = tick_busy;

This isn't a bug, but since the ioc pointer is already passed and
cur_period is read from it inside TP_fast_assign, could period_us, vrate
and busy_level also be read from ioc here?

The neighbouring iocost_ioc_vrate_adj event reads ioc->vtime_base_rate
and ioc->busy_level directly from the ioc pointer in its TP_fast_assign.
Reading them directly would make this event's prototype consistent with
that pattern.

> +		__entry->nr_active = nr_active;
> +		__entry->usage_pct = tick_dur ?
> +			div64_u64(usage_us_sum * 100, tick_dur) : 0;
> +		__entry->running = tick_running;
> +	),
> +
> +	TP_printk("[%s] period=%llu:%uus vrate=%llu busy=%d active=%d usage=%u%% running=%d",
> +		__get_str(devname), __entry->cur_period, __entry->period_us,
> +		__entry->vrate, __entry->busy_level, __entry->nr_active,
> +		__entry->usage_pct, __entry->running
> +	)
> +);
> +
>  TRACE_EVENT(iocost_iocg_forgive_debt,
>
>  	TP_PROTO(struct ioc_gq *iocg, const char *path, struct ioc_now *now,


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35962141400