usr.sbin/pmcstat: return non-zero if the launched command fails #1508
Conversation
Previously pmcstat would return a success return code even if the process that it launched crashed or returned a failure code. Returning a non-zero exit code means that we can more easily detect if a benchmark did not run as expected even when running it under pmcstat. While it might make sense to propagate the exit code/signal via pmcstat, simply returning 1 if the child process fails should be sufficient.
|
If you think this looks reasonable, I'll also send this for review upstream. |
Use calloc(). Walk the class list we get from kernel, so we will not add something the kernel does not know about. Avoid picking headers from /usr/include. Differential Revision: https://reviews.freebsd.org/D36401 (cherry picked from commit 1f56795)
| @@ -1516,5 +1520,5 @@ main(int argc, char **argv) | |||
| ); | |||
| } | |||
|
|
|||
| exit(EX_OK); | |||
| exit(exitcode); | |||
There was a problem hiding this comment.
pmcstat(1) already uses <sysexits.h> values in various other places, so using EXIT_* is not consistent. I can see arguments for both behaviors here as you might be profiling a failing command on purpose and the profiling didn't fail, just the executed command (which is what the current behavior tailors towards). The other error returns in pmcstat (of which they are many) all deal with the profiling failing, not the status of the child process. Arguably in our use case of SPEC we should really be running the tests without pmcstat initially to validate that they succeed (including validating output if necessary, not just if it cores) before trying to run them under pmcstat.
There was a problem hiding this comment.
I still think propagating the error would be nice - how about EX_OSERR? I'm not sure any of the sysexits really match, that one just seemed somewhat appropriate.
Previously pmcstat would return a success return code even if the process that it launched crashed or returned a failure code. Returning a non-zero exit code means that we can more easily detect if a benchmark did not run as expected even when running it under pmcstat.
While it might make sense to propagate the exit code/signal via pmcstat, simply returning 1 if the child process fails should be sufficient.