Skip to content

Commit 6cc45f8

Browse files
lenticularis39rostedt
authored andcommitted
rtla/timerlat: Fix histogram ALL for zero samples
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: stable@vger.kernel.org Link: https://lore.kernel.org/20241127134130.51171-1-tglozar@redhat.com Fixes: 1462501 ("rtla/timerlat: Add a summary for hist mode") Signed-off-by: Tomas Glozar <tglozar@redhat.com> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
1 parent 4bbf902 commit 6cc45f8

File tree

1 file changed

+96
-81
lines changed

1 file changed

+96
-81
lines changed

tools/tracing/rtla/src/timerlat_hist.c

Lines changed: 96 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,21 @@ static void timerlat_hist_header(struct osnoise_tool *tool)
281281
trace_seq_reset(s);
282282
}
283283

284+
/*
285+
* format_summary_value - format a line of summary value (min, max or avg)
286+
* of hist data
287+
*/
288+
static void format_summary_value(struct trace_seq *seq,
289+
int count,
290+
unsigned long long val,
291+
bool avg)
292+
{
293+
if (count)
294+
trace_seq_printf(seq, "%9llu ", avg ? val / count : val);
295+
else
296+
trace_seq_printf(seq, "%9c ", '-');
297+
}
298+
284299
/*
285300
* timerlat_print_summary - print the summary of the hist data to the output
286301
*/
@@ -328,29 +343,23 @@ timerlat_print_summary(struct timerlat_hist_params *params,
328343
if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count)
329344
continue;
330345

331-
if (!params->no_irq) {
332-
if (data->hist[cpu].irq_count)
333-
trace_seq_printf(trace->seq, "%9llu ",
334-
data->hist[cpu].min_irq);
335-
else
336-
trace_seq_printf(trace->seq, " - ");
337-
}
346+
if (!params->no_irq)
347+
format_summary_value(trace->seq,
348+
data->hist[cpu].irq_count,
349+
data->hist[cpu].min_irq,
350+
false);
338351

339-
if (!params->no_thread) {
340-
if (data->hist[cpu].thread_count)
341-
trace_seq_printf(trace->seq, "%9llu ",
342-
data->hist[cpu].min_thread);
343-
else
344-
trace_seq_printf(trace->seq, " - ");
345-
}
352+
if (!params->no_thread)
353+
format_summary_value(trace->seq,
354+
data->hist[cpu].thread_count,
355+
data->hist[cpu].min_thread,
356+
false);
346357

347-
if (params->user_hist) {
348-
if (data->hist[cpu].user_count)
349-
trace_seq_printf(trace->seq, "%9llu ",
350-
data->hist[cpu].min_user);
351-
else
352-
trace_seq_printf(trace->seq, " - ");
353-
}
358+
if (params->user_hist)
359+
format_summary_value(trace->seq,
360+
data->hist[cpu].user_count,
361+
data->hist[cpu].min_user,
362+
false);
354363
}
355364
trace_seq_printf(trace->seq, "\n");
356365

@@ -364,29 +373,23 @@ timerlat_print_summary(struct timerlat_hist_params *params,
364373
if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count)
365374
continue;
366375

367-
if (!params->no_irq) {
368-
if (data->hist[cpu].irq_count)
369-
trace_seq_printf(trace->seq, "%9llu ",
370-
data->hist[cpu].sum_irq / data->hist[cpu].irq_count);
371-
else
372-
trace_seq_printf(trace->seq, " - ");
373-
}
376+
if (!params->no_irq)
377+
format_summary_value(trace->seq,
378+
data->hist[cpu].irq_count,
379+
data->hist[cpu].sum_irq,
380+
true);
374381

375-
if (!params->no_thread) {
376-
if (data->hist[cpu].thread_count)
377-
trace_seq_printf(trace->seq, "%9llu ",
378-
data->hist[cpu].sum_thread / data->hist[cpu].thread_count);
379-
else
380-
trace_seq_printf(trace->seq, " - ");
381-
}
382+
if (!params->no_thread)
383+
format_summary_value(trace->seq,
384+
data->hist[cpu].thread_count,
385+
data->hist[cpu].sum_thread,
386+
true);
382387

383-
if (params->user_hist) {
384-
if (data->hist[cpu].user_count)
385-
trace_seq_printf(trace->seq, "%9llu ",
386-
data->hist[cpu].sum_user / data->hist[cpu].user_count);
387-
else
388-
trace_seq_printf(trace->seq, " - ");
389-
}
388+
if (params->user_hist)
389+
format_summary_value(trace->seq,
390+
data->hist[cpu].user_count,
391+
data->hist[cpu].sum_user,
392+
true);
390393
}
391394
trace_seq_printf(trace->seq, "\n");
392395

