Skip to contentSkip to conversation

perf/core improvements and fixes

GIT PULL New Arnaldo Carvalho de Melo 2 participants 13 messages Feb 23, 2016 – Feb 24, 2016

Subsystems (2)
lore
Share & tools
lore

Arnaldo Carvalho de Melo

author maintainer
#0 lore raw

Hi Ingo,

Please consider pulling, this is on top of the perf-core-for-mingo
submitted recently,

- Arnaldo

The following changes since commit 03e0a7df3efd959e40cd7ff40b1fabddc234ec5a:

perf tools: Introduce bpf-output event (2016-02-22 14:37:21 -0300)

are available in the git repository at:

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

for you to fetch changes up to bea2400621836b028d82c3d6a74053921d70dbd7:

perf tools: Remove strbuf_{remove,splice}() (2016-02-23 16:21:04 -0300)

----------------------------------------------------------------
perf/core improvements and fixes:

User visible:

- Don't stop PMU parsing on alias parse error, allowing the addition of new sysfs PMU files without breaking old tools (Andi Kleen)

- Implement '%' operation in libtraceevent (Daniel Bristot de Oliveira)

- Allow specifying events via -e in 'perf mem record', also listing what events can be specified via 'perf mem record -e list' (Jiri Olsa)

- Improve support to 'data_src', 'weight' and 'addr' fields in 'perf script' (Jiri Olsa)

Infrastructure:

- Export cacheline routines (Jiri Olsa)

- Remove strbuf_{remove,splice}(), dead code (Arnaldo Carvalho de Melo)

----------------------------------------------------------------
Andi Kleen (1):
perf tools: Dont stop PMU parsing on alias parse error

Arnaldo Carvalho de Melo (2):
perf help: No need to use strbuf_remove()
perf tools: Remove strbuf_{remove,splice}()

Daniel Bristot de Oliveira (1):
tools lib traceevent: Implement '%' operation

Jiri Olsa (7):
perf tools: Make cl_address global
perf tools: Introduce cl_offset function
perf tools: Add monitored events array
perf mem: Add -e record option
perf tools: Use ARRAY_SIZE in mem sort display functions perf script: Add data_src and weight column definitions perf script: Display addr/data_src/weight columns for raw events

 tools/lib/traceevent/event-parse.c |  4 +++
 tools/perf/builtin-help.c          |  3 +-
 tools/perf/builtin-mem.c           | 74 +++++++++++++++++++++++++++++++++-----
 tools/perf/builtin-script.c        | 23 +++++++++++-
 tools/perf/util/Build              |  1 +
 tools/perf/util/mem-events.c       | 51 ++++++++++++++++++++++++++
 tools/perf/util/mem-events.h       | 22 ++++++++++++
 tools/perf/util/pmu.c              | 15 ++++----
 tools/perf/util/sort.c             | 15 ++------
 tools/perf/util/sort.h             | 11 ++++++
 tools/perf/util/strbuf.c           | 24 -------------
 tools/perf/util/strbuf.h           |  2 --
 12 files changed, 188 insertions(+), 57 deletions(-)
 create mode 100644 tools/perf/util/mem-events.c
 create mode 100644 tools/perf/util/mem-events.h
author maintainer
#1 lore raw

PATCH 1/11 tools lib traceevent: Implement '%' operation

From: Daniel Bristot de Oliveira <[email protected]>

The operation '%' is not implemented on event-parse.c, causing an error when parsing events with '%' the operation in its printk format. For example,

  # perf record -e sched:sched_deadline_yield ~/yield-test
    Warning: [sched:sched_deadline_yield] unknown op '%'
  ....
  # perf script
    Warning: [sched:sched_deadline_yield] unknown op '%'
        test  1641 [006]  3364.109319: sched:sched_deadline_yield: \
                        [FAILED TO PARSE] now=3364109314595        \
                        deadline=3364139295135 runtime=19975597

