Skip to content

Commit

Permalink
rtla/timerlat: Fix histogram ALL for zero samples
Browse files Browse the repository at this point in the history 
commit 6cc45f8 upstream.

rtla timerlat hist currently computers the minimum, maximum and average
latency even in cases when there are zero samples. This leads to
nonsensical values being calculated for maximum and minimum, and to
divide by zero for average.

A similar bug is fixed by 01b05fc ("rtla/timerlat: Fix histogram
report when a cpu count is 0") but the bug still remains for printing
the sum over all CPUs in timerlat_print_stats_all.

The issue can be reproduced with this command:

$ rtla timerlat hist -U -d 1s
Index
over:
count:
min:
avg:
max:
Floating point exception (core dumped)

(There are always no samples with -U unless the user workload is
created.)

Fix the bug by omitting max/min/avg when sample count is zero,
displaying a dash instead, just like we already do for the individual
CPUs. The logic is moved into a new function called
format_summary_value, which is used for both the individual CPUs
and for the overall summary.

Cc: [email protected]
Link: https://lore.kernel.org/[email protected]
Fixes: 1462501 ("rtla/timerlat: Add a summary for hist mode")
Signed-off-by: Tomas Glozar <[email protected]>
Signed-off-by: Steven Rostedt (Google) <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
  • Loading branch information
@lenticularis39 @gregkh
lenticularis39 authored and gregkh committed Jan 2, 2025
1 parent 1cca920 commit 4c0f79c
Showing 1 changed file with 96 additions and 81 deletions.
177 changes: 96 additions & 81 deletions tools/tracing/rtla/src/timerlat_hist.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,21 @@ static void timerlat_hist_header(struct osnoise_tool *tool)
trace_seq_reset(s);
}

/*
* format_summary_value - format a line of summary value (min, max or avg)
* of hist data
*/
static void format_summary_value(struct trace_seq *seq,
int count,
unsigned long long val,
bool avg)
{
if (count)
trace_seq_printf(seq, "%9llu ", avg ? val / count : val);
else
trace_seq_printf(seq, "%9c ", '-');
}

/*
* timerlat_print_summary - print the summary of the hist data to the output
*/
Expand Down Expand Up @@ -327,29 +342,23 @@ timerlat_print_summary(struct timerlat_hist_params *params,
if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count)
continue;

if (!params->no_irq) {
if (data->hist[cpu].irq_count)
trace_seq_printf(trace->seq, "%9llu ",
data->hist[cpu].min_irq);
else
trace_seq_printf(trace->seq, " - ");
}
if (!params->no_irq)
format_summary_value(trace->seq,
data->hist[cpu].irq_count,
data->hist[cpu].min_irq,
false);

if (!params->no_thread) {
if (data->hist[cpu].thread_count)
trace_seq_printf(trace->seq, "%9llu ",
data->hist[cpu].min_thread);
else
trace_seq_printf(trace->seq, " - ");
}
if (!params->no_thread)
format_summary_value(trace->seq,
data->hist[cpu].thread_count,
data->hist[cpu].min_thread,
false);

if (params->user_hist) {
if (data->hist[cpu].user_count)
trace_seq_printf(trace->seq, "%9llu ",
data->hist[cpu].min_user);
else
trace_seq_printf(trace->seq, " - ");
}
if (params->user_hist)
format_summary_value(trace->seq,
data->hist[cpu].user_count,
data->hist[cpu].min_user,
false);
}
trace_seq_printf(trace->seq, "\n");

Expand All @@ -363,29 +372,23 @@ timerlat_print_summary(struct timerlat_hist_params *params,
if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count)
continue;

if (!params->no_irq) {
if (data->hist[cpu].irq_count)
trace_seq_printf(trace->seq, "%9llu ",
data->hist[cpu].sum_irq / data->hist[cpu].irq_count);
else
trace_seq_printf(trace->seq, " - ");
}
if (!params->no_irq)
format_summary_value(trace->seq,
data->hist[cpu].irq_count,
data->hist[cpu].sum_irq,
true);

if (!params->no_thread) {
if (data->hist[cpu].thread_count)
trace_seq_printf(trace->seq, "%9llu ",
data->hist[cpu].sum_thread / data->hist[cpu].thread_count);
else
trace_seq_printf(trace->seq, " - ");
}
if (!params->no_thread)
format_summary_value(trace->seq,
data->hist[cpu].thread_count,
data->hist[cpu].sum_thread,
true);

