lkml.org 
[lkml]   [2013]   [Nov]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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) {
\
 
 \ /
  Last update: 2013-11-08 18:01    [W:0.104 / U:0.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site