Em Thu, Dec 26, 2013 at 09:11:22AM -0500, David Ahern escreveu:
> On 12/24/13, 8:30 AM, Arnaldo Carvalho de Melo wrote:
> >Em Tue, Dec 24, 2013 at 09:53:42AM -0300, Arnaldo Carvalho de Melo escreveu:
> >>>The thing to check is perf_evlist__{prepare,start}_workload notification
> >>>errors using SIGUSR1, that we need to check for in the caller, and emit
> >>>the message, no?
> >Something like this:
> >
> >1. We tell perf_evlist__prepare_workload that we want a signal if execvp
> >fails, it will be a SIGUSR1
> >
> >2. We catch that signal in 'stat' and check that we got a signal, only
> >problem so far with this signal maze is that we're getting a SIGCHLD
> >while I was expecting a SIGUSR1... I.e. the "if (signr != -1) test
> >really should be if (signr == SIGUSR1), but I'm getting a SIGCHLD there
> >and the elves are tugging me away...
>
> Did the elves release you?
>
> There are all kinds of failure paths with the workload functions. In
> the end I was focusing on perf-stat actually checking the rc on the
> start_workload function. If it fails, then write() failed and
> something happened to the workload process. In that case don't show
> the counters.
>
> Handling the other error paths with appropriate messages will take
> additional effort - as you are finding. ;-)
So, please try the attached patch, then apply this one on top, still
needs work for 'record' and 'trace', that now don't print anything when
a workload fails. I'll work on passing a pointer to evlist to the
sigaction, then all this will be hidden away inside
perf_evlist__prepare_workload, etc.
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 1c76c7a66f78..9d0d52d55ee6 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -58,6 +58,7 @@
#include "util/thread.h"
#include "util/thread_map.h"
+#include <signal.h>
#include <stdlib.h>
#include <sys/prctl.h>
#include <locale.h>
@@ -509,16 +510,17 @@ static void handle_initial_delay(void)
}
}
-static volatile bool workload_exec_failed;
+static volatile int workload_exec_errno;
/*
* perf_evlist__prepare_workload will send a SIGUSR1
* if the fork fails, since we asked by setting its
* want_signal to true.
*/
-static void workload_exec_failed_signal(int signo __maybe_unused)
+static void workload_exec_failed_signal(int signo __maybe_unused, siginfo_t *info,
+ void *ucontext __maybe_unused)
{
- workload_exec_failed = true;
+ workload_exec_errno = info->si_value.sival_int;
}
static int __run_perf_stat(int argc, const char **argv)
@@ -596,13 +598,17 @@ static int __run_perf_stat(int argc, const char **argv)
clock_gettime(CLOCK_MONOTONIC, &ref_time);
if (forks) {
+ struct sigaction act = {
+ .sa_flags = SA_SIGINFO,
+ .sa_sigaction = workload_exec_failed_signal,
+ };
/*
* perf_evlist__prepare_workload will, after we call
* perf_evlist__start_Workload, send a SIGUSR1 if the exec call
* fails, that we will catch in workload_signal to flip
- * workload_exec_failed.
+ * workload_exec_errno.
*/
- signal(SIGUSR1, workload_exec_failed_signal);
+ sigaction(SIGUSR1, &act, NULL);
perf_evlist__start_workload(evsel_list);
handle_initial_delay();
@@ -615,8 +621,11 @@ static int __run_perf_stat(int argc, const char **argv)
}
wait(&status);
- if (workload_exec_failed)
+ if (workload_exec_errno) {
+ const char *emsg = strerror_r(workload_exec_errno, msg, sizeof(msg));
+ pr_err("Workload failed: %s\n", emsg);
return -1;
+ }
if (WIFSIGNALED(status))
psignal(WTERMSIG(status), argv[0]);
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index b08a7ecdcea1..4a30c87d24ec 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1073,9 +1073,14 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist, struct target *tar
execvp(argv[0], (char **)argv);
- perror(argv[0]);
- if (want_signal)
- kill(getppid(), SIGUSR1);
+ if (want_signal) {
+ union sigval val;
+
+ val.sival_int = errno;
+ if (sigqueue(getppid(), SIGUSR1, val))
+ perror(argv[0]);
+ } else
+ perror(argv[0]);
exit(-1);
}