linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] perf: mmap output file - v4
@ 2013-11-08  4:23 David Ahern
  2013-11-08  4:23 ` [PATCH 1/2] perf record: Move existing write_output into helper function David Ahern
  2013-11-08  4:23 ` [PATCH 2/2] perf record: mmap output file - v4 David Ahern
  0 siblings, 2 replies; 11+ messages in thread
From: David Ahern @ 2013-11-08  4:23 UTC (permalink / raw)
  To: acme, linux-kernel; +Cc: mingo, jolsa, David Ahern

Version 4. I think I have addressed all of Ingo's comments plus a few
extra refactorings.

Ingo: updated responses to your comment:

1. out-pages and rounding up to power of 2
   - leveraging the same code used for mmap-pages. A follow on patch
     will do that for both mmap use cases.

2. what happens if a user sets out-pages to zero
   - the parsing function does not allow it

David Ahern (2):
  perf record: Move existing write_output into helper function
  perf record: mmap output file - v4

 tools/perf/Documentation/perf-record.txt |   5 +
 tools/perf/builtin-record.c              | 162 ++++++++++++++++++++++++++++++-
 2 files changed, 166 insertions(+), 1 deletion(-)

-- 
1.8.3.4 (Apple Git-47)


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] perf record: Move existing write_output into helper function
  2013-11-08  4:23 [PATCH 0/2] perf: mmap output file - v4 David Ahern
@ 2013-11-08  4:23 ` David Ahern
  2013-11-12 21:55   ` [tip:perf/urgent] " tip-bot for David Ahern
  2013-11-08  4:23 ` [PATCH 2/2] perf record: mmap output file - v4 David Ahern
  1 sibling, 1 reply; 11+ messages in thread
From: David Ahern @ 2013-11-08  4:23 UTC (permalink / raw)
  To: acme, linux-kernel
  Cc: mingo, jolsa, David Ahern, Frederic Weisbecker, Peter Zijlstra,
	Namhyung Kim, Mike Galbraith, Stephane Eranian

Code move only; no logic changes. In preparation for the mmap
based output option in the next patch.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/builtin-record.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 15280b5e5574..6e6a41856c41 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -76,7 +76,7 @@ struct perf_record {
 	long			samples;
 };
 
-static int write_output(struct perf_record *rec, void *buf, size_t size)
+static int do_write_output(struct perf_record *rec, void *buf, size_t size)
 {
 	struct perf_data_file *file = &rec->file;
 
@@ -97,6 +97,11 @@ static int write_output(struct perf_record *rec, void *buf, size_t size)
 	return 0;
 }
 
+static int write_output(struct perf_record *rec, void *buf, size_t size)
+{
+	return do_write_output(rec, buf, size);
+}
+
 static int process_synthesized_event(struct perf_tool *tool,
 				     union perf_event *event,
 				     struct perf_sample *sample __maybe_unused,
-- 
1.8.3.4 (Apple Git-47)


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] perf record: mmap output file - v4
  2013-11-08  4:23 [PATCH 0/2] perf: mmap output file - v4 David Ahern
  2013-11-08  4:23 ` [PATCH 1/2] perf record: Move existing write_output into helper function David Ahern
