On Wed, Aug 19, 2026 at 12:31 AM Crystal Wood <[email protected]> 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/[email protected]/
Tomas
