Commit da231338 authored by Anand K Mistry's avatar Anand K Mistry Committed by Arnaldo Carvalho de Melo

perf record: Use an eventfd to wakeup when done

The setting and checking of 'done' contains a rare race where the signal
handler setting 'done' is run after checking to break the loop, but
before waiting in evlist__poll(). In this case, the main loop won't wake
up until either another signal is sent, or the perf data fd causes a
wake up.

The following simple script can trigger this condition (but you might
need to run it for several hours):

for ((i = 0; i >= 0; i++)) ; do
  echo "Loop $i"
  delay=$(echo "scale=4; 0.1 * $RANDOM/32768" | bc)
  ./perf record -- sleep 30000000 >/dev/null&
  pid=$!
  sleep $delay
  kill -TERM $pid
  echo "PID $pid"
  wait $pid
done

At some point, the loop will stall. Adding logging, even though perf has
received the SIGTERM and set 'done = 1', perf will remain sleeping until
a second signal is sent.

Committer notes:

Make this dependent on HAVE_EVENTFD_SUPPORT, so that we continue
building on older systems without the eventfd syscall.
Signed-off-by: default avatarAnand K Mistry <amistry@google.com>
Acked-by: default avatarJiri Olsa <jolsa@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20200513122012.v3.1.I4d7421c6bbb1f83ea58419082481082e19097841@changeidSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent ba35fe93
...@@ -56,6 +56,9 @@ ...@@ -56,6 +56,9 @@
#include <unistd.h> #include <unistd.h>
#include <sched.h> #include <sched.h>
#include <signal.h> #include <signal.h>
#ifdef HAVE_EVENTFD_SUPPORT
#include <sys/eventfd.h>
#endif
#include <sys/mman.h> #include <sys/mman.h>
#include <sys/wait.h> #include <sys/wait.h>
#include <sys/types.h> #include <sys/types.h>
...@@ -538,6 +541,9 @@ static int record__pushfn(struct mmap *map, void *to, void *bf, size_t size) ...@@ -538,6 +541,9 @@ static int record__pushfn(struct mmap *map, void *to, void *bf, size_t size)
static volatile int signr = -1; static volatile int signr = -1;
static volatile int child_finished; static volatile int child_finished;
#ifdef HAVE_EVENTFD_SUPPORT
static int done_fd = -1;
#endif
static void sig_handler(int sig) static void sig_handler(int sig)
{ {
...@@ -547,6 +553,21 @@ static void sig_handler(int sig) ...@@ -547,6 +553,21 @@ static void sig_handler(int sig)
signr = sig; signr = sig;
done = 1; done = 1;
#ifdef HAVE_EVENTFD_SUPPORT
{
u64 tmp = 1;
/*
* It is possible for this signal handler to run after done is checked
* in the main loop, but before the perf counter fds are polled. If this
* happens, the poll() will continue to wait even though done is set,
* and will only break out if either another signal is received, or the
* counters are ready for read. To ensure the poll() doesn't sleep when
* done is set, use an eventfd (done_fd) to wake up the poll().
*/
if (write(done_fd, &tmp, sizeof(tmp)) < 0)
pr_err("failed to signal wakeup fd, error: %m\n");
}
#endif // HAVE_EVENTFD_SUPPORT
} }
static void sigsegv_handler(int sig) static void sigsegv_handler(int sig)
...@@ -1547,6 +1568,20 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) ...@@ -1547,6 +1568,20 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
pr_err("Compression initialization failed.\n"); pr_err("Compression initialization failed.\n");
return -1; return -1;
} }
#ifdef HAVE_EVENTFD_SUPPORT
done_fd = eventfd(0, EFD_NONBLOCK);
if (done_fd < 0) {
pr_err("Failed to create wakeup eventfd, error: %m\n");
status = -1;
goto out_delete_session;
}
err = evlist__add_pollfd(rec->evlist, done_fd);
if (err < 0) {
pr_err("Failed to add wakeup eventfd to poll list\n");
status = err;
goto out_delete_session;
}
#endif // HAVE_EVENTFD_SUPPORT
session->header.env.comp_type = PERF_COMP_ZSTD; session->header.env.comp_type = PERF_COMP_ZSTD;
session->header.env.comp_level = rec->opts.comp_level; session->header.env.comp_level = rec->opts.comp_level;
...@@ -1905,6 +1940,10 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) ...@@ -1905,6 +1940,10 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
} }
out_delete_session: out_delete_session:
#ifdef HAVE_EVENTFD_SUPPORT
if (done_fd >= 0)
close(done_fd);
#endif
zstd_fini(&session->zstd_data); zstd_fini(&session->zstd_data);
perf_session__delete(session); perf_session__delete(session);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment