Re: [PATCH v2 2/3] perf sched latency: Auto-scale latency and runtime display units

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The table column alignment in `perf sched latency` is broken by the new formatting widths, contradicting the commit message's claim of maintaining column alignment.
--

--- Patch [2]: [PATCH v2 2/3] perf sched latency: Auto-scale latency and runtime display units ---
commit bd1f9f65443ae53474112d9990f8290686a5aaa3
Author: Aaron Tomlin <atomlin@atomlin.com>

perf sched latency: Auto-scale latency and runtime display units

This commit introduces dynamic scaling for latency and runtime values in
'perf sched latency' output. It replaces hardcoded millisecond output with
auto-scaled units (ns, us, ms, s) and updates the table formatting to
improve readability.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index b3cf678573e0a..00ff5abb08b57 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c

[ ... ]

> @@ -1430,14 +1443,17 @@ static void output_lat_thread(struct perf_sched *sched, struct work_atoms *work_
>  		printf(" ");
>  
>  	avg = work_list->total_lat / work_list->nb_atoms;
> +	scnprintf_latency_unit(runtime_lat, sizeof(runtime_lat), work_list->total_runtime);
> +	scnprintf_latency_unit(avg_lat, sizeof(avg_lat), avg);
> +	scnprintf_latency_unit(max_lat, sizeof(max_lat), work_list->max_lat);
>  	timestamp__scnprintf_usec(work_list->max_lat_start, max_lat_start, sizeof(max_lat_start));
>  	timestamp__scnprintf_usec(work_list->max_lat_end, max_lat_end, sizeof(max_lat_end));
>  
> -	printf("|%11.3f ms |%9" PRIu64 " | avg:%8.3f ms | max:%8.3f ms | max start: %12s s | max end: %12s s\n",
> -	      (double)work_list->total_runtime / NSEC_PER_MSEC,
> -		 work_list->nb_atoms, (double)avg / NSEC_PER_MSEC,
> -		 (double)work_list->max_lat / NSEC_PER_MSEC,
> -		 max_lat_start, max_lat_end);
> +	printf("|%13s |%9" PRIu64 " | avg:%10s | max:%10s | max start: %12s s | max end: %12s s\n",
> +	       runtime_lat,
> +	       work_list->nb_atoms, avg_lat, max_lat,
> +	       max_lat_start, max_lat_end);
> +
>  }

[Severity: Low]
Does this format string break the column alignment?

The table header defines a width of 15 characters for the "Runtime" column
(|   Runtime     |), and 17 characters for the "Avg delay" and "Max delay"
columns. The new formats %13s and avg:%10s produce strings that are shorter
than these expected widths, which appears to cause the vertical bar separators
to misalign.

[ ... ]

> @@ -3612,7 +3629,7 @@ static int perf_sched__lat(struct perf_sched *sched)
>  	perf_sched__sort_lat(sched);
>  
>  	printf("\n -------------------------------------------------------------------------------------------------------------------------------------------\n");
> -	printf("  Task                  |   Runtime ms  |  Count   | Avg delay ms    | Max delay ms    | Max delay start           | Max delay end          |\n");
> +	printf("  Task                  |   Runtime     |  Count   | Avg delay       | Max delay       | Max delay start           | Max delay end          |\n");
>  	printf(" -------------------------------------------------------------------------------------------------------------------------------------------\n");

[ ... ]

> @@ -3626,8 +3643,9 @@ static int perf_sched__lat(struct perf_sched *sched)
>  	}
>  
>  	printf(" -----------------------------------------------------------------------------------------------------------------\n");
> -	printf("  TOTAL:                |%11.3f ms |%9" PRIu64 " |\n",
> -		(double)sched->all_runtime / NSEC_PER_MSEC, sched->all_count);
> +	scnprintf_latency_unit(total_runtime_str, sizeof(total_runtime_str), sched->all_runtime);
> +	printf("  TOTAL:                |%13s |%9" PRIu64 " |\n",
> +	       total_runtime_str, sched->all_count);
>  
>  	printf(" ---------------------------------------------------\n");

[Severity: Low]
Does the use of %13s here also break the vertical alignment of the first
column for the TOTAL row summary?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725173341.679782-1-atomlin@atomlin.com?part=2