Re: [PATCH] tracing/osnoise: Add option to align tlat threads

[Crystal Wood <crwood@redhat.com>]

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