On Wed, 2025-08-27 at 15:34 +0200, Tomas Glozar wrote:
> čt 21. 8. 2025 v 5:58 odesílatel Crystal Wood <crw...@redhat.com> napsal:
> >
> > Some bits like aa_only and the user stuff are in common even though they
> > are only used by timerlat. In the case of user stuff I plan to submit
> > patches for supporting that on osnoise, and it doesn't seem worthwhile to
> > jump through hoops to move some of that code out of code that is otherwise
> > able to be shared. For aa_only, making it non-common would have precluded
> > sharing some code, and the code that the flag inhibits isn't tool-specific
> > (unlike no_aa). Plus, there's other common code referring to tool->aa.
> > ---
>
> aa_only is implemented only for top, because the behavior would be the
> same for hist: just do auto-analysis, no histogram, no top. Of course,
> it doesn't hurt that it is included as a common parameter.
>
> > + stopped = osnoise_trace_is_off(tool, tool->record) && !stop_tracing;
> > + if (stopped) {
> > + printf("rtla hit stop tracing\n");
> > + return_value = FAILED;
> > + }
> > +
>
> Is the output change necessary? The original "osnoise hit stop
> tracing"/"timerlat hit stop tracing" refers to the osnoise and
> timerlat tracers, which stop tracing on threshold (unless mode is
> TRACING_MODE_BPF
> ). We already have ops->tracer for the tracer name, that can be used here.
Certainly not necessary, but it didn't seem like an important
distinction to maintain -- unless there are scripts out there (beyond
our test cases) checking for it that we don't want to break. Though
for not-yet-written scripts I'd imagine having to deal with different
names could be an annoyance as well.
> > /*
> > * timerlat_top_bpf_main_loop - main loop to process events (BPF variant)
> > */
> > static int
> > -timerlat_top_bpf_main_loop(struct osnoise_tool *top,
> > - struct osnoise_tool *record,
> > - struct osnoise_tool *aa,
> > - struct timerlat_params *params,
> > - struct timerlat_u_params *params_u)
> > +__timerlat_top_bpf_main_loop(struct osnoise_tool *tool)
>
> The naming here should be consistent with the comment and with
> timerlat-hist, that is, without the __ prefix, there is no other
> timerlat_top_bpf_main_loop that would create a conflict in this
> version of the patch.
Oops... I think there used to be a non-underscore version at some point
during development. Will fix.
-Crystal
