linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rtla/osnoise: Better report when histogram is empty
@ 2024-06-12 10:36 Luis Claudio R. Goncalves
  2024-06-12 14:12 ` John Kacur
  0 siblings, 1 reply; 3+ messages in thread
From: Luis Claudio R. Goncalves @ 2024-06-12 10:36 UTC (permalink / raw)
  To: Daniel Bristot de Oliveira, Steven Rostedt, John Kacur,
	Clark Williams, linux-trace-kernel, linux-kernel

When osnoise hist does not observe any samples above the threshold,
no entries are recorded and the final report shows empty entries
for the usual statistics (count, min, max, avg):

    [~]# osnoise hist -d 5s -T 500
    # RTLA osnoise histogram
    # Time unit is microseconds (us)
    # Duration:   0 00:00:05
    Index
    over: 
    count:
    min:  
    avg:  
    max:  

That could lead users to confusing interpretations of the results.

A simple solution is to report 0 for count and the statistics, making it
clear that no noise (above the defined threshold) was observed:

    [~]# osnoise hist -d 5s -T 500
    # RTLA osnoise histogram
    # Time unit is microseconds (us)
    # Duration:   0 00:00:05
    Index
    over: 0
    count: 0
    min: 0
    avg: 0
    max: 0


Signed-off-by: Luis Claudio R. Goncalves <lgoncalv@redhat.com>
---
 tools/tracing/rtla/src/osnoise_hist.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c
index 7be17d09f7e85..214e2c93fde01 100644
--- a/tools/tracing/rtla/src/osnoise_hist.c
+++ b/tools/tracing/rtla/src/osnoise_hist.c
@@ -374,6 +374,7 @@ osnoise_print_stats(struct osnoise_hist_params *params, struct osnoise_tool *too
 {
 	struct osnoise_hist_data *data = tool->data;
 	struct trace_instance *trace = &tool->trace;
+	int has_samples = 0;
 	int bucket, cpu;
 	int total;
 
@@ -402,11 +403,25 @@ osnoise_print_stats(struct osnoise_hist_params *params, struct osnoise_tool *too
 			continue;
 		}
 
+		/* There are samples above the threshold */
+		has_samples = 1;
 		trace_seq_printf(trace->seq, "\n");
 		trace_seq_do_printf(trace->seq);
 		trace_seq_reset(trace->seq);
 	}
 
+	/*
+	 * If no samples were recorded, skip calculations, print zeroed statistics
+	 * and return.
+	 */
+	if (!has_samples) {
+		trace_seq_reset(trace->seq);
+		trace_seq_printf(trace->seq, "over: 0\ncount: 0\nmin: 0\navg: 0\nmax: 0\n");
+		trace_seq_do_printf(trace->seq);
+		trace_seq_reset(trace->seq);
+		return;
+	}
+
 	if (!params->no_index)
 		trace_seq_printf(trace->seq, "over: ");
 


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] rtla/osnoise: Better report when histogram is empty
  2024-06-12 10:36 [PATCH] rtla/osnoise: Better report when histogram is empty Luis Claudio R. Goncalves
@ 2024-06-12 14:12 ` John Kacur
  2024-06-21  8:26   ` Daniel Bristot de Oliveira
  0 siblings, 1 reply; 3+ messages in thread
From: John Kacur @ 2024-06-12 14:12 UTC (permalink / raw)
  To: Luis Claudio R. Goncalves
  Cc: Daniel Bristot de Oliveira, Steven Rostedt, Clark Williams,
	linux-trace-kernel, linux-kernel



On Wed, 12 Jun 2024, Luis Claudio R. Goncalves wrote:

> When osnoise hist does not observe any samples above the threshold,
> no entries are recorded and the final report shows empty entries
> for the usual statistics (count, min, max, avg):
> 
>     [~]# osnoise hist -d 5s -T 500
>     # RTLA osnoise histogram
>     # Time unit is microseconds (us)
>     # Duration:   0 00:00:05
>     Index
>     over: 
>     count:
>     min:  
>     avg:  
>     max:  
> 
> That could lead users to confusing interpretations of the results.
> 
> A simple solution is to report 0 for count and the statistics, making it
> clear that no noise (above the defined threshold) was observed:
> 
>     [~]# osnoise hist -d 5s -T 500
>     # RTLA osnoise histogram
>     # Time unit is microseconds (us)
>     # Duration:   0 00:00:05
>     Index
>     over: 0
>     count: 0
>     min: 0
>     avg: 0
>     max: 0
> 
> 
> Signed-off-by: Luis Claudio R. Goncalves <lgoncalv@redhat.com>
> ---
>  tools/tracing/rtla/src/osnoise_hist.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c
> index 7be17d09f7e85..214e2c93fde01 100644
> --- a/tools/tracing/rtla/src/osnoise_hist.c
> +++ b/tools/tracing/rtla/src/osnoise_hist.c
> @@ -374,6 +374,7 @@ osnoise_print_stats(struct osnoise_hist_params *params, struct osnoise_tool *too
>  {
>  	struct osnoise_hist_data *data = tool->data;
>  	struct trace_instance *trace = &tool->trace;
> +	int has_samples = 0;
>  	int bucket, cpu;
>  	int total;
>  
> @@ -402,11 +403,25 @@ osnoise_print_stats(struct osnoise_hist_params *params, struct osnoise_tool *too
>  			continue;
>  		}
>  
> +		/* There are samples above the threshold */
IMHO The comment isn't needed because the variable had_samples is 
descriptive, but it's not a big deal either


> +		has_samples = 1;
>  		trace_seq_printf(trace->seq, "\n");
>  		trace_seq_do_printf(trace->seq);
>  		trace_seq_reset(trace->seq);
>  	}
>  
> +	/*
> +	 * If no samples were recorded, skip calculations, print zeroed statistics
> +	 * and return.
> +	 */
> +	if (!has_samples) {
> +		trace_seq_reset(trace->seq);
> +		trace_seq_printf(trace->seq, "over: 0\ncount: 0\nmin: 0\navg: 0\nmax: 0\n");
> +		trace_seq_do_printf(trace->seq);
> +		trace_seq_reset(trace->seq);
> +		return;
> +	}
> +
>  	if (!params->no_index)
>  		trace_seq_printf(trace->seq, "over: ");
>  
> 
> 
> 


Reviewed-by: John Kacur <jkacur@redhat.com>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] rtla/osnoise: Better report when histogram is empty
  2024-06-12 14:12 ` John Kacur
@ 2024-06-21  8:26   ` Daniel Bristot de Oliveira
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Bristot de Oliveira @ 2024-06-21  8:26 UTC (permalink / raw)
  To: John Kacur, Luis Claudio R. Goncalves
  Cc: Steven Rostedt, Clark Williams, linux-trace-kernel, linux-kernel

On 6/12/24 16:12, John Kacur wrote:
>> +		/* There are samples above the threshold */
> IMHO The comment isn't needed because the variable had_samples is 
> descriptive, but it's not a big deal either

Yeah, I would not do it too, but it is fine.

I am adding it to my queue.

Thanks Luis and John!
-- Daniel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-06-21  8:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-12 10:36 [PATCH] rtla/osnoise: Better report when histogram is empty Luis Claudio R. Goncalves
2024-06-12 14:12 ` John Kacur
2024-06-21  8:26   ` Daniel Bristot de Oliveira

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).