@ 2013-11-08  4:23 ` David Ahern
  2013-11-08  9:34   ` Jiri Olsa
  2013-11-11 11:29   ` Ingo Molnar
  1 sibling, 2 replies; 11+ messages in thread
From: David Ahern @ 2013-11-08  4:23 UTC (permalink / raw)
  To: acme, linux-kernel
  Cc: mingo, jolsa, David Ahern, Frederic Weisbecker, Peter Zijlstra,
	Namhyung Kim, Mike Galbraith, Stephane Eranian

When recording raw_syscalls for the entire system, e.g.,
    perf record -e raw_syscalls:*,sched:sched_switch -a -- sleep 1

you end up with a negative feedback loop as perf itself calls write() fairly
often. This patch handles the problem by mmap'ing the file in chunks of 64M at
a time and copies events from the event buffers to the file avoiding write
system calls.

Before (with write syscall):

    perf record -o /tmp/perf.data -e raw_syscalls:*,sched:sched_switch -a -- sleep 1
    [ perf record: Woken up 0 times to write data ]
    [ perf record: Captured and wrote 81.843 MB /tmp/perf.data (~3575786 samples) ]

After (using mmap):

    perf record -o /tmp/perf.data -e raw_syscalls:*,sched:sched_switch -a -- sleep 1
    [ perf record: Woken up 31 times to write data ]
    [ perf record: Captured and wrote 8.203 MB /tmp/perf.data (~358388 samples) ]

In addition to perf-trace benefits using mmap lowers the overhead of
perf-record. For example,

  perf stat -i -- perf record -g -o /tmp/perf.data openssl speed aes

shows a drop in time, CPU cycles, and instructions all drop by more than a
factor of 3. Jiri also ran a test that showed a big improvement.

v4: Refactoring per Ingo's comments

v3: Removed use of bytes_at_mmap_start at the stat() that set it
    Added user option to control the size of the mmap for writing file.

v2: Removed msync call before munmap per Jiri's suggestion

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/Documentation/perf-record.txt |   5 +
 tools/perf/builtin-record.c              | 155 +++++++++++++++++++++++++++++++
 2 files changed, 160 insertions(+)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 052f7c4dc00c..af11c2dd2360 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -201,6 +201,11 @@ abort events and some memory events in precise mode on modern Intel CPUs.
 --transaction::
 Record transaction flags for transaction related events.
 
+--out-pages=::
+Number of pages to mmap for writing data to file or size specification
+with appended unit character - B/K/M/G. The size is rounded up to have nearest
+pages power of two value.  Default size is 64M.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 6e6a41856c41..72dd983832f5 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -30,6 +30,9 @@
 #include <sched.h>
 #include <sys/mman.h>
 
+/* output file mmap'ed N chunks at a time */
+#define MMAP_OUTPUT_SIZE   (64*1024*1024)
+
 #ifndef HAVE_ON_EXIT_SUPPORT
 #ifndef ATEXIT_MAX
 #define ATEXIT_MAX 32
@@ -65,6 +68,16 @@ static void __handle_on_exit_funcs(void)
 struct perf_record {
 	struct perf_tool	tool;
 	struct perf_record_opts	opts;
+
+	/* for MMAP based file writes */
+	struct {
+		void		*addr;
+		u64		offset;     /* current location within mmap */
+		unsigned int	out_pages;  /* user configurable option */
+		size_t		out_size;   /* size of mmap segments */
+		bool		use;
+	} mmap;
+
 	u64			bytes_written;
 	struct perf_data_file	file;
 	struct perf_evlist	*evlist;
@@ -76,6 +89,95 @@ struct perf_record {
 	long			samples;
 };
 
+static int mmap_next_segment(struct perf_record *rec, off_t offset)
+{
+	struct perf_data_file *file = &rec->file;
+
+	/* extend file to include a new mmap segment */
+	if (ftruncate(file->fd, offset + rec->mmap.out_size) != 0) {
+		pr_err("ftruncate failed\n");
+		return -1;
+	}
+
+	rec->mmap.addr = mmap(NULL, rec->mmap.out_size,
+			      PROT_WRITE | PROT_READ, MAP_SHARED,
+			      file->fd, offset);
+
+	if (rec->mmap.addr == MAP_FAILED) {
+		pr_err("mmap failed: %d: %s\n", errno, strerror(errno));
+
+		/* reset file size */
+		if (ftruncate(file->fd, offset) != 0)
+			pr_err("ftruncate failed too. Is it Halloween?\n");
+
+		return -1;
+	}
+
+	return 0;
+}
+
+static off_t next_mmap_offset(struct perf_record *rec)
+{
+	off_t offset;
+
+	/*
+	 * for first segment, mmap offset is current amount of data
+	 * already written to file. For follow on segments the output
+	 * starts at 0.
+	 */
+	offset = rec->session->header.data_offset + rec->bytes_written;
+	if (offset < (ssize_t) rec->mmap.out_size) {
+		rec->mmap.offset = offset;
+		offset = 0;
+	} else {
+		rec->mmap.offset = 0;
+	}
+
+	/* returning offset within file - used for mmap of next segment */
+	return offset;
+}
+
+static int do_mmap_output(struct perf_record *rec, void *buf, size_t size)
+{
+	u64 remaining;
+	off_t offset;
+
+	if (rec->mmap.addr == NULL) {
+next_segment:
+		offset = next_mmap_offset(rec);
+		if (mmap_next_segment(rec, offset) != 0)
+			return -1;
+	}
+
+	/* amount of space in current mmap segment */
+	remaining = rec->mmap.out_size - rec->mmap.offset;
+
+	/*
+	 * if current size to write is more than the available
+	 * 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);
+		rec->bytes_written += remaining;
+
+		size -= remaining;
+		buf  += 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);
+		rec->bytes_written += size;
+		rec->mmap.offset += size;
+	}
+
+	return 0;
+}
+
 static int do_write_output(struct perf_record *rec, void *buf, size_t size)
 {
 	struct perf_data_file *file = &rec->file;
@@ -99,6 +201,9 @@ static int do_write_output(struct perf_record *rec, void *buf, size_t size)
 
 static int write_output(struct perf_record *rec, void *buf, size_t size)
 {
+	if (rec->mmap.use)
+		return do_mmap_output(rec, buf, size);
+
 	return do_write_output(rec, buf, size);
 }
 
@@ -361,6 +466,46 @@ static void perf_record__init_features(struct perf_record *rec)
 		perf_header__clear_feat(&session->header, HEADER_BRANCH_STACK);
 }
 
+static void mmap_output_fini(struct perf_record *rec)
+{
+	off_t len;
+	int fd;
+
+	if (!rec->mmap.use)
+		return;
+
+	rec->mmap.use = false;
+
+	len = rec->session->header.data_offset + rec->bytes_written;
+	fd = rec->file.fd;
+
+	munmap(rec->mmap.addr, rec->mmap.out_size);
+	rec->mmap.addr = NULL;
+
+	if (ftruncate(fd, len) != 0)
+		pr_err("ftruncate failed\n");
+
+	/*
+	 * Set output pointer to end of file
+	 * eg., needed for buildid processing
+	 */
+	lseek(fd, len, SEEK_SET);
+}
+
+static void mmap_output_init(struct perf_record *rec)
+{
+	struct perf_data_file *file = &rec->file;
+
+	if (file->is_pipe)
+		return;
+
+	if (rec->mmap.out_pages)
+		rec->mmap.out_size = rec->mmap.out_pages * page_size;
+
+	if (rec->mmap.out_size)
+		rec->mmap.use = true;
+}
+
 static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 {
 	int err;
@@ -434,6 +579,8 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 		goto out_delete_session;
 	}
 
+	mmap_output_init(rec);
+
 	machine = &session->machines.host;
 
 	if (file->is_pipe) {
@@ -549,6 +696,8 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 		}
 	}
 
+	mmap_output_fini(rec);
+
 	if (quiet || signr == SIGUSR1)
 		return 0;
 
@@ -810,6 +959,9 @@ static struct perf_record record = {
 			.uses_mmap   = true,
 		},
 	},
+	.mmap = {
+		.out_size = MMAP_OUTPUT_SIZE,
+	},
 };
 
 #define CALLCHAIN_HELP "setup and enables call-graph (stack chain/backtrace) recording: "
@@ -896,6 +1048,9 @@ const struct option record_options[] = {
 		    "sample by weight (on special events only)"),
 	OPT_BOOLEAN(0, "transaction", &record.opts.sample_transaction,
 		    "sample transaction flags (special events only)"),
+	OPT_CALLBACK(0, "out-pages", &record.mmap.out_pages, "pages",
+		     "Number of pages or size with units to use for output (default 64M)",
+		     perf_evlist__parse_mmap_pages),
 	OPT_END()
 };
 
-- 
1.8.3.4 (Apple Git-47)


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] perf record: mmap output file - v4
  2013-11-08  4:23 ` [PATCH 2/2] perf record: mmap output file - v4 David Ahern
