Re: [PATCH] rtla: Simplify osnoise tracer option setting code

[Tomas Glozar]

On Wed, Aug 19, 2026 at 12:31 AM Crystal Wood <crwood@xxxxxxxxxx> wrote:
> >
> > They are called "osnoise options" in the interface (although they are
> > shared with the timerlat tracer), which cannot be changed. I don't
> > like using an esoteric prefix like "osn".
>
> I'm not suggesting changing the kernel interface, just having some
> convention within rtla (and within trace_osnoise.c) to make it clearer
> which things apply to osnoise-the-subsystem versus osnoise-the-specific-
> tracer.  It might be "esoteric" which one gets the abbreviation, but as
> long as it's consistent I think it'd be an improvement over the current
> jumble.
>

After giving it some thought, I believe the best approach is to use
the proper tracer name (osnoise/timerlat), with "common" for shared
options, regardless of the tracefs file names.

For example, "osnoise_set_timerlat_period()" should be just
"timerlat_set_period()", and "osnoise_get_cpus()" should be
"common_get_cpus()". Implementing this with the macro lists is a bit
tricky, though, so I'm leaving that for a future patch.

> >
> > > > + * Long long option get/set/restore/put functions, generated from OSNOISE_LL_OPTIONS.
> > > > + */
> > > > +#define OSNOISE_LL_OPTION(name, path, init_val)                                              \
> > > > +static long long                                                                     \
> > > > +osnoise_get_##name(struct osnoise_context *context)                                  \
> > > > +{                                                                                    \
> > > > +     long long name;                                                                 \
> > > > +                                                                                     \
> > > > +     if (context->name != (init_val))                                                \
> > > > +             return context->name;                                                   \
> > > > +                                                                                     \
> > > > +     if (context->orig_##name != (init_val))                                         \
> > > > +             return context->orig_##name;                                            \
> > > > +                                                                                     \
> > > > +     name = osnoise_read_ll_config(path);                                            \
> > > > +     if (name < 0)                                                                   \
> > > > +             return (init_val);                                                      \
> > > > +                                                                                     \
> > > > +     context->orig_##name = name;                                                    \
> > > > +     return name;                                                                    \
> > > > +}                                                                                    \
> > > > +                                                                                     \
> > > > +int osnoise_set_##name(struct osnoise_context *context, long long name)                      \
> > > > +{                                                                                    \
> > > > +     long long curr = osnoise_get_##name(context);                                   \
> > > > +     int retval;                                                                     \
> > > > +                                                                                     \
> > > > +     if (curr == (init_val))                                                         \
> > > > +             return -1;                                                              \
> > > > +                                                                                     \
> > > > +     retval = osnoise_write_ll_config(path, name);                                   \
> > > > +     if (retval < 0)                                                                 \
> > > > +             return -2;                                                              \
> > > > +                                                                                     \
> > > > +     context->name = name;                                                           \
> > > > +     return 0;                                                                       \
> > > > +}                                                                                    \
> > >
> > > Using "name" for the value is confusing... "val" would be better.
> > >
> >
> > But it's the *name* of the option/field/argument here, not the value.
> > If you use "value" you'll get:
> >
> > #define OSNOISE_LL_OPTION(value, path, init_val)
> >
> > which is incorrect. Would making the macro options in capital letters
> > (i.e. NAME) make it more clear?
>
> That's not what I meant.  I was suggesting this:
>
> int osnoise_set_##name(struct osnoise_context *context, long long val)
>
> -Crystal

Ah, now I get it. I fixed this in the v3 of the patch [1].

[1] https://lore.kernel.org/linux-trace-kernel/20260904085500.488905-1-tglozar@xxxxxxxxxx/

Tomas