[GIT PULL 00/20] perf/core improvements

June 19th, 2012 - 02:10 pm ET by Arnaldo Carvalho de Melo | Report spam

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit e227051b13956b8f71c0abecc41ad351e64671c8:

uprobes: Remove the unnecessary initialization in add_utask() (2012-06-16 09:10:52 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/...acme/linux tags/perf-core-for-mingo

for you to fetch changes up to c0a58fb2bdf033df433cad9009c7dac4c6b872b0:

perf annotate: Check null of sym pointer before using it (2012-06-19 14:30:26 -0300)

-
perf/core improvements and fixes

. Replace event_name with perf_evsel__name, that handles the event modifiers
and doesn't use static variables.

. GTK browser improvements, from Namhyung Kim

. Fix possible NULL pointer deref in the TUI annotate browser, from Samuel Liao

. Add sort by source file:line number, using addr2line.

. Allow printing histogram text snapshots at any point in top/report.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

-
Arnaldo Carvalho de Melo (13):
perf tools: Add sort by src line/number
perf lib: Introduce rtrim
perf hists browser: Implement printing snapshots to files
perf evsel: Carve out event modifier formatting
perf tools: Reconstruct hw cache event with modifiers from perf_event_attr
perf tools: Reconstruct sw event with modifiers from perf_event_attr
perf evsel: Handle all event types in perf_evsel__name
perf tools: Move all users of event_name to perf_evsel__name
perf script: Replace __event_name uses with perf_evsel__name
perf tools: Don't access evsel->name directly
perf tools: Remove __event_name
perf evsel: Reconstruct raw event with modifiers from perf_event_attr
perf evsel: Make some methods private

Jiri Olsa (1):
perf tools: Remove unused evsel parameter from machine__resolve_callchain

Namhyung Kim (5):
perf ui: Introduce struct perf_error_ops
perf ui/gtk: Introduce struct perf_gtk_context
perf ui/gtk: Add GTK statusbar widget to browser window
perf ui/gtk: Add GTK info_bar widget to browser window
perf ui/gtk: Use struct perf_error_ops

Samuel Liao (1):
perf annotate: Check null of sym pointer before using it

tools/perf/Documentation/perf-report.txt | 2 +-
tools/perf/Documentation/perf-top.txt | 2 +-
tools/perf/Makefile | 6 +
tools/perf/builtin-evlist.c | 2 +-
tools/perf/builtin-record.c | 4 +-
tools/perf/builtin-report.c | 10 +-
tools/perf/builtin-sched.c | 2 +-
tools/perf/builtin-script.c | 42 ++
tools/perf/builtin-stat.c | 12 +-
tools/perf/builtin-test.c | 2 +-
tools/perf/builtin-top.c | 12 +-
tools/perf/config/feature-tests.mak | 13 ++
tools/perf/ui/browsers/annotate.c | 4 +-
tools/perf/ui/browsers/hists.c | 210 ++++++++++++++++++++--
tools/perf/ui/gtk/browser.c | 69 +++++++-
tools/perf/ui/gtk/gtk.h | 31 ++++
tools/perf/ui/gtk/setup.c | 5 +
tools/perf/ui/gtk/util.c | 129 ++++++++++++++
tools/perf/ui/tui/setup.c | 6 +
tools/perf/ui/tui/util.c | 243 ++++++++++++++++++++++++++
tools/perf/ui/util.c | 277 ++++++
tools/perf/ui/util.h | 9 +-
tools/perf/util/debug.c | 2 +-
tools/perf/util/debug.h | 23 ++-
tools/perf/util/evsel.c | 196 ++++++++++++++++++
tools/perf/util/evsel.h | 15 +-
tools/perf/util/header.c | 2 +-
tools/perf/util/hist.h | 1 +
tools/perf/util/map.h | 2 +-
tools/perf/util/parse-events-test.c | 4 +-
tools/perf/util/parse-events.c | 203 ++--
tools/perf/util/parse-events.h | 2 -
tools/perf/util/session.c | 9 +-
tools/perf/util/session.h | 4 +-
tools/perf/util/sort.c | 49 ++++++
tools/perf/util/sort.h | 2 +
tools/perf/util/string.c | 22 +++
tools/perf/util/top.c | 2 +-
tools/perf/util/util.h | 2 +
39 files changed, 1118 insertions(+), 514 deletions(-)
create mode 100644 tools/perf/ui/gtk/util.c
create mode 100644 tools/perf/ui/tui/util.c
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Follow the discussion Replies

16 replies Replies

Make a reply

Replies

#1 Arnaldo Carvalho de Melo

June 19th, 2012 - 02:10 pm ET | Report spam

From: Arnaldo Carvalho de Melo

Remove the trailing whitespaces.

Cc: David Ahern
Cc: Frederic Weisbecker
Cc: Jiri Olsa
Cc: Mike Galbraith
Cc: Namhyung Kim
Cc: Paul Mackerras
Cc: Peter Zijlstra
Cc: Stephane Eranian
Link: http://lkml.kernel.org/n/
Signed-off-by: Arnaldo Carvalho de Melo

tools/perf/util/string.c | 22 ++++++++++++++++++++++
tools/perf/util/util.h | 2 ++
2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index d583638..199bc4d 100644
a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -313,3 +313,25 @@ int strtailcmp(const char *s1, const char *s2)
return 0;
}