This patch implements the '%' operation. With this patch, we see the correct output:

  # perf record -e sched:sched_deadline_yield ~/yield-test
    No Warning
  # perf script
        yield-test  4005 [001]  4623.650978: sched:sched_deadline_yield: \
                now=4623.650974050                                       \
                deadline=4623.680957364 remaining_runtime=19979611
---
 tools/lib/traceevent/event-parse.c | 4 ++++
 1 file changed, 4 insertions(+)
tools/lib/traceevent/event-parse.c +4 -0
index c3bd294a63d1..575e75174087 100644
@@ -1951,6 +1951,7 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
 		   strcmp(token, "*") == 0 ||
 		   strcmp(token, "^") == 0 ||
 		   strcmp(token, "/") == 0 ||
+		   strcmp(token, "%") == 0 ||
 		   strcmp(token, "<") == 0 ||
 		   strcmp(token, ">") == 0 ||
 		   strcmp(token, "<=") == 0 ||
@@ -3689,6 +3690,9 @@ eval_num_arg(void *data, int size, struct event_format *event, struct print_arg
 		case '/':
 			val = left / right;
 			break;
+		case '%':
+			val = left % right;
+			break;
 		case '*':
 			val = left * right;
 			break;
Signature · 1 line
2.5.0
author maintainer
#2 lore raw

PATCH 2/11 perf tools: Make cl_address global

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

It'll be used in following patches.

---
 tools/perf/util/sort.c | 6 ------
 tools/perf/util/sort.h | 5 +++++
 2 files changed, 5 insertions(+), 6 deletions(-)
tools/perf/util/sort.c +0 -6
index 6d0f85894f38..5f94ee740c5b 100644
@@ -1015,12 +1015,6 @@ static int hist_entry__snoop_snprintf(struct hist_entry *he, char *bf,
 	return repsep_snprintf(bf, size, "%-*s", width, out);
 }
 
-static inline  u64 cl_address(u64 address)
-{
-	/* return the cacheline of the address */
-	return (address & ~(cacheline_size - 1));
-}
-
 static int64_t
 sort__dcacheline_cmp(struct hist_entry *left, struct hist_entry *right)
 {
tools/perf/util/sort.h +5 -0
index 89a1273fd2da..46f159f9d947 100644
@@ -162,6 +162,11 @@ static inline float hist_entry__get_percent_limit(struct hist_entry *he)
 	return period * 100.0 / total_period;
 }
 
+static inline u64 cl_address(u64 address)
+{
+	/* return the cacheline of the address */
+	return (address & ~(cacheline_size - 1));
+}
 
 enum sort_mode {
 	SORT_MODE__NORMAL,
Signature · 1 line
2.5.0
author maintainer
#3 lore raw

PATCH 3/11 perf tools: Introduce cl_offset function

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

It'll be used in following patches.

---
 tools/perf/util/sort.h | 6 ++++++
 1 file changed, 6 insertions(+)
tools/perf/util/sort.h +6 -0
index 46f159f9d947..5b9c6246de6d 100644
@@ -168,6 +168,12 @@ static inline u64 cl_address(u64 address)
 	return (address & ~(cacheline_size - 1));
 }
 
+static inline u64 cl_offset(u64 address)
+{
+	/* return the cacheline of the address */
+	return (address & (cacheline_size - 1));
+}
+
 enum sort_mode {
 	SORT_MODE__NORMAL,
 	SORT_MODE__BRANCH,
Signature · 1 line
2.5.0
author maintainer
#4 lore raw

PATCH 4/11 perf tools: Add monitored events array

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

It will ease up configuration of memory events and addition of other memory events in following patches.

---
 tools/perf/builtin-mem.c     | 17 +++++++++--------
 tools/perf/util/Build        |  1 +
 tools/perf/util/mem-events.c | 10 ++++++++++
 tools/perf/util/mem-events.h | 19 +++++++++++++++++++
 4 files changed, 39 insertions(+), 8 deletions(-)
 create mode 100644 tools/perf/util/mem-events.c
 create mode 100644 tools/perf/util/mem-events.h
tools/perf/builtin-mem.c +9 -8
index 390170041696..36c56a4b778b 100644
@@ -6,6 +6,7 @@
 #include "util/tool.h"
 #include "util/session.h"
 #include "util/data.h"
+#include "util/mem-events.h"
 
 #define MEM_OPERATION_LOAD	0x1
 #define MEM_OPERATION_STORE	0x2
@@ -34,20 +35,20 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
 
 	rec_argv[i++] = "record";
 
-	if (mem->operation & MEM_OPERATION_LOAD)
+	if (mem->operation & MEM_OPERATION_LOAD) {
+		perf_mem_events[PERF_MEM_EVENTS__LOAD].record = true;
 		rec_argv[i++] = "-W";
+	}
 
 	rec_argv[i++] = "-d";
 
-	if (mem->operation & MEM_OPERATION_LOAD) {
-		rec_argv[i++] = "-e";
-		rec_argv[i++] = "cpu/mem-loads/pp";
-	}
+	for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
+		if (!perf_mem_events[j].record)
+			continue;
 
-	if (mem->operation & MEM_OPERATION_STORE) {
 		rec_argv[i++] = "-e";
-		rec_argv[i++] = "cpu/mem-stores/pp";
-	}
+		rec_argv[i++] = perf_mem_events[j].name;
+	};
 
 	for (j = 1; j < argc; j++, i++)
 		rec_argv[i] = argv[j];
tools/perf/util/Build +1 -0
index a34752d28488..df2b690970ac 100644
@@ -82,6 +82,7 @@ libperf-y += parse-branch-options.o
 libperf-y += parse-regs-options.o
 libperf-y += term.o
 libperf-y += help-unknown-cmd.o
+libperf-y += mem-events.o
 
 libperf-$(CONFIG_LIBBPF) += bpf-loader.o
 libperf-$(CONFIG_BPF_PROLOGUE) += bpf-prologue.o
tools/perf/util/mem-events.c new file +10 -0
new file mode 100644
index 000000000000..c6ba0a19ec9a
@@ -0,0 +1,10 @@
+#include "mem-events.h"
+
+#define E(n) { .name = n }
+
+struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
+	E("cpu/mem-loads,ldlat=30/P"),
+	E("cpu/mem-stores/P"),
+};
+
+#undef E
tools/perf/util/mem-events.h new file +19 -0
new file mode 100644
index 000000000000..c97b21465fa9
@@ -0,0 +1,19 @@
+#ifndef __PERF_MEM_EVENTS_H
+#define __PERF_MEM_EVENTS_H
+
+#include <stdbool.h>
+
+struct perf_mem_event {
+	bool		record;
+	const char	*name;
+};
+
+enum {
+	PERF_MEM_EVENTS__LOAD,
+	PERF_MEM_EVENTS__STORE,
+	PERF_MEM_EVENTS__MAX,
+};
+
+extern struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX];
+
+#endif /* __PERF_MEM_EVENTS_H */
Signature · 1 line
2.5.0
author maintainer
#5 lore raw

