On Fri, 2026-02-27 at 16:04 +0100, Tomas Glozar wrote:
> Add an option called TIMERLAT_ALIGN to osnoise/options, together with a
> corresponding setting osnoise/timerlat_align_us.
>
> This option sets the alignment of wakeup times between different
> timerlat threads, similarly to cyclictest's -A/--aligned option. If
> TIMERLAT_ALIGN is set, the first thread that reaches the first cycle
> records its first wake-up time. Each following thread sets its first
> wake-up time to a fixed offset from the recorded time, and incremenets
> it by the same offset.
Why not just set the initial timer expiration to be
"period + cpu * align_us"? Then you wouldn't need any interaction
between CPUs.
> kernel/trace/trace_osnoise.c | 34 +++++++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
Documentation needs to be updated as well.
Should mention that updating align_us while the timer is running won't
take effect immediately (unlike period, which does).
> diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
> index dee610e465b9..df1d4529d226 100644
> --- a/kernel/trace/trace_osnoise.c
> +++ b/kernel/trace/trace_osnoise.c
> @@ -58,6 +58,7 @@ enum osnoise_options_index {
> OSN_PANIC_ON_STOP,
> OSN_PREEMPT_DISABLE,
> OSN_IRQ_DISABLE,
> + OSN_TIMERLAT_ALIGN,
> OSN_MAX
> };
>
> @@ -66,7 +67,8 @@ static const char * const osnoise_options_str[OSN_MAX] = {
> "OSNOISE_WORKLOAD",
> "PANIC_ON_STOP",
>
> "OSNOISE_PREEMPT_DISABLE",
> - "OSNOISE_IRQ_DISABLE" };
> + "OSNOISE_IRQ_DISABLE",
> + "TIMERLAT_ALIGN" };
Do we really need a flag for this, or can we just interpret a non-zero
align_us value as enabling the feature?
> @@ -1820,6 +1824,7 @@ static int wait_next_period(struct timerlat_variables
> *tlat)
> {
> ktime_t next_abs_period, now;
> u64 rel_period = osnoise_data.timerlat_period * 1000;
> + static atomic64_t align_next;
How will this get reset if the tracer is stopped and restarted?
> now = hrtimer_cb_get_time(&tlat->timer);
> next_abs_period = ns_to_ktime(tlat->abs_period + rel_period);
> @@ -1829,6 +1834,17 @@ static int wait_next_period(struct timerlat_variables
> *tlat)
> */
> tlat->abs_period = (u64) ktime_to_ns(next_abs_period);
>
> + if (test_bit(OSN_TIMERLAT_ALIGN, &osnoise_options) && !tlat->count
> + && atomic64_cmpxchg_relaxed(&align_next, 0, tlat->abs_period)) {
> + /*
> + * Align thread in first cycle on each CPU to the set alignment.
> + */
> + tlat->abs_period =
> atomic64_fetch_add_relaxed(osnoise_data.timerlat_align_us * 1000,
> + &align_next);
> + tlat->abs_period += osnoise_data.timerlat_align_us * 1000;
> + next_abs_period = ns_to_ktime(tlat->abs_period);
> + }
I'm already unclear about the existing purpose of next_abs_period, but
if it has any use at all shouldn't it be to avoid writing intermediate
values like this back to tlat?
-Crystal
