Skip to content

Commit 4c0f79c

Browse files
lenticularis39gregkh
lenticularis39
authored and
gregkh
committed
rtla/timerlat: Fix histogram ALL for zero samples
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]>
1 parent 1cca920 commit 4c0f79c

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
@@ -280,6 +280,21 @@ static void timerlat_hist_header(struct osnoise_tool *tool)
280280
trace_seq_reset(s);
281281
}
282282

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

524527
if (!params->no_irq)
525-
trace_seq_printf(trace->seq, "%9llu ",
526-
sum.sum_irq / sum.irq_count);
528+
format_summary_value(trace->seq,
529+
sum.irq_count,
530+
sum.sum_irq,
531+
true);
527532

528533
if (!params->no_thread)
529-
trace_seq_printf(trace->seq, "%9llu ",
530-
sum.sum_thread / sum.thread_count);
534+
format_summary_value(trace->seq,
535+
sum.thread_count,
536+
sum.sum_thread,
537+
true);
531538

532539
if (params->user_hist)
533-
trace_seq_printf(trace->seq, "%9llu ",
534-
sum.sum_user / sum.user_count);
540+
format_summary_value(trace->seq,
541+
sum.user_count,
542+
sum.sum_user,
543+
true);
535544

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

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

541550
if (!params->no_irq)
542-
trace_seq_printf(trace->seq, "%9llu ",
543-
sum.max_irq);
551+
format_summary_value(trace->seq,
552+
sum.irq_count,
553+
sum.max_irq,
554+
false);
544555

545556
if (!params->no_thread)
546-
trace_seq_printf(trace->seq, "%9llu ",
547-
sum.max_thread);
557+
format_summary_value(trace->seq,
558+
sum.thread_count,
559+
sum.max_thread,
560+
false);
548561

549562
if (params->user_hist)
550-
trace_seq_printf(trace->seq, "%9llu ",
551-
sum.max_user);
563+
format_summary_value(trace->seq,
564+
sum.user_count,
565+
sum.max_user,
566+
false);
552567

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

0 commit comments

Comments
 (0)