Hi,
The folowing patch upgrades the perf with extended performance event description, as proposed in a separate RFC.
You can find it here: http://article.gmane.org/gmane.linux.ports.arm.kernel/72822
It has been tested on a ARMv7 Cortex-A8 incarnation.
Event names can be specified to -e switch, and are reported in the list command.
The perf-predefined commands take precedence (if a collision occurs) over the platform-specific ones.
A workaround for that is though - specify the raw event, as seen provided by the system.
Currently it resides in the sysfs, but I've already been pointed to a sysfs note disclaiming mixed types, multiple lines and fancy formatting. Might swith over to debugfs.
Thanks,
--
Tomasz Fujak
Signed-off-by: Tomasz Fujak <[email protected]>
Reviewed-by: Pawel Osciak <[email protected]>
Reviewed-by: Marek Szyprowski <[email protected]>
Reviewed-by: Kyungmin Park <[email protected]>
---
tools/perf/util/parse-events.c | 224 ++++++++++++++++++++++++++++++++++++++--
1 files changed, 213 insertions(+), 11 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 05d0c5c..3e32198 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -9,6 +9,9 @@
#include "header.h"
#include "debugfs.h"
+#define EXTENDED_EVENT_PATH \
+ "/sys/devices/system/cpu/perf_events/extevents"
+
int nr_counters;
struct perf_event_attr attrs[MAX_COUNTERS];
@@ -60,6 +63,10 @@ static struct event_symbol event_symbols[] = {
#define PERF_EVENT_TYPE(config) __PERF_EVENT_FIELD(config, TYPE)
#define PERF_EVENT_ID(config) __PERF_EVENT_FIELD(config, EVENT)
+static struct event_symbol *extended_event_symbols;
+static unsigned int extended_event_count;
+static int extended_events_initialized;
+
static const char *hw_event_names[] = {
"cycles",
"instructions",
@@ -241,6 +248,157 @@ static const char *tracepoint_id_to_name(u64 config)
return buf;
}
+static unsigned count_lines(const char *str, unsigned size)
+{
+ unsigned count = 0;
+
+ while (size--)
+ count += ('\n' == *str++);
+ return count;
+}
+
+#define BYTES_PER_CHUNK 4096
+/* returns the number of lines read;
+ on fail the return is negative and no memory is allocated
+ otherwise the *buf contains a memory chunk of which first
+ *size bytes are used for file contents
+ */
+static int read_file(int fd, char **buf, unsigned *size)
+{
+ unsigned bytes = BYTES_PER_CHUNK;
+ int lines = 0;
+ char *ptr = malloc(bytes);
+
+ *size = 0;
+ do {
+ int ret = read(fd, ptr + *size, bytes - *size);
+ if (ret < 0) {
+ if (EINTR == errno)
+ continue;
+ else {
+ free(ptr);
+ return -1;
+ }
+ }
+
+ if (!ret)
+ break;
+
+ lines += count_lines(ptr + *size, ret);
+ *size += ret;
+
+ if (*size == bytes) {
+ char *tmp = realloc(ptr, bytes <<= 1);
+ if (!tmp) {
+ free(ptr);
+ return -1;
+ }
+ ptr = tmp;
+ }
+ } while (1);
+
+ *buf = ptr;
+ return lines;
+}
+
+static int parse_extevent_file(struct event_symbol *symbols,
+ unsigned lines, char *buf)
+{
+ char *name, *description, *ptr, *end;
+ unsigned offset = 0, count = 0;
+ int items, eaten;
+ unsigned long long discard;
+
+/* each line format should be "0x%llx\t%s\t%lld-%lld\t%s\n" */
+ while (lines--) {
+ items = sscanf(buf + offset + 2, "%llx",
+ &symbols[count].config);
+ if (1 != items)
+ continue;
+
+ /* skip 0x%llx\t */
+ ptr = strchr(buf + offset, '\t') + 1;
+
+ name = description = NULL;
+
+ end = strchr(ptr, '\t');
+ if (end) {
+ name = strndup(ptr, end - ptr);
+ ptr = end + 1;
+ }
+
+ ptr = end;
+ items = sscanf(ptr, "%lld-%lld\t%n", &discard, &discard,
+ &eaten);
+ if (2 != items)
+ continue;
+
+ ptr += eaten;
+ end = strchr(ptr, '\n');
+ if (end) {
+ description = strndup(ptr, end - ptr);
+ offset = end - buf + 1;
+ } else
+ break;
+
+ if (name && description) {
+ symbols[count].symbol = name;
+ symbols[count].alias = "";
+ ++count;
+ /* description gets lost here */
+ free(description);
+ } else {
+ free(description);
+ free(name);
+ }
+ }
+
+ return count;
+}
+
+static int load_extended_events(const char *extevent_path)
+{
+ int fd;
+ int lines = 0;
+ unsigned size = 0;
+ char *buf = NULL;
+
+ fd = open(extevent_path, O_RDONLY);
+ if (fd < 0)
+ return fd;
+
+ lines = read_file(fd, &buf, &size);
+ close(fd);
+
+ if (0 < lines) {
+ struct event_symbol *symbols = (struct event_symbol *)
+ calloc(sizeof(struct event_symbol), lines);
+ if (symbols) {
+ int parsed_symbols = parse_extevent_file(symbols,
+ lines, buf);
+ if (0 < parsed_symbols) {
+ extended_event_symbols = symbols;
+ extended_event_count = parsed_symbols;
+ } else
+ free(symbols);
+ } else
+ lines = -1;
+ }
+ free(buf);
+
+ return lines;
+}
+
+static struct event_symbol *extevent_find_config(u64 config)
+{
+ unsigned int i;
+ for (i = 0; i < extended_event_count; ++i)
+ if (extended_event_symbols[i].config == config)
+ return &extended_event_symbols[i];
+
+ return NULL;
+}
+
static int is_cache_op_valid(u8 cache_type, u8 cache_op)
{
if (hw_cache_stat[cache_type] & COP(cache_op))
@@ -283,10 +441,16 @@ const char *__event_name(int type, u64 config)
}
switch (type) {
- case PERF_TYPE_HARDWARE:
+ case PERF_TYPE_HARDWARE: {
+ const struct event_symbol *event;
+
if (config < PERF_COUNT_HW_MAX)
return hw_event_names[config];
+ event = extevent_find_config(config);
+ if (event)
+ return event->symbol;
return "unknown-hardware";
+ }
case PERF_TYPE_HW_CACHE: {
u8 cache_type, cache_op, cache_result;
@@ -611,33 +775,34 @@ parse_breakpoint_event(const char **strp, struct perf_event_attr *attr)
return EVT_HANDLED;
}
-static int check_events(const char *str, unsigned int i)
+static int check_event(const char *str, const struct event_symbol *event)
{
int n;
- n = strlen(event_symbols[i].symbol);
- if (!strncmp(str, event_symbols[i].symbol, n))
+ n = strlen(event->symbol);
+ if (!strncmp(str, event->symbol, n))
return n;
- n = strlen(event_symbols[i].alias);
+ n = strlen(event->alias);
if (n)
- if (!strncmp(str, event_symbols[i].alias, n))
+ if (!strncmp(str, event->alias, n))
return n;
return 0;
}
static enum event_result
-parse_symbolic_event(const char **strp, struct perf_event_attr *attr)
+do_parse_symbolic_event(const char **strp, struct perf_event_attr *attr,
+ const struct event_symbol *symbols, unsigned int count)
{
const char *str = *strp;
unsigned int i;
int n;
- for (i = 0; i < ARRAY_SIZE(event_symbols); i++) {
- n = check_events(str, i);
+ for (i = 0; i < count; ++i) {
+ n = check_event(str, &symbols[i]);
if (n > 0) {
- attr->type = event_symbols[i].type;
- attr->config = event_symbols[i].config;
+ attr->type = symbols[i].type;
+ attr->config = symbols[i].config;
*strp = str + n;
return EVT_HANDLED;
}
@@ -646,6 +811,27 @@ parse_symbolic_event(const char **strp, struct perf_event_attr *attr)
}
static enum event_result
+parse_symbolic_event(const char **strp, struct perf_event_attr *attr)
+{
+ return do_parse_symbolic_event(strp, attr,
+ event_symbols, ARRAY_SIZE(event_symbols));
+}
+
+static enum event_result
+parse_extended_event(const char **strp, struct perf_event_attr *attr)
+{
+ if (!extended_events_initialized)
+ extended_events_initialized =
+ load_extended_events(EXTENDED_EVENT_PATH);
+
+ if (extended_events_initialized < 0)
+ return EVT_FAILED;
+
+ return do_parse_symbolic_event(strp, attr, extended_event_symbols,
+ extended_event_count);
+}
+
+static enum event_result
parse_raw_event(const char **strp, struct perf_event_attr *attr)
{
const char *str = *strp;
@@ -744,6 +930,10 @@ parse_event_symbols(const char **str, struct perf_event_attr *attr)
if (ret != EVT_FAILED)
goto modifier;
+ ret = parse_extended_event(str, attr);
+ if (ret != EVT_FAILED)
+ goto modifier;
+
ret = parse_breakpoint_event(str, attr);
if (ret != EVT_FAILED)
goto modifier;
@@ -932,6 +1122,18 @@ void print_events(void)
}
}
+ if (!extended_events_initialized)
+ extended_events_initialized =
+ load_extended_events(EXTENDED_EVENT_PATH);
+
+ if (0 < extended_events_initialized) {
+ for (i = 0; i < extended_event_count; ++i)
+ printf(" %-42s [%s]\n",
+ extended_event_symbols[i].symbol,
+ "Hardware platform-specific event");
+ }
+
+
printf("\n");
printf(" %-42s [%s]\n",
"rNNN", event_type_descriptors[PERF_TYPE_RAW]);
--
1.5.4.3
Em Fri, Jan 22, 2010 at 01:08:59PM +0100, Tomasz Fujak escreveu:
> Signed-off-by: Tomasz Fujak <[email protected]>
> Reviewed-by: Pawel Osciak <[email protected]>
> Reviewed-by: Marek Szyprowski <[email protected]>
> Reviewed-by: Kyungmin Park <[email protected]>
"Extended" seems vague, I think in this context "platform_" would be a
better namespace specifier.
> ---
> tools/perf/util/parse-events.c | 224 ++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 213 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 05d0c5c..3e32198 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -9,6 +9,9 @@
> #include "header.h"
> #include "debugfs.h"
>
> +#define EXTENDED_EVENT_PATH \
> + "/sys/devices/system/cpu/perf_events/extevents"
> +
> int nr_counters;
>
> struct perf_event_attr attrs[MAX_COUNTERS];
> @@ -60,6 +63,10 @@ static struct event_symbol event_symbols[] = {
> #define PERF_EVENT_TYPE(config) __PERF_EVENT_FIELD(config, TYPE)
> #define PERF_EVENT_ID(config) __PERF_EVENT_FIELD(config, EVENT)
>
> +static struct event_symbol *extended_event_symbols;
> +static unsigned int extended_event_count;
> +static int extended_events_initialized;
> +
> static const char *hw_event_names[] = {
> "cycles",
> "instructions",
> @@ -241,6 +248,157 @@ static const char *tracepoint_id_to_name(u64 config)
> return buf;
> }
>
> +static unsigned count_lines(const char *str, unsigned size)
> +{
> + unsigned count = 0;
> +
> + while (size--)
> + count += ('\n' == *str++);
> + return count;
> +}
> +
> +#define BYTES_PER_CHUNK 4096
> +/* returns the number of lines read;
> + on fail the return is negative and no memory is allocated
> + otherwise the *buf contains a memory chunk of which first
> + *size bytes are used for file contents
> + */
> +static int read_file(int fd, char **buf, unsigned *size)
> +{
> + unsigned bytes = BYTES_PER_CHUNK;
> + int lines = 0;
> + char *ptr = malloc(bytes);
malloc can fail... Also why is that you can't process the lines one by
one instead of reading the whole (albeit small) file at once?
> + *size = 0;
> + do {
> + int ret = read(fd, ptr + *size, bytes - *size);
> + if (ret < 0) {
> + if (EINTR == errno)
> + continue;
> + else {
> + free(ptr);
> + return -1;
> + }
> + }
> +
> + if (!ret)
> + break;
> +
> + lines += count_lines(ptr + *size, ret);
> + *size += ret;
> +
> + if (*size == bytes) {
> + char *tmp = realloc(ptr, bytes <<= 1);
> + if (!tmp) {
> + free(ptr);
> + return -1;
> + }
> + ptr = tmp;
> + }
> + } while (1);
> +
> + *buf = ptr;
> + return lines;
> +}
> +
> +static int parse_extevent_file(struct event_symbol *symbols,
> + unsigned lines, char *buf)
> +{
> + char *name, *description, *ptr, *end;
> + unsigned offset = 0, count = 0;
> + int items, eaten;
> + unsigned long long discard;
> +
> +/* each line format should be "0x%llx\t%s\t%lld-%lld\t%s\n" */
> + while (lines--) {
> + items = sscanf(buf + offset + 2, "%llx",
> + &symbols[count].config);
> + if (1 != items)
> + continue;
> +
> + /* skip 0x%llx\t */
> + ptr = strchr(buf + offset, '\t') + 1;
> +
> + name = description = NULL;
> +
> + end = strchr(ptr, '\t');
> + if (end) {
> + name = strndup(ptr, end - ptr);
> + ptr = end + 1;
> + }
> +
> + ptr = end;
> + items = sscanf(ptr, "%lld-%lld\t%n", &discard, &discard,
> + &eaten);
> + if (2 != items)
> + continue;
> +
> + ptr += eaten;
> + end = strchr(ptr, '\n');
> + if (end) {
> + description = strndup(ptr, end - ptr);
> + offset = end - buf + 1;
> + } else
> + break;
> +
> + if (name && description) {
> + symbols[count].symbol = name;
> + symbols[count].alias = "";
> + ++count;
> + /* description gets lost here */
> + free(description);
> + } else {
> + free(description);
> + free(name);
> + }
Having "free(description);" in both cases seems funny :-)
- Arnaldo
> -----Original Message-----
> From: [email protected] [mailto:linux-arm-
> [email protected]] On Behalf Of Arnaldo Carvalho de
> Melo
> Sent: Friday, January 22, 2010 1:26 PM
> To: Tomasz Fujak
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v1 1/1] Extended events (platform-specific) support
> in perf
>
> Em Fri, Jan 22, 2010 at 01:08:59PM +0100, Tomasz Fujak escreveu:
> > Signed-off-by: Tomasz Fujak <[email protected]>
> > Reviewed-by: Pawel Osciak <[email protected]>
> > Reviewed-by: Marek Szyprowski <[email protected]>
> > Reviewed-by: Kyungmin Park <[email protected]>
>
> "Extended" seems vague, I think in this context "platform_" would be a
> better namespace specifier.
Right.
> > +#define BYTES_PER_CHUNK 4096
> > +/* returns the number of lines read;
> > + on fail the return is negative and no memory is allocated
> > + otherwise the *buf contains a memory chunk of which first
> > + *size bytes are used for file contents
> > + */
> > +static int read_file(int fd, char **buf, unsigned *size)
> > +{
> > + unsigned bytes = BYTES_PER_CHUNK;
> > + int lines = 0;
> > + char *ptr = malloc(bytes);
>
> malloc can fail... Also why is that you can't process the lines one by
> one instead of reading the whole (albeit small) file at once?
Right, no malloc check.
The purpose of not processing line-by-line is that then the collection needs
to be dynamic.
Since the file buffer gets dropped after it's processed, the overallocated
memory is returned to the system.
In case of the collection (here an array and a counter) it'd require tricks
not to overallocate, and still be O(#events).
But you're right I didn't do it right - in case the a line does not parse,
memory reserved for it is not freed. Fixing.
> > +
> > + if (name && description) {
> > + symbols[count].symbol = name;
> > + symbols[count].alias = "";
> > + ++count;
> > + /* description gets lost here */
> > + free(description);
> > + } else {
> > + free(description);
> > + free(name);
> > + }
>
> Having "free(description);" in both cases seems funny :-)
I wasn't so bold as to upgrade the perf with human-readable event
description.
Otherwise yes, moving it outside.
>
> - Arnaldo
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Fri, 2010-01-22 at 13:08 +0100, Tomasz Fujak wrote:
there's supposed to be a changelog here...
> Signed-off-by: Tomasz Fujak <[email protected]>
> Reviewed-by: Pawel Osciak <[email protected]>
> Reviewed-by: Marek Szyprowski <[email protected]>
> Reviewed-by: Kyungmin Park <[email protected]>
Surely one of you knew that ;-)
Anyway, I still think it stinks, and as pointed out, it violates the
one-value-per-file sysfs rule.
I really see no reason why you cannot do this in userspace, have
tools/perf/ provide a library that does this for all supported platforms
with a common interface or something.