From b3230c8b44da5838cf396942d5c1ab19f8e8f720 Mon Sep 17 00:00:00 2001 From: Steven Rostedt Date: Fri, 21 May 2010 11:55:21 -0400 Subject: ring-buffer: Reset "real_end" when page is filled The code to store the "lost events" requires knowing the real end of the page. Since the 'commit' includes the padding at the end of a page a "real_end" variable was used to keep track of the end not including the padding. If events were lost, the reader can place the count of events in the padded area if there is enough room. The bug this patch fixes is that when we fill the page we do not reset the real_end variable, and if the writer had wrapped a few times, the real_end would be incorrect. This patch simply resets the real_end if the page was filled. Signed-off-by: Steven Rostedt --- kernel/trace/ring_buffer.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index 7f6059c5aa9..b0702ff7821 100644 --- a/kernel/trace/ring_buffer.c +++ b/kernel/trace/ring_buffer.c @@ -1768,6 +1768,14 @@ rb_reset_tail(struct ring_buffer_per_cpu *cpu_buffer, * must fill the old tail_page with padding. */ if (tail >= BUF_PAGE_SIZE) { + /* + * If the page was filled, then we still need + * to update the real_end. Reset it to zero + * and the reader will ignore it. + */ + if (tail == BUF_PAGE_SIZE) + tail_page->real_end = 0; + local_sub(length, &tail_page->write); return; } -- cgit v1.2.3 From 2711ca237a084286ea1c2dcf82ab2aadab23a00d Mon Sep 17 00:00:00 2001 From: Steven Rostedt Date: Fri, 21 May 2010 13:32:26 -0400 Subject: ring-buffer: Move zeroing out excess in page to ring buffer code Currently the trace splice code zeros out the excess bytes in the page before sending it off to userspace. This is to make sure userspace is not getting anything it should not be when reading the pages, because the excess data was never initialized to zero before writing (for perfomance reasons). But the splice code has no business in doing this work, it should be done by the ring buffer. With the latest changes for recording lost events, the splice code gets it wrong anyway. Move the zeroing out of excess bytes into the ring buffer code. Signed-off-by: Steven Rostedt --- kernel/trace/ring_buffer.c | 11 +++++++++-- kernel/trace/trace.c | 6 ------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index b0702ff7821..1da7b6ea8b8 100644 --- a/kernel/trace/ring_buffer.c +++ b/kernel/trace/ring_buffer.c @@ -3902,12 +3902,12 @@ int ring_buffer_read_page(struct ring_buffer *buffer, ret = read; cpu_buffer->lost_events = 0; + + commit = local_read(&bpage->commit); /* * Set a flag in the commit field if we lost events */ if (missed_events) { - commit = local_read(&bpage->commit); - /* If there is room at the end of the page to save the * missed events, then record it there. */ @@ -3915,10 +3915,17 @@ int ring_buffer_read_page(struct ring_buffer *buffer, memcpy(&bpage->data[commit], &missed_events, sizeof(missed_events)); local_add(RB_MISSED_STORED, &bpage->commit); + commit += sizeof(missed_events); } local_add(RB_MISSED_EVENTS, &bpage->commit); } + /* + * This page may be off to user land. Zero it out here. + */ + if (commit < BUF_PAGE_SIZE) + memset(&bpage->data[commit], 0, BUF_PAGE_SIZE - commit); + out_unlock: spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags); diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index ba0ec81158b..95d0b1a28f9 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -3661,7 +3661,6 @@ tracing_buffers_read(struct file *filp, char __user *ubuf, size_t count, loff_t *ppos) { struct ftrace_buffer_info *info = filp->private_data; - unsigned int pos; ssize_t ret; size_t size; @@ -3688,11 +3687,6 @@ tracing_buffers_read(struct file *filp, char __user *ubuf, if (ret < 0) return 0; - pos = ring_buffer_page_len(info->spare); - - if (pos < PAGE_SIZE) - memset(info->spare + pos, 0, PAGE_SIZE - pos); - read: size = PAGE_SIZE - info->read; if (size > count) -- cgit v1.2.3 From 62e3436b5f3461662929eae102beefbd12127cb1 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Wed, 26 May 2010 13:22:26 -0300 Subject: perf tui: Reset use_browser if stdout is not a tty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The newt initialization routines weren't being called because the output was a file (perf annotate > /tmp/bla) but use_browser was still 1, because ~/.perfconfig had it as 'on', so, later on newt routines segfaulted. Cc: Frédéric Weisbecker Cc: Mike Galbraith Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Tom Zanussi LKML-Reference: Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-annotate.c | 2 +- tools/perf/util/newt.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 08278eda31a..96db5248e99 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -343,7 +343,7 @@ find_next: continue; } - if (use_browser) { + if (use_browser > 0) { key = hist_entry__tui_annotate(he); if (is_exit_key(key)) break; diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c index d54c540f49d..cf182ca132f 100644 --- a/tools/perf/util/newt.c +++ b/tools/perf/util/newt.c @@ -1139,6 +1139,7 @@ void setup_browser(void) struct newtPercentTreeColors *c = &defaultPercentTreeColors; if (!isatty(1) || !use_browser || dump_trace) { + use_browser = 0; setup_pager(); return; } -- cgit v1.2.3 From 5ad90e4ea4a096af9f0a362e34dfae5686a191ef Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Wed, 26 May 2010 13:26:02 -0300 Subject: perf symbols: Add the build id cache to the vmlinux path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So that if the kernel DSO has a build id because record inserted it in the perf.data build id table in the header, or a BUILD_ID event was inserted in the stream, we first look at the build id cache ($HOME/.debug/). If we find it there, try to use it, allowing offline annotation in addition to 'perf report'. Reported-by: Stephane Eranian Cc: Frédéric Weisbecker Cc: Mike Galbraith Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Tom Zanussi LKML-Reference: Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-top.c | 2 +- tools/perf/util/symbol.c | 27 +++++++++++++++++++++++---- tools/perf/util/symbol.h | 2 +- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index 397290a0a76..a66f4272b99 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -1060,7 +1060,7 @@ static void event__process_sample(const event_t *self, pr_err("Can't annotate %s", sym->name); if (sym_filter_entry->map->dso->origin == DSO__ORIG_KERNEL) { pr_err(": No vmlinux file was found in the path:\n"); - vmlinux_path__fprintf(stderr); + machine__fprintf_vmlinux_path(machine, stderr); } else pr_err(".\n"); exit(1); diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index aaa51ba147d..7fd6b151feb 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -1695,9 +1695,20 @@ int dso__load_vmlinux_path(struct dso *self, struct map *map, symbol_filter_t filter) { int i, err = 0; + char *filename; pr_debug("Looking at the vmlinux_path (%d entries long)\n", - vmlinux_path__nr_entries); + vmlinux_path__nr_entries + 1); + + filename = dso__build_id_filename(self, NULL, 0); + if (filename != NULL) { + err = dso__load_vmlinux(self, map, filename, filter); + if (err > 0) { + dso__set_long_name(self, filename); + goto out; + } + free(filename); + } for (i = 0; i < vmlinux_path__nr_entries; ++i) { err = dso__load_vmlinux(self, map, vmlinux_path[i], filter); @@ -1706,7 +1717,7 @@ int dso__load_vmlinux_path(struct dso *self, struct map *map, break; } } - +out: return err; } @@ -2102,13 +2113,21 @@ out_fail: return -1; } -size_t vmlinux_path__fprintf(FILE *fp) +size_t machine__fprintf_vmlinux_path(struct machine *self, FILE *fp) { int i; size_t printed = 0; + struct dso *kdso = self->vmlinux_maps[MAP__FUNCTION]->dso; + + if (kdso->has_build_id) { + char filename[PATH_MAX]; + if (dso__build_id_filename(kdso, filename, sizeof(filename))) + printed += fprintf(fp, "[0] %s\n", filename); + } for (i = 0; i < vmlinux_path__nr_entries; ++i) - printed += fprintf(fp, "[%d] %s\n", i, vmlinux_path[i]); + printed += fprintf(fp, "[%d] %s\n", + i + kdso->has_build_id, vmlinux_path[i]); return printed; } diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h index 5d25b5eb145..5e02d2c1715 100644 --- a/tools/perf/util/symbol.h +++ b/tools/perf/util/symbol.h @@ -216,6 +216,6 @@ int machines__create_guest_kernel_maps(struct rb_root *self); int symbol__init(void); bool symbol_type__is_a(char symbol_type, enum map_type map_type); -size_t vmlinux_path__fprintf(FILE *fp); +size_t machine__fprintf_vmlinux_path(struct machine *self, FILE *fp); #endif /* __PERF_SYMBOL */ -- cgit v1.2.3 From c4fe52a8ee730ed340eba8fe6ccbf26347ebe9aa Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 27 May 2010 09:53:40 -0300 Subject: perf tui: Fix last use_browser problem related to .perfconfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we moved to using ~/.perfconfig to set the value of use_browser, it changed from a boolean to an int so that the convention used for use_pager was followed. That convention is: -1: unspecified, that is what use_{browser,pager} is initialized 0: Don't use the browser (should be TUI), because was explicitely set to 0/off/false on ~/.perfconfig [tui] cmd =, or because we're redirecting the stdout to a file or piping it to some other command (!isatty()). 1: Use the TUI Some code was not properly audited and continued testing it as a boolean, this seems to be the last one. Reported-by: Frédéric Weisbecker Tested-by: Frédéric Weisbecker Cc: Frédéric Weisbecker Cc: Mike Galbraith Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Tom Zanussi LKML-Reference: Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c index dd824cf3b62..6cddff2bc97 100644 --- a/tools/perf/util/debug.c +++ b/tools/perf/util/debug.c @@ -22,7 +22,7 @@ int eprintf(int level, const char *fmt, ...) if (verbose >= level) { va_start(args, fmt); - if (use_browser) + if (use_browser > 0) ret = browser__show_help(fmt, args); else ret = vfprintf(stderr, fmt, args); -- cgit v1.2.3