Re: [PATCH 1/2] perf record: Propagate exit status of a command line workload

[Jiri Olsa]

On Wed, Apr 16, 2014 at 09:30:43AM +0900, Namhyung Kim wrote:

SNIP

>  
> @@ -356,6 +331,7 @@ static void workload_exec_failed_signal(int signo, siginfo_t *info,
>  static int __cmd_record(struct record *rec, int argc, const char **argv)
>  {
>  	int err;
> +	int status = 0;
>  	unsigned long waking = 0;
>  	const bool forks = argc > 0;
>  	struct machine *machine;
> @@ -367,7 +343,6 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>  
>  	rec->progname = argv[0];
>  
> -	on_exit(record__sig_exit, rec);

so record__sig_exit took care of waiting the child even
in case the record fails before running the workload

after your change all error cases just delete session
and go away, ending up with parent dead and go_pipe
released executing the workload

with attached patch, current perf code:

[jolsa@krava perf]$ ./perf.old record -e cycles -m 10G true
rounding mmap pages size to 17179869184 bytes (4194304 pages)
Permission error mapping pages.
Consider increasing /proc/sys/kernel/perf_event_mlock_kb,
or try again with a smaller value of -m/--mmap_pages.
(current value: 4194304)
true: Terminated

with attached patch, your change:

[jolsa@krava perf]$ ./perf record -e cycles -m 10G true
rounding mmap pages size to 17179869184 bytes (4194304 pages)
Permission error mapping pages.
Consider increasing /proc/sys/kernel/perf_event_mlock_kb,
or try again with a smaller value of -m/--mmap_pages.
(current value: 4194304)
unable to read pipe: No such file or directory


I think that after creating the workload, all error paths
need to release(wait) the child if there's any

thanks,
jirka

---
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 59ef280..185f30a 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1072,7 +1072,7 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist, struct target *tar
 		/*
 		 * Wait until the parent tells us to go.
 		 */
-		if (read(go_pipe[0], &bf, 1) == -1)
+		if (read(go_pipe[0], &bf, 1) != 1)
 			perror("unable to read pipe");
 
 		execvp(argv[0], (char **)argv);
--
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/