PATCH 5/11 perf mem: Add -e record option

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Adding -e option for perf mem record command, to be able to specify memory event directly.

Get list of available events:

  $ perf mem record -e list
  ldlat-loads
  ldlat-stores

Monitor ldlat-loads:

  $ perf mem record -e ldlat-loads true

Committer notes:

Further testing:

  # perf mem record -e ldlat-loads true
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.020 MB perf.data (10 samples) ]
  # perf evlist
  cpu/mem-loads,ldlat=30/P
  #
---
 tools/perf/builtin-mem.c     | 63 +++++++++++++++++++++++++++++++++++++++++---
 tools/perf/util/mem-events.c | 47 ++++++++++++++++++++++++++++++---
 tools/perf/util/mem-events.h |  3 +++
 3 files changed, 106 insertions(+), 7 deletions(-)
tools/perf/builtin-mem.c +59 -4
index 36c56a4b778b..b3f8a89ede40 100644
@@ -7,6 +7,7 @@
 #include "util/session.h"
 #include "util/data.h"
 #include "util/mem-events.h"
+#include "util/debug.h"
 
 #define MEM_OPERATION_LOAD	0x1
 #define MEM_OPERATION_STORE	0x2
@@ -22,11 +23,55 @@ struct perf_mem {
 	DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
 };
 
