On Mon, Nov 24, 2025 at 9:46 PM Crystal Wood <[email protected]> wrote:
>
> On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
>
> > +enum restart_result
> > +timerlat_restart(const struct osnoise_tool *tool, struct timerlat_params
> > *params)
> > +{
> > + actions_perform(¶ms->common.threshold_actions);
> > +
> > + if (!params->common.threshold_actions.continue_flag)
> > + /* continue flag not set, break */
> > + return RESTART_STOP;
> > +
> > + /* continue action reached, re-enable tracing */
> > + if (tool->record && trace_instance_start(&tool->record->trace))
> > + goto err;
> > + if (tool->aa && trace_instance_start(&tool->aa->trace))
> > + goto err;
> > + return RESTART_OK;
> > +
> > +err:
> > + err_msg("Error restarting trace\n");
> > + return RESTART_ERROR;
> > +}
>
> The non-BPF functions in common.c have the same logic and should also
> call this. This isn't timerlat-specific.
>
I will replace them here, thanks.
>
> > diff --git a/tools/tracing/rtla/src/timerlat_hist.c
> > b/tools/tracing/rtla/src/timerlat_hist.c
> > index 09a3da3f58630..f14fc56c5b4a5 100644
> > --- a/tools/tracing/rtla/src/timerlat_hist.c
> > +++ b/tools/tracing/rtla/src/timerlat_hist.c
> > @@ -1165,18 +1165,19 @@ static int timerlat_hist_bpf_main_loop(struct
> > osnoise_tool *tool)
> >
> > if (!stop_tracing) {
> > /* Threshold overflow, perform actions on threshold */
> > - actions_perform(¶ms->common.threshold_actions);
> > + enum restart_result result;
> >
> > - if (!params->common.threshold_actions.continue_flag)
> > - /* continue flag not set, break */
> > + result = timerlat_restart(tool, params);
> > + if (result == RESTART_STOP)
> > break;
> >
> > - /* continue action reached, re-enable tracing */
> > - if (tool->record)
> > - trace_instance_start(&tool->record->trace);
> > - if (tool->aa)
> > - trace_instance_start(&tool->aa->trace);
> > - timerlat_bpf_restart_tracing();
> > + if (result == RESTART_ERROR)
> > + return -1;
>
> Does it matter that we're not detaching on an error here? Is this
> something that gets cleaned up automatically (and if so, why do we ever
> need to do it explicitly)?
>
On process exit, it does.
> > +
> > + if (timerlat_bpf_restart_tracing()) {
> > + err_msg("Error restarting BPF trace\n");
> > + return -1;
> > + }
>
> [insert rant about not being able to use exceptions in userspace code in
> the year 2025]
>
I actually find exceptions an anti-pattern. Modern languages like Zig,
Go and Rust came back to error returning.
> -Crystal
>
