* [PATCH v1] perf evlist: Warn when 'sleep' workload is used without system-wide (-a) option @ 2026-08-06 2:10 Ian Rogers 2026-08-06 2:19 ` sashiko-bot 2026-08-06 4:14 ` [PATCH v2] " Ian Rogers 0 siblings, 2 replies; 6+ messages in thread From: Ian Rogers @ 2026-08-06 2:10 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark, linux-perf-users, linux-kernel Cc: Tanushree Shah A common mistake when trying to record system-wide profiles for a given duration is running commands like 'perf record sleep 1' or 'perf stat sleep 1' without passing '-a' / '--all-cpus'. When '-a' is omitted, perf defaults to per-process monitoring of the sleep process itself, which does not collect system-wide activity and records very few events. Add a warning in evlist__prepare_workload() when the workload executable is 'sleep' and system-wide mode is not enabled. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/evlist.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index eb7c0d7be064..d53f594ace86 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) { + const char *cmd = strrchr(argv[0], '/'); + + if (cmd) + cmd++; + else + cmd = argv[0]; + + if (!strcmp(cmd, "sleep")) { + ui__warning("workload '%s' specified without the system-wide (-a) option\n", + cmd); + } + } + evlist__set_workload_cork_fd(evlist, -1); if (pipe(child_ready_pipe) < 0) { -- 2.55.0.629.g250fe7f194-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1] perf evlist: Warn when 'sleep' workload is used without system-wide (-a) option 2026-08-06 2:10 [PATCH v1] perf evlist: Warn when 'sleep' workload is used without system-wide (-a) option Ian Rogers @ 2026-08-06 2:19 ` sashiko-bot 2026-08-06 4:14 ` [PATCH v2] " Ian Rogers 1 sibling, 0 replies; 6+ messages in thread From: sashiko-bot @ 2026-08-06 2:19 UTC (permalink / raw) To: Ian Rogers; +Cc: linux-perf-users 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 <irogers@google.com> 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/20260806021038.2291417-1-irogers@google.com?part=1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] perf evlist: Warn when 'sleep' workload is used without system-wide (-a) option 2026-08-06 2:10 [PATCH v1] perf evlist: Warn when 'sleep' workload is used without system-wide (-a) option Ian Rogers 2026-08-06 2:19 ` sashiko-bot @ 2026-08-06 4:14 ` Ian Rogers 2026-08-17 21:45 ` Ian Rogers ` (2 more replies) 1 sibling, 3 replies; 6+ messages in thread From: Ian Rogers @ 2026-08-06 4:14 UTC (permalink / raw) To: irogers, acme, namhyung Cc: adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, tshah A common mistake when trying to record system-wide profiles for a given duration is running commands like 'perf record sleep 1' or 'perf stat sleep 1' without passing '-a' / '--all-cpus'. When '-a' is omitted, perf defaults to per-process monitoring of the sleep process itself, which does not collect system-wide activity and records very few events. Add a warning in evlist__prepare_workload() when the workload executable is 'sleep' and system-wide mode is not enabled. Assisted-by: Antigravity:gemini-3.6-flash Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/evlist.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index eb7c0d7be064..b3dd097fbb2c 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -11,6 +11,7 @@ #include <inttypes.h> #include <signal.h> #include <stdlib.h> +#include <string.h> #include <fcntl.h> #include <linux/bitops.h> @@ -1562,6 +1563,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__none(target)) { + const char *cmd = strrchr(argv[0], '/'); + + if (cmd) + cmd++; + else + cmd = argv[0]; + + if (!strcmp(cmd, "sleep")) { + ui__warning("workload '%s' specified without the system-wide (-a) option\n", + cmd); + } + } + evlist__set_workload_cork_fd(evlist, -1); if (pipe(child_ready_pipe) < 0) { -- 2.55.0.629.g250fe7f194-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf evlist: Warn when 'sleep' workload is used without system-wide (-a) option 2026-08-06 4:14 ` [PATCH v2] " Ian Rogers @ 2026-08-17 21:45 ` Ian Rogers 2026-08-17 23:02 ` Namhyung Kim 2026-08-18 21:23 ` Namhyung Kim 2 siblings, 0 replies; 6+ messages in thread From: Ian Rogers @ 2026-08-17 21:45 UTC (permalink / raw) To: irogers, acme, namhyung Cc: adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, tshah On Wed, Aug 5, 2026 at 9:14 PM Ian Rogers <irogers@google.com> wrote: > > A common mistake when trying to record system-wide profiles for a given > duration is running commands like 'perf record sleep 1' or 'perf stat > sleep 1' without passing '-a' / '--all-cpus'. When '-a' is omitted, perf > defaults to per-process monitoring of the sleep process itself, which > does not collect system-wide activity and records very few events. > > Add a warning in evlist__prepare_workload() when the workload executable > is 'sleep' and system-wide mode is not enabled. > > Assisted-by: Antigravity:gemini-3.6-flash > Signed-off-by: Ian Rogers <irogers@google.com> Ping. Thanks, Ian > --- > tools/perf/util/evlist.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index eb7c0d7be064..b3dd097fbb2c 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -11,6 +11,7 @@ > #include <inttypes.h> > #include <signal.h> > #include <stdlib.h> > +#include <string.h> > > #include <fcntl.h> > #include <linux/bitops.h> > @@ -1562,6 +1563,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__none(target)) { > + const char *cmd = strrchr(argv[0], '/'); > + > + if (cmd) > + cmd++; > + else > + cmd = argv[0]; > + > + if (!strcmp(cmd, "sleep")) { > + ui__warning("workload '%s' specified without the system-wide (-a) option\n", > + cmd); > + } > + } > + > evlist__set_workload_cork_fd(evlist, -1); > > if (pipe(child_ready_pipe) < 0) { > -- > 2.55.0.629.g250fe7f194-goog > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf evlist: Warn when 'sleep' workload is used without system-wide (-a) option 2026-08-06 4:14 ` [PATCH v2] " Ian Rogers 2026-08-17 21:45 ` Ian Rogers @ 2026-08-17 23:02 ` Namhyung Kim 2026-08-18 21:23 ` Namhyung Kim 2 siblings, 0 replies; 6+ messages in thread From: Namhyung Kim @ 2026-08-17 23:02 UTC (permalink / raw) To: Ian Rogers Cc: acme, adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, tshah On Wed, Aug 05, 2026 at 09:14:14PM -0700, Ian Rogers wrote: > A common mistake when trying to record system-wide profiles for a given > duration is running commands like 'perf record sleep 1' or 'perf stat > sleep 1' without passing '-a' / '--all-cpus'. When '-a' is omitted, perf > defaults to per-process monitoring of the sleep process itself, which > does not collect system-wide activity and records very few events. > > Add a warning in evlist__prepare_workload() when the workload executable > is 'sleep' and system-wide mode is not enabled. > > Assisted-by: Antigravity:gemini-3.6-flash > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/util/evlist.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index eb7c0d7be064..b3dd097fbb2c 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -11,6 +11,7 @@ > #include <inttypes.h> > #include <signal.h> > #include <stdlib.h> > +#include <string.h> > > #include <fcntl.h> > #include <linux/bitops.h> > @@ -1562,6 +1563,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__none(target)) { As I read the code, argv and target should be provided. So the condition simply can be target__none(). I'll make the change. > + const char *cmd = strrchr(argv[0], '/'); > + > + if (cmd) > + cmd++; > + else > + cmd = argv[0]; > + > + if (!strcmp(cmd, "sleep")) { > + ui__warning("workload '%s' specified without the system-wide (-a) option\n", > + cmd); Now it's not just the system-wide option, but I guess it's most likey what users want. I hope this would improve UX of the tool a bit. :) Thanks, Namhyung > + } > + } > + > evlist__set_workload_cork_fd(evlist, -1); > > if (pipe(child_ready_pipe) < 0) { > -- > 2.55.0.629.g250fe7f194-goog > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf evlist: Warn when 'sleep' workload is used without system-wide (-a) option 2026-08-06 4:14 ` [PATCH v2] " Ian Rogers 2026-08-17 21:45 ` Ian Rogers 2026-08-17 23:02 ` Namhyung Kim @ 2026-08-18 21:23 ` Namhyung Kim 2 siblings, 0 replies; 6+ messages in thread From: Namhyung Kim @ 2026-08-18 21:23 UTC (permalink / raw) To: acme, Ian Rogers Cc: adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, tshah On Wed, 05 Aug 2026 21:14:14 -0700, Ian Rogers wrote: > A common mistake when trying to record system-wide profiles for a given > duration is running commands like 'perf record sleep 1' or 'perf stat > sleep 1' without passing '-a' / '--all-cpus'. When '-a' is omitted, perf > defaults to per-process monitoring of the sleep process itself, which > does not collect system-wide activity and records very few events. > > Add a warning in evlist__prepare_workload() when the workload executable > is 'sleep' and system-wide mode is not enabled. > > [...] Applied to perf-tools-next, thanks! Best regards, Namhyung ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-18 21:23 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-06 2:10 [PATCH v1] perf evlist: Warn when 'sleep' workload is used without system-wide (-a) option Ian Rogers 2026-08-06 2:19 ` sashiko-bot 2026-08-06 4:14 ` [PATCH v2] " Ian Rogers 2026-08-17 21:45 ` Ian Rogers 2026-08-17 23:02 ` Namhyung Kim 2026-08-18 21:23 ` Namhyung Kim
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.