+static int parse_record_events(const struct option *opt,
+			       const char *str, int unset __maybe_unused)
+{
+	struct perf_mem *mem = *(struct perf_mem **)opt->value;
+	int j;
+
+	if (strcmp(str, "list")) {
+		if (!perf_mem_events__parse(str)) {
+			mem->operation = 0;
+			return 0;
+		}
+		exit(-1);
+	}
+
+	for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
+		struct perf_mem_event *e = &perf_mem_events[j];
+
+		fprintf(stderr, "%-20s%s",
+			e->tag, verbose ? "" : "\n");
+		if (verbose)
+			fprintf(stderr, " [%s]\n", e->name);
+	}
+	exit(0);
+}
+
+static const char * const __usage[] = {
+	"perf mem record [<options>] [<command>]",
+	"perf mem record [<options>] -- <command> [<options>]",
+	NULL
+};
+
+static const char * const *record_mem_usage = __usage;
+
 static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
 {
 	int rec_argc, i = 0, j;
 	const char **rec_argv;
 	int ret;
+	struct option options[] = {
+	OPT_CALLBACK('e', "event", &mem, "event",
+		     "event selector. use 'perf mem record -e list' to list available events",
+		     parse_record_events),
+	OPT_INCR('v', "verbose", &verbose,
+		 "be more verbose (show counter open errors, etc)"),
+	OPT_END()
+	};
+
+	argc = parse_options(argc, argv, options, record_mem_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
 
 	rec_argc = argc + 7; /* max number of arguments */
 	rec_argv = calloc(rec_argc + 1, sizeof(char *));
@@ -35,10 +80,11 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
 
 	rec_argv[i++] = "record";
 
-	if (mem->operation & MEM_OPERATION_LOAD) {
+	if (mem->operation & MEM_OPERATION_LOAD)
 		perf_mem_events[PERF_MEM_EVENTS__LOAD].record = true;
+
+	if (perf_mem_events[PERF_MEM_EVENTS__LOAD].record)
 		rec_argv[i++] = "-W";
-	}
 
 	rec_argv[i++] = "-d";
 
@@ -50,9 +96,19 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
 		rec_argv[i++] = perf_mem_events[j].name;
 	};
 
-	for (j = 1; j < argc; j++, i++)
+	for (j = 0; j < argc; j++, i++)
 		rec_argv[i] = argv[j];
 
+	if (verbose > 0) {
+		pr_debug("calling: record ");
+
+		while (rec_argv[j]) {
+			pr_debug("%s ", rec_argv[j]);
+			j++;
+		}
+		pr_debug("\n");
+	}
+
 	ret = cmd_record(i, rec_argv, NULL);
 	free(rec_argv);
 	return ret;
@@ -299,7 +355,6 @@ int cmd_mem(int argc, const char **argv, const char *prefix __maybe_unused)
 		NULL
 	};
 
-
 	argc = parse_options_subcommand(argc, argv, mem_options, mem_subcommands,
 					mem_usage, PARSE_OPT_STOP_AT_NON_OPTION);
 
tools/perf/util/mem-events.c +44 -3

54 lines not on this page · raw message

tools/perf/util/mem-events.h +3 -0
index c97b21465fa9..2995bae6ac33 100644
@@ -5,6 +5,7 @@
 
 struct perf_mem_event {
 	bool		record;
+	const char	*tag;
 	const char	*name;
 };
 
@@ -16,4 +17,6 @@ enum {
 
 extern struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX];
 
