linux-kernel - Re: [rtla 07/13] rtla: Introduce timerlat

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <fb5b468b38ac9570a5f3fb948452d1b5b03c9f9c.camel@redhat.com>
Date: Mon, 24 Nov 2025 18:46:36 -0600
From: Crystal Wood <crwood@...hat.com>
To: Wander Lairson Costa <wander@...hat.com>, Steven Rostedt	
 <rostedt@...dmis.org>, Tomas Glozar <tglozar@...hat.com>, Ivan Pravdin	
 <ipravdin.official@...il.com>, John Kacur <jkacur@...hat.com>, Costa
 Shulyupin	 <costa.shul@...hat.com>, Tiezhu Yang <yangtiezhu@...ngson.cn>,
 "open list:Real-time Linux Analysis (RTLA) tools"	
 <linux-trace-kernel@...r.kernel.org>, open list
 <linux-kernel@...r.kernel.org>,  "open list:BPF
 [MISC]:Keyword:(?:\\b|_)bpf(?:\\b|_)"	 <bpf@...r.kernel.org>
Subject: Re: [rtla 07/13] rtla: Introduce timerlat_restart() helper

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