@ 2013-11-08  9:34   ` Jiri Olsa
  2013-11-08 16:52     ` David Ahern
  2013-11-11 11:29   ` Ingo Molnar
  1 sibling, 1 reply; 11+ messages in thread
From: Jiri Olsa @ 2013-11-08  9:34 UTC (permalink / raw)
  To: David Ahern
  Cc: acme, linux-kernel, mingo, Frederic Weisbecker, Peter Zijlstra,
	Namhyung Kim, Mike Galbraith, Stephane Eranian

On Thu, Nov 07, 2013 at 09:23:25PM -0700, David Ahern wrote:

SNIP

>  
> +static void mmap_output_fini(struct perf_record *rec)
> +{
> +	off_t len;
> +	int fd;
> +
> +	if (!rec->mmap.use)
> +		return;
> +
> +	rec->mmap.use = false;
> +
> +	len = rec->session->header.data_offset + rec->bytes_written;
> +	fd = rec->file.fd;
> +
> +	munmap(rec->mmap.addr, rec->mmap.out_size);
> +	rec->mmap.addr = NULL;
> +
> +	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

jirka

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] perf record: mmap output file - v4
  2013-11-08  9:34   ` Jiri Olsa
@ 2013-11-08 16:52     ` David Ahern
  0 siblings, 0 replies; 11+ messages in thread
From: David Ahern @ 2013-11-08 16:52 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: acme, linux-kernel, mingo, Frederic Weisbecker, Peter Zijlstra,
	Namhyung Kim, Mike Galbraith, Stephane Eranian

[-- Attachment #1: Type: text/plain, Size: 1131 bytes --]

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




[-- Attachment #2: perf-record-handle-mmap-write-failure.patch --]
[-- Type: text/plain, Size: 2021 bytes --]

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) {

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] perf record: mmap output file - v4
  2013-11-08  4:23 ` [PATCH 2/2] perf record: mmap output file - v4 David Ahern
  2013-11-08  9:34   ` Jiri Olsa
@ 2013-11-11 11:29   ` Ingo Molnar
  2013-11-11 14:58     ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2013-11-11 11:29 UTC (permalink / raw)
  To: David Ahern
  Cc: acme, linux-kernel, jolsa, Frederic Weisbecker, Peter Zijlstra,
	Namhyung Kim, Mike Galbraith, Stephane Eranian


