From: Arnaldo Carvalho de Melo <[email protected]>
IOW: Now 'perf record -a' works, this was a bug introduced in:
856e96608a72412d319e498a3a7c557571f811bd
"perf record: Properly synchronize child creation"
Also fix the -C usage, i.e. allow for profiling all the tasks in one
CPU.
Reported-by: Pekka Enberg <[email protected]>
Cc: Frédéric Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-record.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 63136d0..2654253 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -402,7 +402,7 @@ static void atexit_header(void)
perf_header__write(&session->header, output, true);
}
-static int __cmd_record(int argc __used, const char **argv)
+static int __cmd_record(int argc, const char **argv)
{
int i, counter;
struct stat st;
@@ -411,6 +411,7 @@ static int __cmd_record(int argc __used, const char **argv)
int err;
unsigned long waking = 0;
int child_ready_pipe[2], go_pipe[2];
+ const bool forks = target_pid == -1 && argc > 0;
char buf;
page_size = sysconf(_SC_PAGE_SIZE);
@@ -422,7 +423,7 @@ static int __cmd_record(int argc __used, const char **argv)
signal(SIGCHLD, sig_handler);
signal(SIGINT, sig_handler);
- if (pipe(child_ready_pipe) < 0 || pipe(go_pipe) < 0) {
+ if (forks && (pipe(child_ready_pipe) < 0 || pipe(go_pipe) < 0)) {
perror("failed to create pipes");
exit(-1);
}
@@ -483,7 +484,7 @@ static int __cmd_record(int argc __used, const char **argv)
atexit(atexit_header);
- if (target_pid == -1) {
+ if (forks) {
pid = fork();
if (pid < 0) {
perror("failed to fork");
@@ -550,7 +551,7 @@ static int __cmd_record(int argc __used, const char **argv)
return err;
}
- if (!system_wide)
+ if (!system_wide && profile_cpu == -1)
event__synthesize_thread(pid, process_synthesized_event,
session);
else
@@ -569,7 +570,8 @@ static int __cmd_record(int argc __used, const char **argv)
/*
* Let the child rip
*/
- close(go_pipe[1]);
+ if (forks)
+ close(go_pipe[1]);
for (;;) {
int hits = samples;
@@ -667,7 +669,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
argc = parse_options(argc, argv, options, record_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
- if (!argc && target_pid == -1 && (!system_wide || profile_cpu == -1))
+ if (!argc && target_pid == -1 && !system_wide && profile_cpu == -1)
usage_with_options(record_usage, options);
symbol__init();
--
1.6.2.5
On Sat, 2009-12-19 at 19:22 -0200, Arnaldo Carvalho de Melo wrote:
> @@ -550,7 +551,7 @@ static int __cmd_record(int argc __used, const char **argv)
> return err;
> }
>
> - if (!system_wide)
> + if (!system_wide && profile_cpu == -1)
> event__synthesize_thread(pid, process_synthesized_event,
> session);
Not actually sure about this bit though, since we now have
per-task-per-cpu things, the profile_cpu is also used to !system_wide.
Em Sat, Dec 19, 2009 at 10:39:01PM +0100, Peter Zijlstra escreveu:
> On Sat, 2009-12-19 at 19:22 -0200, Arnaldo Carvalho de Melo wrote:
> > @@ -550,7 +551,7 @@ static int __cmd_record(int argc __used, const char **argv)
> > return err;
> > }
> >
> > - if (!system_wide)
> > + if (!system_wide && profile_cpu == -1)
> > event__synthesize_thread(pid, process_synthesized_event,
> > session);
>
> Not actually sure about this bit though, since we now have
> per-task-per-cpu things, the profile_cpu is also used to !system_wide.
this actually means: if it is not system wide and not for an specific
cpu, it must be for an specific pid.
Not clear, right, you may want to profile an specific pid but only when
it is running on an specific CPU... But at least this patch allows for
specifiying either an specific CPU or an specific pid or systemwide.
- Arnaldo
On Sat, 2009-12-19 at 19:22 -0200, Arnaldo Carvalho de Melo wrote:
> @@ -422,7 +423,7 @@ static int __cmd_record(int argc __used, const char **argv)
> signal(SIGCHLD, sig_handler);
> signal(SIGINT, sig_handler);
>
> - if (pipe(child_ready_pipe) < 0 || pipe(go_pipe) < 0) {
> + if (forks && (pipe(child_ready_pipe) < 0 || pipe(go_pipe) < 0)) {
> perror("failed to create pipes");
> exit(-1);
> }
you can avoid this by simply putting the whole thing into the if (forks)
thing below.
On Sat, 2009-12-19 at 19:58 -0200, Arnaldo Carvalho de Melo wrote:
> Em Sat, Dec 19, 2009 at 10:39:01PM +0100, Peter Zijlstra escreveu:
> > On Sat, 2009-12-19 at 19:22 -0200, Arnaldo Carvalho de Melo wrote:
> > > @@ -550,7 +551,7 @@ static int __cmd_record(int argc __used, const char **argv)
> > > return err;
> > > }
> > >
> > > - if (!system_wide)
> > > + if (!system_wide && profile_cpu == -1)
> > > event__synthesize_thread(pid, process_synthesized_event,
> > > session);
> >
> > Not actually sure about this bit though, since we now have
> > per-task-per-cpu things, the profile_cpu is also used to !system_wide.
>
> this actually means: if it is not system wide and not for an specific
> cpu, it must be for an specific pid.
But it can be for a specific pid and specific cpu.
> Not clear, right, you may want to profile an specific pid but only when
> it is running on an specific CPU... But at least this patch allows for
> specifiying either an specific CPU or an specific pid or systemwide.
Ah, right, not enough state to tell what we meant, fair enough.