+/**
+ * rtrim - Removes trailing whitespace from @s.
+ * @s: The string to be stripped.
+ *
+ * Note that the first trailing whitespace is replaced with a %NUL-terminator
+ * in the given string @s. Returns @s.
+ */
+char *rtrim(char *s)
+{
+ size_t size = strlen(s);
+ char *end;
+
+ if (!size)
+ return s;
+
+ end = s + size - 1;
+ while (end >= s && isspace(*end))
+ end--;
+ *(end + 1) = '\0';
+
+ return s;
+}
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 2daaedb..b13c733 100644
a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -264,4 +264,6 @@ bool is_power_of_2(unsigned long n)

size_t hex_width(u64 v);

+char *rtrim(char *s);
+
#endif
1.7.1

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Reply to this message

#2 Arnaldo Carvalho de Melo

June 19th, 2012 - 02:10 pm ET | Report spam

From: Arnaldo Carvalho de Melo

Now that __event_name is gone, no need to export __perf_evsel__[hs]w_name().

Cc: David Ahern
Cc: Frederic Weisbecker
Cc: Jiri Olsa
Cc: Mike Galbraith
Cc: Namhyung Kim
Cc: Paul Mackerras
Cc: Peter Zijlstra
Cc: Stephane Eranian
Link: http://lkml.kernel.org/n/
Signed-off-by: Arnaldo Carvalho de Melo

tools/perf/util/evsel.c | 6 +++
tools/perf/util/evsel.h | 5 --
2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 8f0e9dd..876f639 100644
a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -78,7 +78,7 @@ static const char *perf_evsel__hw_names[PERF_COUNT_HW_MAX] = {
"ref-cycles",
};