+int perf_mem_events__parse(const char *str);
+
 #endif /* __PERF_MEM_EVENTS_H */
Signature · 1 line
2.5.0
author maintainer
#6 lore raw

PATCH 6/11 perf tools: Use ARRAY_SIZE in mem sort display functions

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

There's no need to define extra macros for that.

---
 tools/perf/util/sort.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
tools/perf/util/sort.c +3 -6
index 5f94ee740c5b..5388f7940474 100644
@@ -838,7 +838,6 @@ static const char * const tlb_access[] = {
 	"Walker",
 	"Fault",
 };
-#define NUM_TLB_ACCESS (sizeof(tlb_access)/sizeof(const char *))
 
 static int hist_entry__tlb_snprintf(struct hist_entry *he, char *bf,
 				    size_t size, unsigned int width)
@@ -860,7 +859,7 @@ static int hist_entry__tlb_snprintf(struct hist_entry *he, char *bf,
 	/* already taken care of */
 	m &= ~(PERF_MEM_TLB_HIT|PERF_MEM_TLB_MISS);
 
-	for (i = 0; m && i < NUM_TLB_ACCESS; i++, m >>= 1) {
+	for (i = 0; m && i < ARRAY_SIZE(tlb_access); i++, m >>= 1) {
 		if (!(m & 0x1))
 			continue;
 		if (l) {
@@ -915,7 +914,6 @@ static const char * const mem_lvl[] = {
 	"I/O",
 	"Uncached",
 };
-#define NUM_MEM_LVL (sizeof(mem_lvl)/sizeof(const char *))
 
 static int hist_entry__lvl_snprintf(struct hist_entry *he, char *bf,
 				    size_t size, unsigned int width)
@@ -937,7 +935,7 @@ static int hist_entry__lvl_snprintf(struct hist_entry *he, char *bf,
 	/* already taken care of */
 	m &= ~(PERF_MEM_LVL_HIT|PERF_MEM_LVL_MISS);
 
-	for (i = 0; m && i < NUM_MEM_LVL; i++, m >>= 1) {
+	for (i = 0; m && i < ARRAY_SIZE(mem_lvl); i++, m >>= 1) {
 		if (!(m & 0x1))
 			continue;
 		if (l) {
@@ -983,7 +981,6 @@ static const char * const snoop_access[] = {
 	"Hit",
 	"HitM",
 };
-#define NUM_SNOOP_ACCESS (sizeof(snoop_access)/sizeof(const char *))
 
 static int hist_entry__snoop_snprintf(struct hist_entry *he, char *bf,
 				    size_t size, unsigned int width)
@@ -998,7 +995,7 @@ static int hist_entry__snoop_snprintf(struct hist_entry *he, char *bf,
 	if (he->mem_info)
 		m = he->mem_info->data_src.mem_snoop;
 
-	for (i = 0; m && i < NUM_SNOOP_ACCESS; i++, m >>= 1) {
+	for (i = 0; m && i < ARRAY_SIZE(snoop_access); i++, m >>= 1) {
 		if (!(m & 0x1))
 			continue;
 		if (l) {
Signature · 1 line
2.5.0
author maintainer
#7 lore raw

PATCH 7/11 perf script: Add data_src and weight column definitions

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Adding data_src and weight column definitions, so it's displayed for related sample types.

---
 tools/perf/builtin-script.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
tools/perf/builtin-script.c +20 -0
index c691214d820f..b7f1e8e22dcf 100644
@@ -58,6 +58,8 @@ enum perf_output_field {
 	PERF_OUTPUT_IREGS	    = 1U << 14,
 	PERF_OUTPUT_BRSTACK	    = 1U << 15,
 	PERF_OUTPUT_BRSTACKSYM	    = 1U << 16,
+	PERF_OUTPUT_DATA_SRC	    = 1U << 17,
+	PERF_OUTPUT_WEIGHT	    = 1U << 18,
 };
 
 struct output_option {
@@ -81,6 +83,8 @@ struct output_option {
 	{.str = "iregs", .field = PERF_OUTPUT_IREGS},
 	{.str = "brstack", .field = PERF_OUTPUT_BRSTACK},
 	{.str = "brstacksym", .field = PERF_OUTPUT_BRSTACKSYM},
+	{.str = "data_src", .field = PERF_OUTPUT_DATA_SRC},
+	{.str = "weight",   .field = PERF_OUTPUT_WEIGHT},
 };
 
 /* default set to maintain compatibility with current format */
@@ -242,6 +246,16 @@ static int perf_evsel__check_attr(struct perf_evsel *evsel,
 					   PERF_OUTPUT_ADDR, allow_user_set))
 		return -EINVAL;
 
+	if (PRINT_FIELD(DATA_SRC) &&
+		perf_evsel__check_stype(evsel, PERF_SAMPLE_DATA_SRC, "DATA_SRC",
+					PERF_OUTPUT_DATA_SRC))
+		return -EINVAL;
+
+	if (PRINT_FIELD(WEIGHT) &&
+		perf_evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT, "WEIGHT",
+					PERF_OUTPUT_WEIGHT))
+		return -EINVAL;
+
 	if (PRINT_FIELD(SYM) && !PRINT_FIELD(IP) && !PRINT_FIELD(ADDR)) {
 		pr_err("Display of symbols requested but neither sample IP nor "
 			   "sample address\nis selected. Hence, no addresses to convert "
@@ -673,6 +687,12 @@ static void process_event(struct perf_script *script, union perf_event *event,
 	if (PRINT_FIELD(ADDR))
 		print_sample_addr(event, sample, thread, attr);
 
+	if (PRINT_FIELD(DATA_SRC))
+		printf("%16" PRIx64, sample->data_src);
+
+	if (PRINT_FIELD(WEIGHT))
+		printf("%16" PRIu64, sample->weight);
+
 	if (PRINT_FIELD(IP)) {
 		if (!symbol_conf.use_callchain)
 			printf(" ");
Signature · 1 line
2.5.0
author maintainer
#8 lore raw

PATCH 8/11 perf script: Display addr/data_src/weight columns for raw events

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Adding addr/data_src/weight columns for raw events.

Example:

  $ perf script
  ...
  true 11883 322960.489590: ...  ffff8801aa0b8400        68501042             246 ffffffff813b2cd
  true 11883 322960.489600: ...  ffff8800b90b38d8        68501042             251 ffffffff811d0b7
  true 11883 322960.489612: ...  ffff880196893130        6a100142              94 ffffffff8177fb8
  true 11883 322960.489637: ...  ffff880164277b40        68100842             101 ffffffff813b2cd
  true 11883 322960.489683: ...  ffff880035d3d818        68501042             201 ffffffff811d0b7
  true 11883 322960.489733: ...      7fb9616efcf0        68100242             199     7fb961aaba9
  true 11883 322960.489818: ...  ffffea000481c39c        6a100142             122 ffffffff811b634

^^^^^^^^^^^^^^^^ ^^^^^^^^ ^^^
addr data_src weight

---
 tools/perf/builtin-script.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
tools/perf/builtin-script.c +2 -1
index b7f1e8e22dcf..f4caf4898245 100644
@@ -135,7 +135,8 @@ static struct {
 			      PERF_OUTPUT_CPU | PERF_OUTPUT_TIME |
 			      PERF_OUTPUT_EVNAME | PERF_OUTPUT_IP |
 			      PERF_OUTPUT_SYM | PERF_OUTPUT_DSO |
-			      PERF_OUTPUT_PERIOD,
+			      PERF_OUTPUT_PERIOD |  PERF_OUTPUT_ADDR |
+			      PERF_OUTPUT_DATA_SRC | PERF_OUTPUT_WEIGHT,
 
 		.invalid_fields = PERF_OUTPUT_TRACE,
 	},
Signature · 1 line
2.5.0
author maintainer
#9 lore raw

PATCH 9/11 perf tools: Dont stop PMU parsing on alias parse error

In reply to Arnaldo Carvalho de Melo

From: Andi Kleen <[email protected]>

When an error happens during alias parsing currently the complete parsing of all attributes of the PMU is stopped. This is breaks old perf on a newer kernel that may have not-yet-know alias attributes (such as .scale or .per-pkg).

Continue when some attribute is unparseable.

This is IMHO a stable candidate and should be backported to older versions to avoid problems with newer kernels.

v2: Print warnings when something goes wrong.
v3: Change warning to debug output

---
 tools/perf/util/pmu.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
tools/perf/util/pmu.c +8 -7
index cf59fbaee491..ce61f79dbaae 100644
@@ -284,13 +284,12 @@ static int pmu_aliases_parse(char *dir, struct list_head *head)
 {
 	struct dirent *evt_ent;
 	DIR *event_dir;
-	int ret = 0;
 
 	event_dir = opendir(dir);
 	if (!event_dir)
 		return -EINVAL;
 
-	while (!ret && (evt_ent = readdir(event_dir))) {
+	while ((evt_ent = readdir(event_dir))) {
 		char path[PATH_MAX];
 		char *name = evt_ent->d_name;
 		FILE *file;
@@ -306,17 +305,19 @@ static int pmu_aliases_parse(char *dir, struct list_head *head)
 
 		snprintf(path, PATH_MAX, "%s/%s", dir, name);
 
-		ret = -EINVAL;
 		file = fopen(path, "r");
-		if (!file)
-			break;
+		if (!file) {
+			pr_debug("Cannot open %s\n", path);
+			continue;
+		}
 
-		ret = perf_pmu__new_alias(head, dir, name, file);
+		if (perf_pmu__new_alias(head, dir, name, file) < 0)
+			pr_debug("Cannot set up %s\n", name);
 		fclose(file);
 	}
 
 	closedir(event_dir);
-	return ret;
+	return 0;
 }
 
 /*
Signature · 1 line
2.5.0
author maintainer
#10 lore raw

PATCH 10/11 perf help: No need to use strbuf_remove()

In reply to Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <[email protected]>

It is the only user of this function, just use the strlen() to skip the prefix.

---
 tools/perf/builtin-help.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
tools/perf/builtin-help.c +1 -2
index 96c1a4cfbbbf..f4dd2b48f90f 100644
@@ -86,8 +86,7 @@ static int check_emacsclient_version(void)
 		return -1;
 	}
 
-	strbuf_remove(&buffer, 0, strlen("emacsclient"));
-	version = atoi(buffer.buf);
+	version = atoi(buffer.buf + strlen("emacsclient"));
 
 	if (version < 22) {
 		fprintf(stderr,
Signature · 1 line
2.5.0
author maintainer
#11 lore raw

PATCH 11/11 perf tools: Remove strbuf_{remove,splice}()

In reply to Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <[email protected]>

No users, nuke them.

---
 tools/perf/util/strbuf.c | 24 ------------------------
 tools/perf/util/strbuf.h |  2 --
 2 files changed, 26 deletions(-)
tools/perf/util/strbuf.c +0 -24
index 25671fa16618..d3d279275432 100644
@@ -51,30 +51,6 @@ void strbuf_grow(struct strbuf *sb, size_t extra)
 	ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
 }
 
-static void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
-				   const void *data, size_t dlen)
-{
-	if (pos + len < pos)
-		die("you want to use way too much memory");
-	if (pos > sb->len)
-		die("`pos' is too far after the end of the buffer");
-	if (pos + len > sb->len)
-		die("`pos + len' is too far after the end of the buffer");
-
-	if (dlen >= len)
-		strbuf_grow(sb, dlen - len);
-	memmove(sb->buf + pos + dlen,
-			sb->buf + pos + len,
-			sb->len - pos - len);
-	memcpy(sb->buf + pos, data, dlen);
-	strbuf_setlen(sb, sb->len + dlen - len);
-}
-
-void strbuf_remove(struct strbuf *sb, size_t pos, size_t len)
-{
-	strbuf_splice(sb, pos, len, NULL, 0);
-}
-
 void strbuf_add(struct strbuf *sb, const void *data, size_t len)
 {
 	strbuf_grow(sb, len);
tools/perf/util/strbuf.h +0 -2
index 529f2f035249..7a32c838884d 100644
@@ -77,8 +77,6 @@ static inline void strbuf_addch(struct strbuf *sb, int c) {
 	sb->buf[sb->len] = '\0';
 }
 
-extern void strbuf_remove(struct strbuf *, size_t pos, size_t len);
-
 extern void strbuf_add(struct strbuf *, const void *, size_t);
 static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
 	strbuf_add(sb, s, strlen(s));
Signature · 1 line
2.5.0
#12 lore raw
In reply to Arnaldo Carvalho de Melo
Quoted 63 lines · Arnaldo Carvalho de Melo

Hi Ingo,

Please consider pulling, this is on top of the perf-core-for-mingo
submitted recently,

- Arnaldo

The following changes since commit 03e0a7df3efd959e40cd7ff40b1fabddc234ec5a:

perf tools: Introduce bpf-output event (2016-02-22 14:37:21 -0300)

are available in the git repository at:

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

for you to fetch changes up to bea2400621836b028d82c3d6a74053921d70dbd7:

perf tools: Remove strbuf_{remove,splice}() (2016-02-23 16:21:04 -0300)

----------------------------------------------------------------
perf/core improvements and fixes:

User visible:

- Don't stop PMU parsing on alias parse error, allowing the addition of new sysfs PMU files without breaking old tools (Andi Kleen)

- Implement '%' operation in libtraceevent (Daniel Bristot de Oliveira)

- Allow specifying events via -e in 'perf mem record', also listing what events can be specified via 'perf mem record -e list' (Jiri Olsa)

- Improve support to 'data_src', 'weight' and 'addr' fields in 'perf script' (Jiri Olsa)

Infrastructure:

- Export cacheline routines (Jiri Olsa)

- Remove strbuf_{remove,splice}(), dead code (Arnaldo Carvalho de Melo)

----------------------------------------------------------------
Andi Kleen (1):
perf tools: Dont stop PMU parsing on alias parse error

Arnaldo Carvalho de Melo (2):
perf help: No need to use strbuf_remove()
perf tools: Remove strbuf_{remove,splice}()

Daniel Bristot de Oliveira (1):
tools lib traceevent: Implement '%' operation

Jiri Olsa (7):
perf tools: Make cl_address global
perf tools: Introduce cl_offset function
perf tools: Add monitored events array
perf mem: Add -e record option
perf tools: Use ARRAY_SIZE in mem sort display functions perf script: Add data_src and weight column definitions perf script: Display addr/data_src/weight columns for raw events

Quoted 15 lines
 tools/lib/traceevent/event-parse.c |  4 +++
 tools/perf/builtin-help.c          |  3 +-
 tools/perf/builtin-mem.c           | 74 +++++++++++++++++++++++++++++++++-----
 tools/perf/builtin-script.c        | 23 +++++++++++-
 tools/perf/util/Build              |  1 +
 tools/perf/util/mem-events.c       | 51 ++++++++++++++++++++++++++
 tools/perf/util/mem-events.h       | 22 ++++++++++++
 tools/perf/util/pmu.c              | 15 ++++----
 tools/perf/util/sort.c             | 15 ++------
 tools/perf/util/sort.h             | 11 ++++++
 tools/perf/util/strbuf.c           | 24 -------------
 tools/perf/util/strbuf.h           |  2 --
 12 files changed, 188 insertions(+), 57 deletions(-)
 create mode 100644 tools/perf/util/mem-events.c
 create mode 100644 tools/perf/util/mem-events.h

Pulled, thanks a lot Arnaldo!

Ingo