Re: [PATCH v9 4/4] perf sched latency: Add histogram and time interval options

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing explicit POSIX libc header inclusions for musl compatibility.
--

--- Patch [4]: [PATCH v9 4/4] perf sched latency: Add histogram and time interval options ---
commit 045c1ada1f2d7eef344279deacd80eabe1267dcc
Author: Aaron Tomlin <[email protected]>
Subject: perf sched latency: Add histogram and time interval options

This commit adds --histogram, --hist-mode, and --time options to the
perf sched latency command. These options provide a visual ASCII bar chart
distribution of how long tasks waited for CPU runtime across latency
buckets within a specific time interval.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c

[ ... ]

> @@ -257,6 +324,59 @@ static int scnprintf_latency_unit(char *buf, size_t size, u64 nsecs)
>  	return scnprintf(buf, size, "%6.3f s ", (double)nsecs / NSEC_PER_SEC);
>  }
>  
> +static int latency_bucket(struct perf_sched *sched, u64 delta_ns)

[ ... ]

> +static void print_latency_histogram(struct perf_sched *sched, u64 *hist,
> +				    u64 total_count, const char *title)
> +{
> +	const char **bucket_names = (sched->hist_mode == HIST_MODE_LINEAR) ?
> +		linear_bucket_names : lat_bucket_names;
> +	int bar_total = 40;
> +	char bar[] = "########################################";
> +	int i;
> +
> +	if (total_count == 0)
> +		return;
> +
> +	printf("\n %s (total samples: %" PRIu64 ")\n", title, total_count);

[Severity: Low]
Does this file explicitly include <stdio.h>? The patch introduces new calls
to printf() in print_latency_histogram(), which can cause musl libc builds
to fail due to missing declarations if the file relies on implicit header
inclusions.

[ ... ]

> @@ -3649,6 +3803,24 @@ static int perf_sched__lat(struct perf_sched *sched)
>  
>  	setup_pager();
>  
> +	if (sched->hist_mode_str) {
> +		sched->show_histogram = true;
> +		if (!strcmp(sched->hist_mode_str, "linear"))

[Severity: Low]
Are we sure <string.h> is explicitly included? This introduces a new use
of strcmp() in perf_sched__lat(), which has similar musl compatibility
requirements for explicit POSIX header inclusions.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4