* David Ahern <dsahern@gmail.com> wrote:

> When recording raw_syscalls for the entire system, e.g.,
>     perf record -e raw_syscalls:*,sched:sched_switch -a -- sleep 1
> 
> you end up with a negative feedback loop as perf itself calls write() fairly
> often. This patch handles the problem by mmap'ing the file in chunks of 64M at
> a time and copies events from the event buffers to the file avoiding write
> system calls.
> 
> Before (with write syscall):
> 
>     perf record -o /tmp/perf.data -e raw_syscalls:*,sched:sched_switch -a -- sleep 1
>     [ perf record: Woken up 0 times to write data ]
>     [ perf record: Captured and wrote 81.843 MB /tmp/perf.data (~3575786 samples) ]
> 
> After (using mmap):
> 
>     perf record -o /tmp/perf.data -e raw_syscalls:*,sched:sched_switch -a -- sleep 1
>     [ perf record: Woken up 31 times to write data ]
>     [ perf record: Captured and wrote 8.203 MB /tmp/perf.data (~358388 samples) ]
> 
> In addition to perf-trace benefits using mmap lowers the overhead of
> perf-record. For example,
> 
>   perf stat -i -- perf record -g -o /tmp/perf.data openssl speed aes
> 
> shows a drop in time, CPU cycles, and instructions all drop by more than a
> factor of 3. Jiri also ran a test that showed a big improvement.
> 
> v4: Refactoring per Ingo's comments
> 
> v3: Removed use of bytes_at_mmap_start at the stat() that set it
>     Added user option to control the size of the mmap for writing file.
> 
> v2: Removed msync call before munmap per Jiri's suggestion
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Stephane Eranian <eranian@google.com>
> ---
>  tools/perf/Documentation/perf-record.txt |   5 +
>  tools/perf/builtin-record.c              | 155 +++++++++++++++++++++++++++++++
>  2 files changed, 160 insertions(+)

