Re: [PATCH v1] perf evlist: Warn when 'sleep' workload is used without system-wide (-a) option

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The warning for using the 'sleep' workload incorrectly triggers for valid targeted profiling commands.
- [Low] Missing explicit POSIX `<string.h>` header inclusion for libc string functions.
--

--- Patch [1]: [PATCH v1] perf evlist: Warn when 'sleep' workload is used without system-wide (-a) option ---
commit 2e20acd0f78b6776954ca8e2c6f58698c556aa50
Author: Ian Rogers <[email protected]>

perf evlist: Warn when 'sleep' workload is used without system-wide (-a) option

This commit adds a warning to evlist__prepare_workload() when the workload
executable is 'sleep' and system-wide mode is not enabled. This is intended
to catch cases where users forget the '-a' flag and unintentionally only
profile the sleep process itself.

> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index eb7c0d7be064e..d53f594ace86a 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1562,6 +1562,20 @@ int evlist__prepare_workload(struct evlist *evlist, struct target *target, const
>  	int child_ready_pipe[2], go_pipe[2];
>  	char bf;
>  
> +	if (argv && argv[0] && target && !target->system_wide) {

[Severity: Medium]
Does this warning incorrectly trigger when running a targeted profile?

If a user specifies a target (like -p <pid> or -C 0) and uses sleep as the
duration, target->system_wide is false. However, in these targeted modes,
perf correctly monitors the specified target rather than the sleep process,
making the warning spurious.

Should this check instead verify if perf has fallen back to monitoring the
workload itself (for instance, by checking target__none(target))?

> +		const char *cmd = strrchr(argv[0], '/');
> +
> +		if (cmd)
> +			cmd++;
> +		else
> +			cmd = argv[0];
> +
> +		if (!strcmp(cmd, "sleep")) {

[Severity: Low]
This isn't a bug, but should <string.h> be explicitly included in this file
to guarantee musl libc compatibility?

The perf subsystem guidelines require direct, explicit inclusion of POSIX
headers for libc functions like strcmp() and strrchr() to prevent musl
compilation failures. Relying on implicit inclusions can cause builds to
break in environments where headers strictly separate declarations.

> +			ui__warning("workload '%s' specified without the system-wide (-a) option\n",
> +				    cmd);
> +		}
> +	}
> +

[ ... ]

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