if (params->user_hist) {
if (data->hist[cpu].user_count)
trace_seq_printf(trace->seq, "%9llu ",
data->hist[cpu].sum_user / data->hist[cpu].user_count);
else
trace_seq_printf(trace->seq, " - ");
}
if (params->user_hist)
format_summary_value(trace->seq,
data->hist[cpu].user_count,
data->hist[cpu].sum_user,
true);
}
trace_seq_printf(trace->seq, "\n");

Expand All @@ -399,29 +402,23 @@ timerlat_print_summary(struct timerlat_hist_params *params,
if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count)
continue;

if (!params->no_irq) {
if (data->hist[cpu].irq_count)
trace_seq_printf(trace->seq, "%9llu ",
data->hist[cpu].max_irq);
else
trace_seq_printf(trace->seq, " - ");
}
if (!params->no_irq)
format_summary_value(trace->seq,
data->hist[cpu].irq_count,
data->hist[cpu].max_irq,
false);

if (!params->no_thread) {
if (data->hist[cpu].thread_count)
trace_seq_printf(trace->seq, "%9llu ",
data->hist[cpu].max_thread);
else
trace_seq_printf(trace->seq, " - ");
}
if (!params->no_thread)
format_summary_value(trace->seq,
data->hist[cpu].thread_count,
data->hist[cpu].max_thread,
false);

if (params->user_hist) {
if (data->hist[cpu].user_count)
trace_seq_printf(trace->seq, "%9llu ",
data->hist[cpu].max_user);
else
trace_seq_printf(trace->seq, " - ");
}
if (params->user_hist)
format_summary_value(trace->seq,
data->hist[cpu].user_count,
data->hist[cpu].max_user,
false);
}
trace_seq_printf(trace->seq, "\n");
trace_seq_do_printf(trace->seq);
Expand Down Expand Up @@ -505,50 +502,68 @@ timerlat_print_stats_all(struct timerlat_hist_params *params,
trace_seq_printf(trace->seq, "min: ");

if (!params->no_irq)
trace_seq_printf(trace->seq, "%9llu ",
sum.min_irq);
format_summary_value(trace->seq,
sum.irq_count,
sum.min_irq,
false);

if (!params->no_thread)
trace_seq_printf(trace->seq, "%9llu ",
sum.min_thread);
format_summary_value(trace->seq,
sum.thread_count,
sum.min_thread,
false);

if (params->user_hist)
trace_seq_printf(trace->seq, "%9llu ",
sum.min_user);
format_summary_value(trace->seq,
sum.user_count,
sum.min_user,
false);

trace_seq_printf(trace->seq, "\n");

if (!params->no_index)
trace_seq_printf(trace->seq, "avg: ");

if (!params->no_irq)
trace_seq_printf(trace->seq, "%9llu ",
sum.sum_irq / sum.irq_count);
format_summary_value(trace->seq,
sum.irq_count,
sum.sum_irq,
true);

if (!params->no_thread)
trace_seq_printf(trace->seq, "%9llu ",
sum.sum_thread / sum.thread_count);
format_summary_value(trace->seq,
sum.thread_count,
sum.sum_thread,
true);

if (params->user_hist)
trace_seq_printf(trace->seq, "%9llu ",
sum.sum_user / sum.user_count);
format_summary_value(trace->seq,
sum.user_count,
sum.sum_user,
true);

trace_seq_printf(trace->seq, "\n");

if (!params->no_index)
trace_seq_printf(trace->seq, "max: ");

if (!params->no_irq)
trace_seq_printf(trace->seq, "%9llu ",
sum.max_irq);
format_summary_value(trace->seq,
sum.irq_count,
sum.max_irq,
false);

if (!params->no_thread)
trace_seq_printf(trace->seq, "%9llu ",
sum.max_thread);
format_summary_value(trace->seq,
sum.thread_count,
sum.max_thread,
false);

if (params->user_hist)
trace_seq_printf(trace->seq, "%9llu ",
sum.max_user);
format_summary_value(trace->seq,
sum.user_count,
sum.max_user,
false);

trace_seq_printf(trace->seq, "\n");
trace_seq_do_printf(trace->seq);
Expand Down

0 comments on commit 4c0f79c

Please sign in to comment.