-const char *__perf_evsel__hw_name(u64 config)
+static const char *__perf_evsel__hw_name(u64 config)
{
if (config < PERF_COUNT_HW_MAX && perf_evsel__hw_names[config])
return perf_evsel__hw_names[config];
@@ -140,7 +140,7 @@ static const char *perf_evsel__sw_names[PERF_COUNT_SW_MAX] = {
"emulation-faults",
};

-const char *__perf_evsel__sw_name(u64 config)
+static const char *__perf_evsel__sw_name(u64 config)
{
if (config < PERF_COUNT_SW_MAX && perf_evsel__sw_names[config])
return perf_evsel__sw_names[config];
@@ -219,7 +219,7 @@ int __perf_evsel__hw_cache_type_op_res_name(u8 type, u8 op, u8 result,
perf_evsel__hw_cache_op[op][1]);
}

-int __perf_evsel__hw_cache_name(u64 config, char *bf, size_t size)
+static int __perf_evsel__hw_cache_name(u64 config, char *bf, size_t size)
{
u8 op, result, type = (config >> 0) & 0xff;
const char *err = "unknown-ext-hardware-cache-type";
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index c936feb..67cc503 100644
a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -95,11 +95,6 @@ const char *perf_evsel__hw_cache_result[PERF_COUNT_HW_CACHE_RESULT_MAX]
[PERF_EVSEL__MAX_ALIASES];
int __perf_evsel__hw_cache_type_op_res_name(u8 type, u8 op, u8 result,
char *bf, size_t size);
-int __perf_evsel__hw_cache_name(u64 config, char *bf, size_t size);
-
-const char *__perf_evsel__hw_name(u64 config);
-const char *__perf_evsel__sw_name(u64 config);
-
const char *perf_evsel__name(struct perf_evsel *evsel);

int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
1.7.1

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Reply to this message

#3 Arnaldo Carvalho de Melo

June 19th, 2012 - 02:10 pm ET | Report spam

From: Arnaldo Carvalho de Melo

One needs to use perf_evsel__name() so that if needed the name gets
synthesized and stored in evsel->name, from where perf_evsel__name()
will serve from them on.

Cc: David Ahern
Cc: Frederic Weisbecker
Cc: Jiri Olsa
Cc: Mike Galbraith
Cc: Namhyung Kim
Cc: Paul Mackerras
Cc: Peter Zijlstra
Cc: Stephane Eranian
Link: http://lkml.kernel.org/n/
Signed-off-by: Arnaldo Carvalho de Melo

tools/perf/builtin-sched.c | 2 +-
tools/perf/util/parse-events-test.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index b125e07..9fe77b1 100644
a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1601,7 +1601,7 @@ static int perf_sched__process_tracepoint_sample(struct perf_tool *tool,

if (thread == NULL) {
pr_debug("problem processing %s event, skipping it.",
- evsel->name);
+ perf_evsel__name(evsel));
return -1;
}

diff --git a/tools/perf/util/parse-events-test.c b/tools/perf/util/parse-events-test.c
index 76b98e2..d0cf7c1 100644
a/tools/perf/util/parse-events-test.c
+++ b/tools/perf/util/parse-events-test.c
@@ -418,14 +418,14 @@ static int test__checkevent_pmu_name(struct perf_evlist *evlist)
TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->nr_entries);
TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->attr.type);
TEST_ASSERT_VAL("wrong config", 1 == evsel->attr.config);
- TEST_ASSERT_VAL("wrong name", !strcmp(evsel->name, "krava"));
+ TEST_ASSERT_VAL("wrong name", !strcmp(perf_evsel__name(evsel), "krava"));

/* cpu/config=2/" */
evsel = list_entry(evsel->node.next, struct perf_evsel, node);
TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->nr_entries);
TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->attr.type);
TEST_ASSERT_VAL("wrong config", 2 == evsel->attr.config);
- TEST_ASSERT_VAL("wrong name", !strcmp(evsel->name, "raw 0x2"));
+ TEST_ASSERT_VAL("wrong name", !strcmp(perf_evsel__name(evsel), "raw 0x2"));

return 0;
}
1.7.1

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Reply to this message

#4 Arnaldo Carvalho de Melo

June 19th, 2012 - 02:10 pm ET | Report spam

From: Arnaldo Carvalho de Melo

Not needed anymore, the parsing code can just leave evsel->name as NULL
and the first call to perf_evsel__name() will do exactly what was being
pre-cached using __event_name().

Cc: David Ahern
Cc: Frederic Weisbecker
Cc: Jiri Olsa
Cc: Mike Galbraith
Cc: Namhyung Kim
Cc: Paul Mackerras
Cc: Peter Zijlstra
Cc: Stephane Eranian
Link: http://lkml.kernel.org/n/
Signed-off-by: Arnaldo Carvalho de Melo

tools/perf/util/parse-events.c | 61 ++++
tools/perf/util/parse-events.h | 1 -
2 files changed, 6 insertions(+), 56 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 517e647..eacf932 100644
a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -161,24 +161,6 @@ struct tracepoint_path *tracepoint_id_to_path(u64 config)
return NULL;
}

