Re: [rtla 07/13] rtla: Introduce timerlat_restart() helper

[Crystal Wood]

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(&params->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.


> 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(&params->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)?

> +
> +                     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]

-Crystal