summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndrew Price <anprice@redhat.com>2014-04-27 01:55:57 +0100
committerChris Mason <clm@fb.com>2014-09-24 12:02:09 -0700
commit49559b611256ea542f3f9f74951bb787c418fbf8 (patch)
tree9e9ee4e93c410c088478c9d76d3a3834e9cafd4c
parent30420b587db62a3d919d7bc68ef2dd07df5c75fd (diff)
downloadblktrace-49559b611256ea542f3f9f74951bb787c418fbf8.tar.gz
blktrace-49559b611256ea542f3f9f74951bb787c418fbf8.tar.bz2
iowatcher: Convert start_mpstat to run_program
For consistency and deduplication, use run_program in start_mpstat. Add the ability to pass a path to run_program, which will be opened in the spawned process and used as stdout, in order to capture mpstat output. This fixes a tricky descriptor leak in start_mpstat which could have caused a race condition if it was fixed with close(). Some output formatting tweaks have also been added and a bug from a previous patch, where tracers were killed immediately when -p wasn't specified, has been fixed. Signed-off-by: Andrew Price <anprice@redhat.com>
-rw-r--r--iowatcher/blkparse.c2
-rw-r--r--iowatcher/main.c33
-rw-r--r--iowatcher/tracers.c112
-rw-r--r--iowatcher/tracers.h4
4 files changed, 52 insertions, 99 deletions
diff --git a/iowatcher/blkparse.c b/iowatcher/blkparse.c
index e732af7..f837f39 100644
--- a/iowatcher/blkparse.c
+++ b/iowatcher/blkparse.c
@@ -801,7 +801,7 @@ static int dump_traces(struct tracelist *traces, int count, char *dumpfile)
argv[i++] = tl->name;
}
- err = run_program(argc, argv, 1, NULL);
+ err = run_program(argc, argv, 1, NULL, NULL);
if (err)
fprintf(stderr, "%s exited with %d, expected 0\n", argv[0], err);
free(argv);
diff --git a/iowatcher/main.c b/iowatcher/main.c
index c9ddd9c..4441dfc 100644
--- a/iowatcher/main.c
+++ b/iowatcher/main.c
@@ -33,6 +33,7 @@
#include <getopt.h>
#include <limits.h>
#include <float.h>
+#include <signal.h>
#include "plot.h"
#include "blkparse.h"
@@ -1620,16 +1621,9 @@ int main(int ac, char **av)
}
if (num_blktrace_devices) {
- char *path;
-
+ char *path = join_path(blktrace_dest_dir, blktrace_outfile);
dest_mkdir(blktrace_dest_dir);
- if (num_blktrace_devices > 1) {
- snprintf(line, line_len, "%s/%s", blktrace_dest_dir,
- blktrace_outfile);
- dest_mkdir(line);
- }
-
- path = join_path(blktrace_dest_dir, blktrace_outfile);
+ dest_mkdir(path);
snprintf(line, line_len, "%s.dump", path);
unlink(line);
@@ -1637,19 +1631,24 @@ int main(int ac, char **av)
ret = start_blktrace(blktrace_devices, num_blktrace_devices,
blktrace_outfile, blktrace_dest_dir);
if (ret) {
- fprintf(stderr, "exiting due to blktrace failure\n");
- exit(1);
+ perror("Exiting due to blktrace failure");
+ exit(ret);
}
- start_mpstat(path);
+ snprintf(line, line_len, "%s.mpstat", path);
+ ret = start_mpstat(line);
+ if (ret) {
+ perror("Exiting due to mpstat failure");
+ exit(ret);
+ }
if (prog_argv && prog_argc) {
- ret = run_program(prog_argc, prog_argv, 1, NULL);
- if (ret != 127)
- printf("%s exited with %d\n", prog_argv[0], ret);
+ run_program(prog_argc, prog_argv, 1, NULL, NULL);
+ wait_for_tracers(SIGINT);
+ } else {
+ printf("Tracing until interrupted...\n");
+ wait_for_tracers(0);
}
-
- wait_for_tracers();
free(path);
}
diff --git a/iowatcher/tracers.c b/iowatcher/tracers.c
index 6ecf24c..4c3d10d 100644
--- a/iowatcher/tracers.c
+++ b/iowatcher/tracers.c
@@ -40,50 +40,13 @@
extern char **environ;
-static int line_len = 1024;
-static char line[1024];
-
static pid_t blktrace_pid = 0;
static pid_t mpstat_pid = 0;
-char *mpstat_args[] = {
- "mpstat",
- "-P", "ALL", "1",
- NULL,
-};
-
-int stop_tracer(pid_t *tracer_pid)
-{
- int ret;
- pid_t pid = *tracer_pid;
- pid_t pid_ret;
- int status = 0;
-
- if (pid == 0)
- return 0;
-
- *tracer_pid = 0;
- ret = kill(pid, SIGTERM);
- if (ret) {
- fprintf(stderr, "failed to stop tracer pid %lu error %s\n",
- (unsigned long)pid, strerror(errno));
- return -errno;
- }
- pid_ret = waitpid(pid, &status, WUNTRACED);
- if (pid_ret == pid && WIFEXITED(status) == 0) {
- fprintf(stderr, "blktrace returns error %d\n", WEXITSTATUS(status));
- }
- return 0;
-}
-
static void sig_handler_for_quit(int val)
{
fprintf(stderr, "Received signal %d. Terminating tracers.\n", val);
- if (blktrace_pid) {
- wait_program(blktrace_pid, "blktrace", SIGTERM);
- blktrace_pid = 0;
- }
- stop_tracer(&mpstat_pid);
+ wait_for_tracers(SIGTERM);
}
int start_blktrace(char **devices, int num_devices, char *trace_name, char *dest)
@@ -127,7 +90,7 @@ int start_blktrace(char **devices, int num_devices, char *trace_name, char *dest
argv[argc] = NULL;
signal(SIGTERM, sig_handler_for_quit);
signal(SIGINT, sig_handler_for_quit);
- ret = run_program(argc, argv, 0, &blktrace_pid);
+ ret = run_program(argc, argv, 0, &blktrace_pid, NULL);
return ret;
}
@@ -151,6 +114,8 @@ int wait_program(pid_t pid, const char *pname, int sig)
ret = WEXITSTATUS(status);
if (ret == 127) /* spawnp failed after forking */
fprintf(stderr, "Failed to run '%s'\n", pname);
+ else
+ fprintf(stderr, "Exit (%d): %s\n", ret, pname);
} else if (WIFSIGNALED(status) && sig && WTERMSIG(status) != sig) {
fprintf(stderr, "'%s' killed by signal %d\n", pname, WTERMSIG(status));
ret = -1;
@@ -158,18 +123,26 @@ int wait_program(pid_t pid, const char *pname, int sig)
return ret;
}
-int run_program(int argc, char **argv, int wait, pid_t *pid)
+int run_program(int argc, char **argv, int wait, pid_t *pid, char *outpath)
{
int i;
int err;
pid_t _pid;
+ posix_spawn_file_actions_t facts;
+ posix_spawn_file_actions_t *factp = NULL;
- fprintf(stderr, "running");
+ if (outpath != NULL) {
+ posix_spawn_file_actions_init(&facts);
+ posix_spawn_file_actions_addopen(&facts, 1, outpath, O_WRONLY|O_CREAT|O_TRUNC, 0600);
+ factp = &facts;
+ }
+
+ fprintf(stderr, "Start");
for (i = 0; i < argc; i++)
- fprintf(stderr, " '%s'", argv[i]);
+ fprintf(stderr, " %s", argv[i]);
fprintf(stderr, "\n");
- err = posix_spawnp(&_pid, argv[0], NULL, NULL, argv, environ);
+ err = posix_spawnp(&_pid, argv[0], factp, NULL, argv, environ);
if (err != 0) {
fprintf(stderr, "Could not run '%s': %s\n", argv[0], strerror(err));
} else if (wait) {
@@ -183,53 +156,34 @@ int run_program(int argc, char **argv, int wait, pid_t *pid)
return err;
}
-int wait_for_tracers(void)
+int wait_for_tracers(int sig)
{
+ int err;
if (blktrace_pid != 0) {
- int err = wait_program(blktrace_pid, "blktrace", SIGINT);
+ err = wait_program(blktrace_pid, "blktrace", sig);
if (err)
exit(1);
blktrace_pid = 0;
}
if (mpstat_pid != 0) {
- int status = 0;
- stop_tracer(&mpstat_pid);
- waitpid(mpstat_pid, &status, WUNTRACED);
+ err = wait_program(mpstat_pid, "mpstat", sig);
+ if (err)
+ exit(1);
mpstat_pid = 0;
}
return 0;
}
-int start_mpstat(char *trace_name)
+int start_mpstat(char *path)
{
- int fd;
- pid_t pid;
-
- snprintf(line, line_len, "%s.mpstat", trace_name);
-
- fd = open(line, O_WRONLY | O_CREAT | O_TRUNC, 0600);
- if (fd < 0) {
- fprintf(stderr, "unable to open %s for writing err %s\n",
- line, strerror(errno));
- exit(1);
- }
- pid = fork();
- if (pid == 0) {
- int ret;
-
- close(1);
- ret = dup2(fd, 1);
- if (ret < 0) {
- fprintf(stderr, "failed to setup output file for mpstat\n");
- exit(1);
- }
- ret = execvp("mpstat", mpstat_args);
- if (ret < 0) {
- fprintf(stderr, "failed to exec mpstat err %s\n",
- strerror(ret));
- exit(1);
- }
- }
- mpstat_pid = pid;
- return 0;
+ int ret;
+ int argc = 4;
+ char *argv[] = {
+ "mpstat",
+ "-P", "ALL", "1",
+ NULL,
+ };
+
+ ret = run_program(argc, argv, 0, &mpstat_pid, path);
+ return ret;
}
diff --git a/iowatcher/tracers.h b/iowatcher/tracers.h
index 77c31a6..9b4f6a5 100644
--- a/iowatcher/tracers.h
+++ b/iowatcher/tracers.h
@@ -17,12 +17,12 @@
*/
#ifndef __IOWATCH_TRACERS
#define __IOWATCH_TRACERS
-int run_program(int argc, char **argv, int wait, pid_t *pid);
+int run_program(int argc, char **argv, int wait, pid_t *pid, char *stdoutpath);
int wait_program(pid_t pid, const char *pname, int signal);
int stop_blktrace(void);
int start_blktrace(char **devices, int num_devices, char *trace_name, char *dest);
int start_mpstat(char *trace_name);
-int wait_for_tracers(void);
+int wait_for_tracers(int sig);
#endif