-#define TP_PATH_LEN (MAX_EVENT_LENGTH * 2 + 1)
-static const char *tracepoint_id_to_name(u64 config)
-{
- static char buf[TP_PATH_LEN];
- struct tracepoint_path *path;
-
- path = tracepoint_id_to_path(config);
- if (path) {
- snprintf(buf, TP_PATH_LEN, "%s:%s", path->system, path->name);
- free(path->name);
- free(path->system);
- free(path);
- } else
- snprintf(buf, TP_PATH_LEN, "%s:%s", "unknown", "unknown");
-
- return buf;
-}
-
const char *event_type(int type)
{
switch (type) {
@@ -201,36 +183,6 @@ const char *event_type(int type)
return "unknown";
}

-const char *__event_name(int type, u64 config)
-{
- static char buf[32];
-
- if (type == PERF_TYPE_RAW) {
- sprintf(buf, "raw 0x%" PRIx64, config);
- return buf;
- }
-
- switch (type) {
- case PERF_TYPE_HARDWARE:
- return __perf_evsel__hw_name(config);
-
- case PERF_TYPE_HW_CACHE:
- __perf_evsel__hw_cache_name(config, buf, sizeof(buf));
- return buf;
-
- case PERF_TYPE_SOFTWARE:
- return __perf_evsel__sw_name(config);
-
- case PERF_TYPE_TRACEPOINT:
- return tracepoint_id_to_name(config);
-
- default:
- break;
- }
-
- return "unknown";
-}
-
static int add_event(struct list_head **_list, int *idx,
struct perf_event_attr *attr, char *name)
{
@@ -252,7 +204,8 @@ static int add_event(struct list_head **_list, int *idx,
return -ENOMEM;
}

- evsel->name = strdup(name);
+ if (name)
+ evsel->name = strdup(name);
list_add_tail(&evsel->node, list);
*_list = list;
return 0;
@@ -545,8 +498,7 @@ int parse_events_add_numeric(struct list_head **list, int *idx,
config_attr(&attr, head_config, 1))
return -EINVAL;

- return add_event(list, idx, &attr,
- (char *) __event_name(type, config));
+ return add_event(list, idx, &attr, NULL);
}

static int parse_events__is_name_term(struct parse_events__term *term)
@@ -554,8 +506,7 @@ static int parse_events__is_name_term(struct parse_events__term *term)
return term->type_term == PARSE_EVENTS__TERM_TYPE_NAME;
}

-static char *pmu_event_name(struct perf_event_attr *attr,
- struct list_head *head_terms)
+static char *pmu_event_name(struct list_head *head_terms)
{
struct parse_events__term *term;

@@ -563,7 +514,7 @@ static char *pmu_event_name(struct perf_event_attr *attr,
if (parse_events__is_name_term(term))
return term->val.str;

- return (char *) __event_name(PERF_TYPE_RAW, attr->config);
+ return NULL;
}

int parse_events_add_pmu(struct list_head **list, int *idx,
@@ -588,7 +539,7 @@ int parse_events_add_pmu(struct list_head **list, int *idx,
return -EINVAL;

return add_event(list, idx, &attr,
- pmu_event_name(&attr, head_config));
+ pmu_event_name(head_config));
}

void parse_events_update_lists(struct list_head *list_event,
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index d7eb070..1784f06 100644
a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -26,7 +26,6 @@ extern struct tracepoint_path *tracepoint_id_to_path(u64 config);
extern bool have_tracepoints(struct list_head *evlist);

const char *event_type(int type);
-extern const char *__event_name(int type, u64 config);

extern int parse_events_option(const struct option *opt, const char *str,
int unset);
1.7.1

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Reply to this message

#5 Arnaldo Carvalho de Melo

June 19th, 2012 - 02:10 pm ET | Report spam

From: Namhyung Kim

The GtkInfoBar is a modern UI component to display messages without
bothering the main window. It'll be used for showing a warning message.

As the GtkInfoBar requires 2.18 (or newer) version of GTK+ library, add
availability check to Makefile too.

Suggested-by: Sunjin Yang
Signed-off-by: Namhyung Kim
Acked-by: Pekka Enberg
Cc: Paul Mackerras
Cc: Pekka Enberg
Cc: Peter Zijlstra
Link: http://lkml.kernel.org/r/
Signed-off-by: Arnaldo Carvalho de Melo

tools/perf/Makefile | 3 +++
tools/perf/config/feature-tests.mak | 13 +++++++++++++
tools/perf/ui/gtk/browser.c | 33 +++++++++++++++++++++++++++++++++
tools/perf/ui/gtk/gtk.h | 12 ++++++++++++
4 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index c0ee917..d698c11 100644
a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -523,6 +523,9 @@ else
msg := $(warning GTK2 not found, disables GTK2 support. Please install gtk2-devel or libgtk2.0-dev);
BASIC_CFLAGS += -DNO_GTK2_SUPPORT
else
+ ifeq ($(call try-cc,$(SOURCE_GTK2_INFOBAR),$(FLAGS_GTK2)),y)
+ BASIC_CFLAGS += -DHAVE_GTK_INFO_BAR
+ endif
BASIC_CFLAGS += $(shell pkg-config --cflags gtk+-2.0)
EXTLIBS += $(shell pkg-config --libs gtk+-2.0)
LIB_OBJS += $(OUTPUT)ui/gtk/browser.o
diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-tests.mak
index d9084e0..6c18785 100644
a/tools/perf/config/feature-tests.mak
+++ b/tools/perf/config/feature-tests.mak
@@ -78,6 +78,19 @@ int main(int argc, char *argv[])
return 0;
}
endef
+
+define SOURCE_GTK2_INFOBAR
+#pragma GCC diagnostic ignored \"-Wstrict-prototypes\"
+#include <gtk/gtk.h>
+#pragma GCC diagnostic error \"-Wstrict-prototypes\"
+
+int main(void)
+{
+ gtk_info_bar_new();
+
+ return 0;
+}
+endef
endif

ifndef NO_LIBPERL
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index ece360d..fd41e8d 100644
a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -122,6 +122,34 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
gtk_container_add(GTK_CONTAINER(window), view);
}