Looks very clean now!

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] perf record: mmap output file - v4
  2013-11-11 11:29   ` Ingo Molnar
@ 2013-11-11 14:58     ` Arnaldo Carvalho de Melo
  2013-11-11 15:17       ` David Ahern
  0 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-11-11 14:58 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: David Ahern, linux-kernel, jolsa, Frederic Weisbecker,
	Peter Zijlstra, Namhyung Kim, Mike Galbraith, Stephane Eranian

Em Mon, Nov 11, 2013 at 12:29:06PM +0100, Ingo Molnar escreveu:
> * David Ahern <dsahern@gmail.com> wrote:

> > shows a drop in time, CPU cycles, and instructions all drop by more than a
> > factor of 3. Jiri also ran a test that showed a big improvement.
> > 
> > v4: Refactoring per Ingo's comments
> > 
> > v3: Removed use of bytes_at_mmap_start at the stat() that set it
> >     Added user option to control the size of the mmap for writing file.
> > 
> > v2: Removed msync call before munmap per Jiri's suggestion

> Looks very clean now!
 
> Acked-by: Ingo Molnar <mingo@kernel.org>

Applied the prep patch, but waiting for v5 as stated by David when
answering Jiri's concerns.

- Arnaldo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] perf record: mmap output file - v4
  2013-11-11 14:58     ` Arnaldo Carvalho de Melo
@ 2013-11-11 15:17       ` David Ahern
  2013-11-11 20:41         ` Ingo Molnar
  0 siblings, 1 reply; 11+ messages in thread
From: David Ahern @ 2013-11-11 15:17 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Ingo Molnar
  Cc: linux-kernel, jolsa, Frederic Weisbecker, Peter Zijlstra,
	Namhyung Kim, Mike Galbraith, Stephane Eranian

On 11/11/13, 7:58 AM, Arnaldo Carvalho de Melo wrote:
>
>> Looks very clean now!
>
>> Acked-by: Ingo Molnar <mingo@kernel.org>
>
> Applied the prep patch, but waiting for v5 as stated by David when
> answering Jiri's concerns.

I'll re-send - giving some time for comments on the use of 
setjmp/longjmp to bounce out of memcpy and error exit when filesystem 
runs out of space.

David

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] perf record: mmap output file - v4
  2013-11-11 15:17       ` David Ahern
@ 2013-11-11 20:41         ` Ingo Molnar
  2013-11-11 20:44           ` David Ahern
  0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2013-11-11 20:41 UTC (permalink / raw)
  To: David Ahern
  Cc: Arnaldo Carvalho de Melo, linux-kernel, jolsa,
	Frederic Weisbecker, Peter Zijlstra, Namhyung Kim, Mike Galbraith,
	Stephane Eranian


* David Ahern <dsahern@gmail.com> wrote:

> On 11/11/13, 7:58 AM, Arnaldo Carvalho de Melo wrote:
> >
> >>Looks very clean now!
> >
> >>Acked-by: Ingo Molnar <mingo@kernel.org>
> >
> >Applied the prep patch, but waiting for v5 as stated by David when
> >answering Jiri's concerns.
> 
> I'll re-send - giving some time for comments on the use of 
> setjmp/longjmp to bounce out of memcpy and error exit when filesystem 
> runs out of space.

Maybe that could be an add-on patch instead?

All the rest would already be an improvement, right?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] perf record: mmap output file - v4
  2013-11-11 20:41         ` Ingo Molnar