@@ -400,29 +403,23 @@ timerlat_print_summary(struct timerlat_hist_params *params,
400403
if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count)
401404
continue;
402405

403-
if (!params->no_irq) {
404-
if (data->hist[cpu].irq_count)
405-
trace_seq_printf(trace->seq, "%9llu ",
406-
data->hist[cpu].max_irq);
407-
else
408-
trace_seq_printf(trace->seq, " - ");
409-
}
406+
if (!params->no_irq)
407+
format_summary_value(trace->seq,
408+
data->hist[cpu].irq_count,
409+
data->hist[cpu].max_irq,
410+
false);
410411

411-
if (!params->no_thread) {
412-
if (data->hist[cpu].thread_count)
413-
trace_seq_printf(trace->seq, "%9llu ",
414-
data->hist[cpu].max_thread);
415-
else
416-
trace_seq_printf(trace->seq, " - ");
417-
}
412+
if (!params->no_thread)
413+
format_summary_value(trace->seq,
414+
data->hist[cpu].thread_count,
415+
data->hist[cpu].max_thread,
416+
false);
418417

419-
if (params->user_hist) {
420-
if (data->hist[cpu].user_count)
421-
trace_seq_printf(trace->seq, "%9llu ",
422-
data->hist[cpu].max_user);
423-
else
424-
trace_seq_printf(trace->seq, " - ");
425-
}
418+
if (params->user_hist)
419+
format_summary_value(trace->seq,
420+
data->hist[cpu].user_count,
421+
data->hist[cpu].max_user,
422+
false);
426423
}
427424
trace_seq_printf(trace->seq, "\n");
428425
trace_seq_do_printf(trace->seq);
@@ -506,50 +503,68 @@ timerlat_print_stats_all(struct timerlat_hist_params *params,
506503
trace_seq_printf(trace->seq, "min: ");
507504

508505
if (!params->no_irq)
509-
trace_seq_printf(trace->seq, "%9llu ",
510-
sum.min_irq);
506+
format_summary_value(trace->seq,
507+
sum.irq_count,
508+
sum.min_irq,
509+
false);
511510

512511
if (!params->no_thread)
513-
trace_seq_printf(trace->seq, "%9llu ",
514-
sum.min_thread);
512+
format_summary_value(trace->seq,
513+
sum.thread_count,
514+
sum.min_thread,
515+
false);
515516

516517
if (params->user_hist)
517-
trace_seq_printf(trace->seq, "%9llu ",
518-
sum.min_user);
518+
format_summary_value(trace->seq,
519+
sum.user_count,
520+
sum.min_user,
521+
false);
519522

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

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

525528
if (!params->no_irq)
526-
trace_seq_printf(trace->seq, "%9llu ",
527-
sum.sum_irq / sum.irq_count);
529+
format_summary_value(trace->seq,
530+
sum.irq_count,
531+
sum.sum_irq,
532+
true);
528533

529534
if (!params->no_thread)
530-
trace_seq_printf(trace->seq, "%9llu ",
531-
sum.sum_thread / sum.thread_count);
535+
format_summary_value(trace->seq,
536+
sum.thread_count,
537+
sum.sum_thread,
538+
true);
532539

533540
if (params->user_hist)
534-
trace_seq_printf(trace->seq, "%9llu ",
535-
sum.sum_user / sum.user_count);
541+
format_summary_value(trace->seq,
542+
sum.user_count,
543+
sum.sum_user,
544+
true);
536545

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

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

542551
if (!params->no_irq)
543-
trace_seq_printf(trace->seq, "%9llu ",
544-
sum.max_irq);
552+
format_summary_value(trace->seq,
553+
sum.irq_count,
554+
sum.max_irq,
555+
false);
545556

546557
if (!params->no_thread)
547-
trace_seq_printf(trace->seq, "%9llu ",
548-
sum.max_thread);
558+
format_summary_value(trace->seq,
559+
sum.thread_count,
560+
sum.max_thread,
561+
false);
549562

550563
if (params->user_hist)
551-
trace_seq_printf(trace->seq, "%9llu ",
552-
sum.max_user);
564+
format_summary_value(trace->seq,
565+
sum.user_count,
566+
sum.max_user,
567+
false);
553568

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

0 commit comments

Comments
 (0)