Messages in this thread Patch in this message | | | Date | Fri, 08 Nov 2013 09:52:17 -0700 | From | David Ahern <> | Subject | Re: [PATCH 2/2] perf record: mmap output file - v4 |
| |
On 11/8/13, 2:34 AM, Jiri Olsa wrote: >> + if (ftruncate(fd, len) != 0) >> + pr_err("ftruncate failed\n"); > > I think we should fail here and dont let the finishing > code run on probably corrupted file. > > the code that process build IDs could even get stuck
Yes, I'll fix that in v5.
It also got me to thinking about failure scenarios -- like out of space. I was looking at this path using a size-limited tmpfs (max 1M) and writing perf.data into it.
Today (without this mmap output page) if the write() fails due to lack of space then perf emits the message: failed to write perf data, error: No space left on device
and stops — killing the workload too. Trying to read and process the perf.data file fails with a SIGBUS error. I just sent a patch this addresses that problem.
Using that patch as a start point and moving to mmap-based output if the filesystem runs out of space the memcpy generates a SIGBUS and we need to jump out of the memcpy. The attached fixes that problem.
Any objections to using a sigjmp? Other option is to die in the signal handler. That option is harder to clean up.
David
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 947970e182a5..b50f384d9a36 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -29,9 +29,11 @@ #include <unistd.h> #include <sched.h> #include <sys/mman.h> +#include <setjmp.h> /* output file mmap'ed N chunks at a time */ #define MMAP_OUTPUT_SIZE (64*1024*1024) +sigjmp_buf mmap_jmp; #ifndef HAVE_ON_EXIT_SUPPORT #ifndef ATEXIT_MAX @@ -141,6 +143,7 @@ static int do_mmap_output(struct perf_record *rec, void *buf, size_t size) { u64 remaining; off_t offset; + volatile size_t total_len = 0; if (rec->mmap.addr == NULL) { next_segment: @@ -157,20 +160,23 @@ next_segment: * space write what we can then go back and create the * next segment */ - if (size > remaining) { - memcpy(rec->mmap.addr + rec->mmap.offset, buf, remaining); + if (setjmp(mmap_jmp) != 0) { + pr_err("mmap copy failed.\n"); + return -1; + } + if (size-total_len > remaining) { + memcpy(rec->mmap.addr + rec->mmap.offset, buf+total_len, remaining); rec->bytes_written += remaining; - size -= remaining; - buf += remaining; + total_len += remaining; munmap(rec->mmap.addr, rec->mmap.out_size); goto next_segment; } /* more data to copy and it fits in the current segment */ - if (size) { - memcpy(rec->mmap.addr + rec->mmap.offset, buf, size); + if (size - total_len) { + memcpy(rec->mmap.addr + rec->mmap.offset, buf+total_len, size-total_len); rec->bytes_written += size; rec->mmap.offset += size; } @@ -272,6 +278,9 @@ static void sig_handler(int sig) if (sig == SIGCHLD) child_finished = 1; + if (sig == SIGBUS) + longjmp(mmap_jmp, 1); + done = 1; signr = sig; } @@ -526,6 +535,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv) signal(SIGINT, sig_handler); signal(SIGUSR1, sig_handler); signal(SIGTERM, sig_handler); + signal(SIGBUS, sig_handler); session = perf_session__new(file, false, NULL); if (session == NULL) { | |