+#ifdef HAVE_GTK_INFO_BAR
+static GtkWidget *perf_gtk__setup_info_bar(void)
+{
+ GtkWidget *info_bar;
+ GtkWidget *label;
+ GtkWidget *content_area;
+
+ info_bar = gtk_info_bar_new();
+ gtk_widget_set_no_show_all(info_bar, TRUE);
+
+ label = gtk_label_new("");
+ gtk_widget_show(label);
+
+ content_area = gtk_info_bar_get_content_area(GTK_INFO_BAR(info_bar));
+ gtk_container_add(GTK_CONTAINER(content_area), label);
+
+ gtk_info_bar_add_button(GTK_INFO_BAR(info_bar), GTK_STOCK_OK,
+ GTK_RESPONSE_OK);
+ g_signal_connect(info_bar, "response",
+ G_CALLBACK(gtk_widget_hide), NULL);
+
+ pgctx->info_bar = info_bar;
+ pgctx->message_label = label;
+
+ return info_bar;
+}
+#endif
+
static GtkWidget *perf_gtk__setup_statusbar(void)
{
GtkWidget *stbar;
@@ -145,6 +173,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
struct perf_evsel *pos;
GtkWidget *vbox;
GtkWidget *notebook;
+ GtkWidget *info_bar;
GtkWidget *statbar;
GtkWidget *window;

@@ -189,6 +218,10 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,

gtk_box_pack_start(GTK_BOX(vbox), notebook, TRUE, TRUE, 0);

+ info_bar = perf_gtk__setup_info_bar();
+ if (info_bar)
+ gtk_box_pack_start(GTK_BOX(vbox), info_bar, FALSE, FALSE, 0);
+
statbar = perf_gtk__setup_statusbar();
gtk_box_pack_start(GTK_BOX(vbox), statbar, FALSE, FALSE, 0);

diff --git a/tools/perf/ui/gtk/gtk.h b/tools/perf/ui/gtk/gtk.h
index 2061678..a4d0f2b 100644
a/tools/perf/ui/gtk/gtk.h
+++ b/tools/perf/ui/gtk/gtk.h
@@ -10,6 +10,11 @@

struct perf_gtk_context {
GtkWidget *main_window;
+
+#ifdef HAVE_GTK_INFO_BAR
+ GtkWidget *info_bar;
+ GtkWidget *message_label;
+#endif
GtkWidget *statbar;
guint statbar_ctx_id;
};
@@ -24,4 +29,11 @@ static inline bool perf_gtk__is_active_context(struct perf_gtk_context *ctx)
struct perf_gtk_context *perf_gtk__activate_context(GtkWidget *window);
int perf_gtk__deactivate_context(struct perf_gtk_context **ctx);

+#ifndef HAVE_GTK_INFO_BAR
+static inline GtkWidget *perf_gtk__setup_info_bar(void)
+{
+ return NULL;
+}
+#endif
+
#endif /* _PERF_GTK_H_ */
1.7.1

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Reply to this message

Create a new topicNext page

Make a reply

12 3 4

Make your own search

[GIT PULL 00/20] perf/core improvements

Similar topics

Replies