@ 2013-11-11 20:44           ` David Ahern
  0 siblings, 0 replies; 11+ messages in thread
From: David Ahern @ 2013-11-11 20:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Arnaldo Carvalho de Melo, linux-kernel, jolsa,
	Frederic Weisbecker, Peter Zijlstra, Namhyung Kim, Mike Galbraith,
	Stephane Eranian

On 11/11/13, 1:41 PM, Ingo Molnar wrote:
>
> * David Ahern <dsahern@gmail.com> wrote:
>
>> On 11/11/13, 7:58 AM, Arnaldo Carvalho de Melo wrote:
>>>
>>>> Looks very clean now!
>>>
>>>> Acked-by: Ingo Molnar <mingo@kernel.org>
>>>
>>> Applied the prep patch, but waiting for v5 as stated by David when
>>> answering Jiri's concerns.
>>
>> I'll re-send - giving some time for comments on the use of
>> setjmp/longjmp to bounce out of memcpy and error exit when filesystem
>> runs out of space.
>
> Maybe that could be an add-on patch instead?

That's what Arnaldo and I were thinking.
>
> All the rest would already be an improvement, right?

Yes. The longjmp handles the out-of-space failure condition trying to 
write the data to a file. With mmap writes you get a SIGBUS when the 
filesystem is full and you need to bail out of the memcpy.

David

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tip:perf/urgent] perf record: Move existing write_output into helper function
  2013-11-08  4:23 ` [PATCH 1/2] perf record: Move existing write_output into helper function David Ahern
@ 2013-11-12 21:55   ` tip-bot for David Ahern
  0 siblings, 0 replies; 11+ messages in thread
From: tip-bot for David Ahern @ 2013-11-12 21:55 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, eranian, hpa, mingo, peterz, efault, namhyung,
	jolsa, fweisbec, dsahern, tglx

Commit-ID:  a9986fad6645b98d5bb3c2f83c22efb0761ca272
Gitweb:     http://git.kernel.org/tip/a9986fad6645b98d5bb3c2f83c22efb0761ca272
Author:     David Ahern <dsahern@gmail.com>
AuthorDate: Thu, 7 Nov 2013 21:23:24 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 11 Nov 2013 15:56:40 -0300

perf record: Move existing write_output into helper function

Code move only; no logic changes. In preparation for the mmap based
output option in the next patch.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1383884605-30968-2-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 8f5af32..880227e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -76,7 +76,7 @@ struct perf_record {
 	long			samples;
 };
 
-static int write_output(struct perf_record *rec, void *buf, size_t size)
+static int do_write_output(struct perf_record *rec, void *buf, size_t size)
 {
 	struct perf_data_file *file = &rec->file;
 
@@ -97,6 +97,11 @@ static int write_output(struct perf_record *rec, void *buf, size_t size)
 	return 0;
 }
 
+static int write_output(struct perf_record *rec, void *buf, size_t size)
+{
+	return do_write_output(rec, buf, size);
+}
+
 static int process_synthesized_event(struct perf_tool *tool,
 				     union perf_event *event,
 				     struct perf_sample *sample __maybe_unused,

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-11-12 21:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-08  4:23 [PATCH 0/2] perf: mmap output file - v4 David Ahern
2013-11-08  4:23 ` [PATCH 1/2] perf record: Move existing write_output into helper function David Ahern
2013-11-12 21:55   ` [tip:perf/urgent] " tip-bot for David Ahern
2013-11-08  4:23 ` [PATCH 2/2] perf record: mmap output file - v4 David Ahern
2013-11-08  9:34   ` Jiri Olsa
2013-11-08 16:52     ` David Ahern
2013-11-11 11:29   ` Ingo Molnar
2013-11-11 14:58     ` Arnaldo Carvalho de Melo
2013-11-11 15:17       ` David Ahern
2013-11-11 20:41         ` Ingo Molnar
2013-11-11 20:44           ` David Ahern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).