Re: [PATCH] perf stat: Do not show stats if workload fails

From: Arnaldo Carvalho de Melo
Date: Tue Dec 24 2013 - 08:35:37 EST


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...

- Arnaldo

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index dab98b50c9fe..d2350fef8cde 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -144,6 +144,7 @@ static struct timespec ref_time;
static struct cpu_map *aggr_map;
static int (*aggr_get_id)(struct cpu_map *m, int cpu);

+static volatile int signr = -1;
static volatile int done = 0;

struct perf_stat {
@@ -531,7 +532,7 @@ static int __run_perf_stat(int argc, const char **argv)

if (forks) {
if (perf_evlist__prepare_workload(evsel_list, &target, argv,
- false, false) < 0) {
+ false, true) < 0) {
perror("failed to prepare workload");
return -1;
}
@@ -598,6 +599,8 @@ static int __run_perf_stat(int argc, const char **argv)
wait(&status);
if (WIFSIGNALED(status))
psignal(WTERMSIG(status), argv[0]);
+ else if (signr != -1)
+ return -1;
} else {
handle_initial_delay();
while (!done) {
@@ -1335,8 +1338,6 @@ static void print_stat(int argc, const char **argv)
}
}

-static volatile int signr = -1;
-
static void skip_signal(int signo)
{
if ((child_pid == -1) || interval)
@@ -1785,6 +1786,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
signal(SIGCHLD, skip_signal);
signal(SIGALRM, skip_signal);
signal(SIGABRT, skip_signal);
+ signal(SIGUSR1, skip_signal);

status = 0;
for (run_idx = 0; forever || run_idx < run_count; run_idx++) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/