linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* perf: Add support for full Intel event lists v3
@ 2014-05-12 22:51 Andi Kleen
  2014-05-12 22:51 ` [PATCH 1/9] perf, tools: Add jsmn `jasmine' JSON parser Andi Kleen
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: Andi Kleen @ 2014-05-12 22:51 UTC (permalink / raw)
  To: acme; +Cc: linux-kernel, peterz, eranian, namhyung, jolsa

[v2: Review feedback addressed and some minor improvements]
[v3: More review feedback addressed and handle test failures better.
Ported to latest tip/core.]

perf has high level events which are useful in many cases. However
there are some tuning situations where low level events in the CPU
are needed. Traditionally this required specifying the event in 
raw form (very awkward) or using non standard frontends
like ocperf or patching in libpfm.

Intel CPUs can have very large event files (Haswell has ~336 core events,
much more if you add uncore or all the offcore combinations), which is too
large to describe through the kernel interface. It would require tying up
significant amounts of unswappable memory for this.

oprofile always had separate event list files that were maintained by 
the CPU vendors. The oprofile events were shipped with the tool.
The Intel events get updated regularly, for example to add references
to the specification updates or add new events.

Unfortunately oprofile usually did not keep up with these updates,
so the events in oprofile were often out of date. In addition
it ties up quite a bit of disk space, mostly for CPUs you don't have.

This patch kit implements another mechanism that avoids these problems.
Intel releases the event lists for CPUs in a standardized JSON format
on a download server.

I implemented an automatic downloader to get the event file for the
current CPU.  The events are stored in ~/.events.
Then perf adds a parser that converts the JSON format into perf event
aliases, which then can be used directly as any other perf event.

The parsing is done using a simple existing JSON library.

The events are still abstracted for perf, but the abstraction mechanism is
through the downloaded file instead of through the kernel.

The JSON format and perf parser has some minor Intelisms, but they
are simple and small and optional. It's easy to extend, so it would be
possible to use it for other CPUs too, add different pmu attributes, and
add new download sites to the downloader tool.

Currently only core events are supported, uncore may come at a later
point. No kernel changes, all code in perf user tools only.

Some of the parser files are partially shared with separate event parser
library and are thus 2-clause BSD licensed.

Patches also available from
git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc perf/json

Example output:

% perf download 
Downloading models file
Downloading readme.txt
2014-03-05 10:39:33 URL:https://download.01.org/perfmon/readme.txt [10320/10320] -> "readme.txt" [1]
2014-03-05 10:39:34 URL:https://download.01.org/perfmon/mapfile.csv [1207/1207] -> "mapfile.csv" [1]
Downloading events file
% perf list
...
  br_inst_exec.all_branches                          [Speculative and retired
                                                      branches]
  br_inst_exec.all_conditional                       [Speculative and retired
                                                      macro-conditional
                                                      branches]
  br_inst_exec.all_direct_jmp                        [Speculative and retired
                                                      macro-unconditional
                                                      branches excluding
                                                      calls and indirects]
... 333 more new events ...

% perf stat -e br_inst_exec.all_direct_jmp true

 Performance counter stats for 'true':

             6,817      cpu/br_inst_exec.all_direct_jmp/                                   

       0.003503212 seconds time elapsed

One nice feature is that a pointer to the specification update is now
included in the description, which will hopefully clear up many problems:

% perf list
...
  mem_load_uops_l3_hit_retired.xsnp_hit              [Retired load uops which
                                                      data sources were L3
                                                      and cross-core snoop
                                                      hits in on-pkg core
                                                      cache. Supports address
                                                      when precise. Spec
                                                      update: HSM26, HSM30
                                                      (Precise event)]
...


-Andi

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

* [PATCH 1/9] perf, tools: Add jsmn `jasmine' JSON parser
  2014-05-12 22:51 perf: Add support for full Intel event lists v3 Andi Kleen
@ 2014-05-12 22:51 ` Andi Kleen
  2014-05-14  3:25   ` Namhyung Kim
  2014-05-12 22:51 ` [PATCH 2/9] perf, tools: Add support for text descriptions of events and alias add Andi Kleen
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Andi Kleen @ 2014-05-12 22:51 UTC (permalink / raw)
  To: acme; +Cc: linux-kernel, peterz, eranian, namhyung, jolsa, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

I need a JSON parser. This adds the simplest JSON
parser I could find -- Serge Zaitsev's jsmn `jasmine' --
to the perf library. I merely converted it to (mostly)
Linux style and added support for non 0 terminated input.

The parser is quite straight forward and does not
copy any data, just returns tokens with offsets
into the input buffer. So it's relatively efficient
and simple to use.

The code is not fully checkpatch clean, but I didn't
want to completely fork the upstream code.

Original source: http://zserge.bitbucket.org/jsmn.html

In addition I added a simple wrapper that mmaps a json
file and provides some straight forward access functions.

Used in follow-on patches to parse event files.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Makefile.perf |   4 +
 tools/perf/util/jsmn.c   | 297 +++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/jsmn.h   |  65 +++++++++++
 tools/perf/util/json.c   | 155 +++++++++++++++++++++++++
 tools/perf/util/json.h   |  13 +++
 5 files changed, 534 insertions(+)
 create mode 100644 tools/perf/util/jsmn.c
 create mode 100644 tools/perf/util/jsmn.h
 create mode 100644 tools/perf/util/json.c
 create mode 100644 tools/perf/util/json.h

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 2baf61c..43d4109 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -300,6 +300,8 @@ LIB_H += ui/progress.h
 LIB_H += ui/util.h
 LIB_H += ui/ui.h
 LIB_H += util/data.h
+LIB_H += util/jsmn.h
+LIB_H += util/json.h
 
 LIB_OBJS += $(OUTPUT)util/abspath.o
 LIB_OBJS += $(OUTPUT)util/alias.o
@@ -373,6 +375,8 @@ LIB_OBJS += $(OUTPUT)util/stat.o
 LIB_OBJS += $(OUTPUT)util/record.o
 LIB_OBJS += $(OUTPUT)util/srcline.o
 LIB_OBJS += $(OUTPUT)util/data.o
+LIB_OBJS += $(OUTPUT)util/jsmn.o
+LIB_OBJS += $(OUTPUT)util/json.o
 
 LIB_OBJS += $(OUTPUT)ui/setup.o
 LIB_OBJS += $(OUTPUT)ui/helpline.o
diff --git a/tools/perf/util/jsmn.c b/tools/perf/util/jsmn.c
new file mode 100644
index 0000000..3e4b711
--- /dev/null
+++ b/tools/perf/util/jsmn.c
@@ -0,0 +1,297 @@
+/*
+ * Copyright (c) 2010 Serge A. Zaitsev
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Slightly modified by AK to not assume 0 terminated input.
+ */
+
+#include <stdlib.h>
+#include "jsmn.h"
+
+/*
+ * Allocates a fresh unused token from the token pull.
+ */
+static jsmntok_t *jsmn_alloc_token(jsmn_parser *parser,
+				   jsmntok_t *tokens, size_t num_tokens)
+{
+	jsmntok_t *tok;
+
+	if ((unsigned)parser->toknext >= num_tokens)
+		return NULL;
+	tok = &tokens[parser->toknext++];
+	tok->start = tok->end = -1;
+	tok->size = 0;
+	return tok;
+}
+
+/*
+ * Fills token type and boundaries.
+ */
+static void jsmn_fill_token(jsmntok_t *token, jsmntype_t type,
+			    int start, int end)
+{
+	token->type = type;
+	token->start = start;
+	token->end = end;
+	token->size = 0;
+}
+
+/*
+ * Fills next available token with JSON primitive.
+ */
+static jsmnerr_t jsmn_parse_primitive(jsmn_parser *parser, const char *js,
+				      size_t len,
+				      jsmntok_t *tokens, size_t num_tokens)
+{
+	jsmntok_t *token;
+	int start;
+
+	start = parser->pos;
+
+	for (; parser->pos < len; parser->pos++) {
+		switch (js[parser->pos]) {
+#ifndef JSMN_STRICT
+		/*
+		 * In strict mode primitive must be followed by ","
+		 * or "}" or "]"
+		 */
+		case ':':
+#endif
+		case '\t':
+		case '\r':
+		case '\n':
+		case ' ':
+		case ',':
+		case ']':
+		case '}':
+			goto found;
+		default:
+			break;
+		}
+		if (js[parser->pos] < 32 || js[parser->pos] >= 127) {
+			parser->pos = start;
+			return JSMN_ERROR_INVAL;
+		}
+	}
+#ifdef JSMN_STRICT
+	/*
+	 * In strict mode primitive must be followed by a
+	 * comma/object/array.
+	 */
+	parser->pos = start;
+	return JSMN_ERROR_PART;
+#endif
+
+found:
+	token = jsmn_alloc_token(parser, tokens, num_tokens);
+	if (token == NULL) {
+		parser->pos = start;
+		return JSMN_ERROR_NOMEM;
+	}
+	jsmn_fill_token(token, JSMN_PRIMITIVE, start, parser->pos);
+	parser->pos--;
+	return JSMN_SUCCESS;
+}
+
+/*
+ * Fills next token with JSON string.
+ */
+static jsmnerr_t jsmn_parse_string(jsmn_parser *parser, const char *js,
+				   size_t len,
+				   jsmntok_t *tokens, size_t num_tokens)
+{
+	jsmntok_t *token;
+	int start = parser->pos;
+
+	parser->pos++;
+
+	/* Skip starting quote */
+	for (; parser->pos < len; parser->pos++) {
+		char c = js[parser->pos];
+
+		/* Quote: end of string */
+		if (c == '\"') {
+			token = jsmn_alloc_token(parser, tokens, num_tokens);
+			if (token == NULL) {
+				parser->pos = start;
+				return JSMN_ERROR_NOMEM;
+			}
+			jsmn_fill_token(token, JSMN_STRING, start+1,
+					parser->pos);
+			return JSMN_SUCCESS;
+		}
+
+		/* Backslash: Quoted symbol expected */
+		if (c == '\\') {
+			parser->pos++;
+			switch (js[parser->pos]) {
+				/* Allowed escaped symbols */
+			case '\"':
+			case '/':
+			case '\\':
+			case 'b':
+			case 'f':
+			case 'r':
+			case 'n':
+			case 't':
+				break;
+				/* Allows escaped symbol \uXXXX */
+			case 'u':
+				/* TODO */
+				break;
+				/* Unexpected symbol */
+			default:
+				parser->pos = start;
+				return JSMN_ERROR_INVAL;
+			}
+		}
+	}
+	parser->pos = start;
+	return JSMN_ERROR_PART;
+}
+
+/*
+ * Parse JSON string and fill tokens.
+ */
+jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len,
+		     jsmntok_t *tokens,
+		     unsigned int num_tokens)
+{
+	jsmnerr_t r;
+	int i;
+	jsmntok_t *token;
+
+	for (; parser->pos < len; parser->pos++) {
+		char c;
+		jsmntype_t type;
+
+		c = js[parser->pos];
+		switch (c) {
+		case '{':
+		case '[':
+			token = jsmn_alloc_token(parser, tokens, num_tokens);
+			if (token == NULL)
+				return JSMN_ERROR_NOMEM;
+			if (parser->toksuper != -1)
+				tokens[parser->toksuper].size++;
+			token->type = (c == '{' ? JSMN_OBJECT : JSMN_ARRAY);
+			token->start = parser->pos;
+			parser->toksuper = parser->toknext - 1;
+			break;
+		case '}':
+		case ']':
+			type = (c == '}' ? JSMN_OBJECT : JSMN_ARRAY);
+			for (i = parser->toknext - 1; i >= 0; i--) {
+				token = &tokens[i];
+				if (token->start != -1 && token->end == -1) {
+					if (token->type != type)
+						return JSMN_ERROR_INVAL;
+					parser->toksuper = -1;
+					token->end = parser->pos + 1;
+					break;
+				}
+			}
+			/* Error if unmatched closing bracket */
+			if (i == -1)
+				return JSMN_ERROR_INVAL;
+			for (; i >= 0; i--) {
+				token = &tokens[i];
+				if (token->start != -1 && token->end == -1) {
+					parser->toksuper = i;
+					break;
+				}
+			}
+			break;
+		case '\"':
+			r = jsmn_parse_string(parser, js, len, tokens,
+					      num_tokens);
+			if (r < 0)
+				return r;
+			if (parser->toksuper != -1)
+				tokens[parser->toksuper].size++;
+			break;
+		case '\t':
+		case '\r':
+		case '\n':
+		case ':':
+		case ',':
+		case ' ':
+			break;
+#ifdef JSMN_STRICT
+			/*
+			 * In strict mode primitives are:
+			 * numbers and booleans.
+			 */
+		case '-':
+		case '0':
+		case '1':
+		case '2':
+		case '3':
+		case '4':
+		case '5':
+		case '6':
+		case '7':
+		case '8':
+		case '9':
+		case 't':
+		case 'f':
+		case 'n':
+#else
+			/*
+			 * In non-strict mode every unquoted value
+			 * is a primitive.
+			 */
+		default:
+#endif
+			r = jsmn_parse_primitive(parser, js, len, tokens,
+						 num_tokens);
+			if (r < 0)
+				return r;
+			if (parser->toksuper != -1)
+				tokens[parser->toksuper].size++;
+			break;
+
+#ifdef JSMN_STRICT
+			/* Unexpected char in strict mode */
+		default:
+			return JSMN_ERROR_INVAL;
+#endif
+		}
+	}
+
+	for (i = parser->toknext - 1; i >= 0; i--) {
+		/* Unmatched opened object or array */
+		if (tokens[i].start != -1 && tokens[i].end == -1)
+			return JSMN_ERROR_PART;
+	}
+
+	return JSMN_SUCCESS;
+}
+
+/*
+ * Creates a new parser based over a given  buffer with an array of tokens
+ * available.
+ */
+void jsmn_init(jsmn_parser *parser)
+{
+	parser->pos = 0;
+	parser->toknext = 0;
+	parser->toksuper = -1;
+}
diff --git a/tools/perf/util/jsmn.h b/tools/perf/util/jsmn.h
new file mode 100644
index 0000000..8f432b0
--- /dev/null
+++ b/tools/perf/util/jsmn.h
@@ -0,0 +1,65 @@
+#ifndef __JSMN_H_
+#define __JSMN_H_
+
+/*
+ * JSON type identifier. Basic types are:
+ *	o Object
+ *	o Array
+ *	o String
+ *	o Other primitive: number, boolean (true/false) or null
+ */
+typedef enum {
+	JSMN_PRIMITIVE = 0,
+	JSMN_OBJECT = 1,
+	JSMN_ARRAY = 2,
+	JSMN_STRING = 3
+} jsmntype_t;
+
+typedef enum {
+	/* Not enough tokens were provided */
+	JSMN_ERROR_NOMEM = -1,
+	/* Invalid character inside JSON string */
+	JSMN_ERROR_INVAL = -2,
+	/* The string is not a full JSON packet, more bytes expected */
+	JSMN_ERROR_PART = -3,
+	/* Everything was fine */
+	JSMN_SUCCESS = 0
+} jsmnerr_t;
+
+/*
+ * JSON token description.
+ * @param		type	type (object, array, string etc.)
+ * @param		start	start position in JSON data string
+ * @param		end		end position in JSON data string
+ */
+typedef struct {
+	jsmntype_t type;
+	int start;
+	int end;
+	int size;
+} jsmntok_t;
+
+/*
+ * JSON parser. Contains an array of token blocks available. Also stores
+ * the string being parsed now and current position in that string
+ */
+typedef struct {
+	unsigned int pos; /* offset in the JSON string */
+	int toknext; /* next token to allocate */
+	int toksuper; /* superior token node, e.g parent object or array */
+} jsmn_parser;
+
+/*
+ * Create JSON parser over an array of tokens
+ */
+void jsmn_init(jsmn_parser *parser);
+
+/*
+ * Run JSON parser. It parses a JSON data string into and array of tokens,
+ * each describing a single JSON object.
+ */
+jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js,
+		     size_t len,
+		     jsmntok_t *tokens, unsigned int num_tokens);
+
+#endif /* __JSMN_H_ */
diff --git a/tools/perf/util/json.c b/tools/perf/util/json.c
new file mode 100644
index 0000000..e505da9
--- /dev/null
+++ b/tools/perf/util/json.c
@@ -0,0 +1,155 @@
+/* Parse JSON files using the JSMN parser. */
+
+/*
+ * Copyright (c) 2014, Intel Corporation
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+*/
+
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/fcntl.h>
+#include <stdio.h>
+#include <unistd.h>
+#include "jsmn.h"
+#include "json.h"
+#include <linux/kernel.h>
+
+static char *mapfile(const char *fn, size_t *size)
+{
+	unsigned ps = sysconf(_SC_PAGESIZE);
+	struct stat st;
+	char *map = NULL;
+	int err;
+	int fd = open(fn, O_RDONLY);
+
+	if (fd < 0)
+		return NULL;
+	err = fstat(fd, &st);
+	if (err < 0)
+		goto out;
+	*size = st.st_size;
+	map = mmap(NULL,
+		   (st.st_size + ps - 1) & ~(ps -1),
+		   PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
+	if (map == (char *)MAP_FAILED)
+		map = NULL;
+out:
+	close(fd);
+	return map;
+}
+
+static void unmapfile(char *map, size_t size)
+{
+	unsigned ps = sysconf(_SC_PAGESIZE);
+	munmap(map, roundup(size, ps));
+}
+
+/*
+ * Parse json file using jsmn. Return array of tokens,
+ * and mapped file. Caller needs to free array.
+ */
+jsmntok_t *parse_json(const char *fn, char **map, size_t *size, int *len)
+{
+	jsmn_parser parser;
+	jsmntok_t *tokens;
+	jsmnerr_t res;
+	unsigned sz;
+
+	*map = mapfile(fn, size);
+	if (!*map)
+		return NULL;
+	/* Heuristic */
+	sz = *size * 16;
+	tokens = malloc(sz);
+	if (!tokens)
+		goto error;
+	jsmn_init(&parser);
+	res = jsmn_parse(&parser, *map, *size, tokens,
+			 sz / sizeof(jsmntok_t));
+	if (res != JSMN_SUCCESS) {
+		pr_err("%s: json error %d\n", fn, res);
+		goto error_free;
+	}
+	if (len)
+		*len = parser.toknext;
+	return tokens;
+error_free:
+	free(tokens);
+error:
+	unmapfile(*map, *size);
+	return NULL;
+}
+
+void free_json(char *map, size_t size, jsmntok_t *tokens)
+{
+	free(tokens);
+	unmapfile(map, size);
+}
+
+static int countchar(char *map, char c, int end)
+{
+	int i;
+	int count = 0;
+	for (i = 0; i < end; i++)
+		if (map[i] == c)
+			count++;
+	return count;
+}
+
+/* Return line number of a jsmn token */
+int json_line(char *map, jsmntok_t *t)
+{
+	return countchar(map, '\n', t->start) + 1;
+}
+
+static const char *jsmn_types[] = {
+	[JSMN_PRIMITIVE] = "primitive",
+	[JSMN_ARRAY] = "array",
+	[JSMN_OBJECT] = "object",
+	[JSMN_STRING] = "string"
+};
+
+#define LOOKUP(a, i) ((i) < (sizeof(a)/sizeof(*(a))) ? ((a)[i]) : "?")
+
+/* Return type name of a jsmn token */
+const char *json_name(jsmntok_t *t)
+{
+	return LOOKUP(jsmn_types, t->type);
+}
+
+int json_len(jsmntok_t *t)
+{
+	return t->end - t->start;
+}
+
+/* Is string t equal to s? */
+int json_streq(char *map, jsmntok_t *t, const char *s)
+{
+	unsigned len = t->end - t->start;
+	return len == strlen(s) && !strncasecmp(map + t->start, s, len);
+}
diff --git a/tools/perf/util/json.h b/tools/perf/util/json.h
new file mode 100644
index 0000000..37dd9e9
--- /dev/null
+++ b/tools/perf/util/json.h
@@ -0,0 +1,13 @@
+#ifndef JSON_H
+#define JSON_H 1
+
+#include "jsmn.h"
+
+jsmntok_t *parse_json(const char *fn, char **map, size_t *size, int *len);
+void free_json(char *map, size_t size, jsmntok_t *tokens);
+int json_line(char *map, jsmntok_t *t);
+const char *json_name(jsmntok_t *t);
+int json_streq(char *map, jsmntok_t *t, const char *s);
+int json_len(jsmntok_t *t);
+
+#endif
-- 
1.9.0


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

* [PATCH 2/9] perf, tools: Add support for text descriptions of events and alias add
  2014-05-12 22:51 perf: Add support for full Intel event lists v3 Andi Kleen
  2014-05-12 22:51 ` [PATCH 1/9] perf, tools: Add jsmn `jasmine' JSON parser Andi Kleen
@ 2014-05-12 22:51 ` Andi Kleen
  2014-05-12 22:51 ` [PATCH 3/9] perf, tools: Add support for reading JSON event files Andi Kleen
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Andi Kleen @ 2014-05-12 22:51 UTC (permalink / raw)
  To: acme; +Cc: linux-kernel, peterz, eranian, namhyung, jolsa, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

Change pmu.c to allow descriptions of events and add interfaces
to add aliases. Used in the next patch.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/pmu.c | 127 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 98 insertions(+), 29 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 7a811eb..baec090 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -14,6 +14,7 @@
 
 struct perf_pmu_alias {
 	char *name;
+	char *desc;
 	struct list_head terms;
 	struct list_head list;
 	char unit[UNIT_MAX_LEN+1];
@@ -171,17 +172,12 @@ error:
 	return -1;
 }
 
-static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FILE *file)
+static int __perf_pmu__new_alias(struct list_head *list, char *name,
+				 char *dir, char *desc, char *val)
 {
 	struct perf_pmu_alias *alias;
-	char buf[256];
 	int ret;
 
-	ret = fread(buf, 1, sizeof(buf), file);
-	if (ret == 0)
-		return -EINVAL;
-	buf[ret] = 0;
-
 	alias = malloc(sizeof(*alias));
 	if (!alias)
 		return -ENOMEM;
@@ -190,24 +186,45 @@ static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FI
 	alias->scale = 1.0;
 	alias->unit[0] = '\0';
 
-	ret = parse_events_terms(&alias->terms, buf);
+	ret = parse_events_terms(&alias->terms, val);
 	if (ret) {
+		pr_err("Cannot parse alias %s: %d\n", val, ret);
 		free(alias);
 		return ret;
 	}
 
 	alias->name = strdup(name);
-	/*
-	 * load unit name and scale if available
-	 */
-	perf_pmu__parse_unit(alias, dir, name);
-	perf_pmu__parse_scale(alias, dir, name);
 
+	if (dir) {
+		/*
+		 * load unit name and scale if available
+		 */
+		perf_pmu__parse_unit(alias, dir, name);
+		perf_pmu__parse_scale(alias, dir, name);
+	}
+
+	alias->desc = desc ? strdup(desc) : NULL;
 	list_add_tail(&alias->list, list);
 
 	return 0;
 }
 
+static int perf_pmu__new_alias(struct list_head *list,
+			       char *dir,
+			       char *name,
+			       FILE *file)
+{
+	char buf[256];
+	int ret;
+
+	ret = fread(buf, 1, sizeof(buf), file);
+	if (ret == 0)
+		return -EINVAL;
+	buf[ret] = 0;
+
+	return __perf_pmu__new_alias(list, name, dir, NULL, buf);
+}
+
 /*
  * Process all the sysfs attributes located under the directory
  * specified in 'dir' parameter.
@@ -720,11 +737,51 @@ static char *format_alias_or(char *buf, int len, struct perf_pmu *pmu,
 	return buf;
 }
 
-static int cmp_string(const void *a, const void *b)
+struct pair {
+	char *name;
+	char *desc;
+};
+
+static int cmp_pair(const void *a, const void *b)
+{
+	const struct pair *as = a;
+	const struct pair *bs = b;
+
+	/* Put downloaded event list last */
+	if (!!as->desc != !!bs->desc)
+		return !!as->desc - !!bs->desc;
+	return strcmp(as->name, bs->name);
+}
+
+static void wordwrap(char *s, int start, int max, int corr)
 {
-	const char * const *as = a;
-	const char * const *bs = b;
-	return strcmp(*as, *bs);
+	int column = start;
+	int n;
+
+	while (*s) {
+		int wlen = strcspn(s, " \t");
+
+		if (column + wlen >= max && column > start) {
+			printf("\n%*s", start, "");
+			column = start + corr;
+		}
+		n = printf("%s%.*s", column > start ? " " : "", wlen, s);
+		if (n <= 0)
+			break;
+		s += wlen;
+		column += n;
+		while (isspace(*s))
+			s++;
+	}
+}
+
+static int get_columns(void)
+{
+	/*
+	 * Should ask the terminal with TIOCGWINSZ here, but we
+	 * need the original fd before the pager.
+	 */
+	return 79;
 }
 
 void print_pmu_events(const char *event_glob, bool name_only)
@@ -734,21 +791,24 @@ void print_pmu_events(const char *event_glob, bool name_only)
 	char buf[1024];
 	int printed = 0;
 	int len, j;
-	char **aliases;
+	struct pair *aliases;
+	int numdesc = 0;
+	int columns = get_columns();
 
 	pmu = NULL;
 	len = 0;
 	while ((pmu = perf_pmu__scan(pmu)) != NULL)
 		list_for_each_entry(alias, &pmu->aliases, list)
 			len++;
-	aliases = malloc(sizeof(char *) * len);
+	aliases = malloc(sizeof(struct pair) * len);
 	if (!aliases)
 		return;
 	pmu = NULL;
 	j = 0;
 	while ((pmu = perf_pmu__scan(pmu)) != NULL)
 		list_for_each_entry(alias, &pmu->aliases, list) {
-			char *name = format_alias(buf, sizeof(buf), pmu, alias);
+			char *name = alias->desc ? alias->name :
+				format_alias(buf, sizeof(buf), pmu, alias);
 			bool is_cpu = !strcmp(pmu->name, "cpu");
 
 			if (event_glob != NULL &&
@@ -756,22 +816,31 @@ void print_pmu_events(const char *event_glob, bool name_only)
 			      (!is_cpu && strglobmatch(alias->name,
 						       event_glob))))
 				continue;
-			aliases[j] = name;
-			if (is_cpu && !name_only)
-				aliases[j] = format_alias_or(buf, sizeof(buf),
-							      pmu, alias);
-			aliases[j] = strdup(aliases[j]);
+			aliases[j].name = name;
+			if (is_cpu && !name_only && !alias->desc)
+				aliases[j].name = format_alias_or(buf,
+								  sizeof(buf),
+								  pmu, alias);
+			aliases[j].name = strdup(aliases[j].name);
+			aliases[j].desc = alias->desc;
 			j++;
 		}
 	len = j;
-	qsort(aliases, len, sizeof(char *), cmp_string);
+	qsort(aliases, len, sizeof(struct pair), cmp_pair);
 	for (j = 0; j < len; j++) {
 		if (name_only) {
-			printf("%s ", aliases[j]);
+			printf("%s ", aliases[j].name);
 			continue;
 		}
-		printf("  %-50s [Kernel PMU event]\n", aliases[j]);
-		zfree(&aliases[j]);
+		if (aliases[j].desc) {
+			if (numdesc++ == 0 && printed)
+				printf("\n");
+			printf("  %-50s [", aliases[j].name);
+			wordwrap(aliases[j].desc, 53, columns, 1);
+			printf("]\n");
+		} else
+			printf("  %-50s [Kernel PMU event]\n", aliases[j].name);
+		zfree(&aliases[j].name);
 		printed++;
 	}
 	if (printed)
-- 
1.9.0


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

* [PATCH 3/9] perf, tools: Add support for reading JSON event files
  2014-05-12 22:51 perf: Add support for full Intel event lists v3 Andi Kleen
  2014-05-12 22:51 ` [PATCH 1/9] perf, tools: Add jsmn `jasmine' JSON parser Andi Kleen
  2014-05-12 22:51 ` [PATCH 2/9] perf, tools: Add support for text descriptions of events and alias add Andi Kleen
@ 2014-05-12 22:51 ` Andi Kleen
  2014-05-14  5:10   ` Namhyung Kim
  2014-05-12 22:51 ` [PATCH 4/9] perf, tools: Automatically look for event file name for cpu v2 Andi Kleen
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Andi Kleen @ 2014-05-12 22:51 UTC (permalink / raw)
  To: acme; +Cc: linux-kernel, peterz, eranian, namhyung, jolsa, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

Add a parser for Intel style JSON event files. This allows
to use an Intel event list directly with perf. The Intel
event lists can be quite large and are too big to store
in unswappable kernel memory.

The parser code knows how to convert the JSON fields
to perf fields. The conversion code is straight forward.
It knows (very little) Intel specific information, and can be easily
extended to handle fields for other CPUs.

The parser code is partially shared with an independent parsing
library, which is 2-clause BSD licenced. To avoid any conflicts I marked
those files as BSD licenced too. As part of perf they become GPLv2.

The events are handled using the existing alias machinery.

We output the BriefDescription in perf list.

Right now the json file can be specified as an argument
to perf stat/record/list. Followon patches will automate this.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-list.txt   |   6 +
 tools/perf/Documentation/perf-record.txt |   3 +
 tools/perf/Documentation/perf-stat.txt   |   3 +
 tools/perf/Makefile.perf                 |   2 +
 tools/perf/builtin-list.c                |   2 +
 tools/perf/builtin-record.c              |   3 +
 tools/perf/builtin-stat.c                |   2 +
 tools/perf/util/jevents.c                | 248 +++++++++++++++++++++++++++++++
 tools/perf/util/jevents.h                |   3 +
 tools/perf/util/pmu.c                    |  14 ++
 tools/perf/util/pmu.h                    |   2 +
 11 files changed, 288 insertions(+)
 create mode 100644 tools/perf/util/jevents.c
 create mode 100644 tools/perf/util/jevents.h

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 6fce6a6..d399e04 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -15,6 +15,12 @@ DESCRIPTION
 This command displays the symbolic event types which can be selected in the
 various perf commands with the -e option.
 
+OPTIONS
+-------
+--json-file=::
+Specify JSON event list file to use for parsing events.
+
+
 [[EVENT_MODIFIERS]]
 EVENT MODIFIERS
 ---------------
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index c71b0f3..8a1e0d9 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -213,6 +213,9 @@ if combined with -a or -C options.
 After starting the program, wait msecs before measuring. This is useful to
 filter out the startup phase of the program, which is often very different.
 
+--json-file=::
+Specify JSON event list file to use for parsing events.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 29ee857..a946283 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -142,6 +142,9 @@ filter out the startup phase of the program, which is often very different.
 
 Print statistics of transactional execution if supported.
 
+--json-file=::
+Specify JSON event list file to use for parsing events.
+
 EXAMPLES
 --------
 
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 43d4109..8800838 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -302,6 +302,7 @@ LIB_H += ui/ui.h
 LIB_H += util/data.h
 LIB_H += util/jsmn.h
 LIB_H += util/json.h
+LIB_H += util/jevents.h
 
 LIB_OBJS += $(OUTPUT)util/abspath.o
 LIB_OBJS += $(OUTPUT)util/alias.o
@@ -377,6 +378,7 @@ LIB_OBJS += $(OUTPUT)util/srcline.o
 LIB_OBJS += $(OUTPUT)util/data.o
 LIB_OBJS += $(OUTPUT)util/jsmn.o
 LIB_OBJS += $(OUTPUT)util/json.o
+LIB_OBJS += $(OUTPUT)util/jevents.o
 
 LIB_OBJS += $(OUTPUT)ui/setup.o
 LIB_OBJS += $(OUTPUT)ui/helpline.o
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 011195e..ca3dd18 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -20,6 +20,8 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	int i;
 	const struct option list_options[] = {
+		OPT_STRING(0, "json-file", &json_file, "json file",
+			   "Read event json file"),
 		OPT_END()
 	};
 	const char * const list_usage[] = {
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e4c85b8..e4a5be0 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -25,6 +25,7 @@
 #include "util/cpumap.h"
 #include "util/thread_map.h"
 #include "util/data.h"
+#include "util/pmu.h"
 
 #include <unistd.h>
 #include <sched.h>
@@ -871,6 +872,8 @@ const struct option record_options[] = {
 		    "sample transaction flags (special events only)"),
 	OPT_BOOLEAN(0, "per-thread", &record.opts.target.per_thread,
 		    "use per-thread mmaps"),
+	OPT_STRING(0, "json-file", &json_file, "json file",
+		   "Read event json file"),
 	OPT_END()
 };
 
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 65a151e..461ff0a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1668,6 +1668,8 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 		     "aggregate counts per physical processor core", AGGR_CORE),
 	OPT_UINTEGER('D', "delay", &initial_delay,
 		     "ms to wait before starting measurement after program start"),
+	OPT_STRING(0, "json-file", &json_file, "json file",
+		   "Read event json file"),
 	OPT_END()
 	};
 	const char * const stat_usage[] = {
diff --git a/tools/perf/util/jevents.c b/tools/perf/util/jevents.c
new file mode 100644
index 0000000..0173b68
--- /dev/null
+++ b/tools/perf/util/jevents.c
@@ -0,0 +1,248 @@
+/* Parse event JSON files */
+
+/*
+ * Copyright (c) 2014, Intel Corporation
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+*/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+#include <ctype.h>
+#include "jsmn.h"
+#include "json.h"
+#include "jevents.h"
+
+static void addfield(char *map, char **dst, const char *sep,
+		     const char *a, jsmntok_t *bt)
+{
+	unsigned len = strlen(a) + 1 + strlen(sep);
+	int olen = *dst ? strlen(*dst) : 0;
+	int blen = bt ? json_len(bt) : 0;
+
+	*dst = realloc(*dst, len + olen + blen);
+	if (!*dst)
+		exit(ENOMEM);
+	if (!olen)
+		*(*dst) = 0;
+	else
+		strcat(*dst, sep);
+	strcat(*dst, a);
+	if (bt)
+		strncat(*dst, map + bt->start, blen);
+}
+
+static void fixname(char *s)
+{
+	for (; *s; s++) {
+		*s = tolower(*s);
+		/*
+		 * Remove '.' for now, until the parser
+		 * can deal with it.
+		 */
+		if (*s == '.')
+			*s = '_';
+	}
+}
+
+static void fixdesc(char *s)
+{
+	char *e = s + strlen(s);
+
+	/* Remove trailing dots that look ugly in perf list */
+	--e;
+	while (e >= s && isspace(*e))
+		--e;
+	if (*e == '.')
+		*e = 0;
+}
+
+#define EXPECT(e, t, m) do { if (!(e)) {			\
+	jsmntok_t *loc = (t);					\
+	if (!(t)->start && (t) > tokens)			\
+		loc = (t) - 1;					\
+	fprintf(stderr, "%s:%d: " m ", got %s\n", fn,		\
+		json_line(map, loc),				\
+		json_name(t));					\
+	goto out_free;						\
+} } while (0)
+
+static struct msrmap {
+	const char *num;
+	const char *pname;
+} msrmap[] = {
+	{ "0x3F6", "ldlat=" },
+	{ "0x1A6", "offcore_rsp=" },
+	{ "0x1A7", "offcore_rsp=" },
+	{ NULL, NULL }
+};
+
+static struct field {
+	const char *field;
+	const char *kernel;
+} fields[] = {
+	{ "EventCode",	"event=" },
+	{ "UMask",	"umask=" },
+	{ "CounterMask", "cmask=" },
+	{ "Invert",	"inv=" },
+	{ "AnyThread",	"any=" },
+	{ "EdgeDetect",	"edge=" },
+	{ "SampleAfterValue", "period=" },
+	{ NULL, NULL }
+};
+
+static void cut_comma(char *map, jsmntok_t *newval)
+{
+	int i;
+
+	/* Cut off everything after comma */
+	for (i = newval->start; i < newval->end; i++) {
+		if (map[i] == ',')
+			newval->end = i;
+	}
+}
+
+static int match_field(char *map, jsmntok_t *field, int nz,
+		       char **event, jsmntok_t *val)
+{
+	struct field *f;
+	jsmntok_t newval = *val;
+
+	for (f = fields; f->field; f++)
+		if (json_streq(map, field, f->field) && nz) {
+			cut_comma(map, &newval);
+			addfield(map, event, ",", f->kernel, &newval);
+			return 1;
+		}
+	return 0;
+}
+
+static struct msrmap *lookup_msr(char *map, jsmntok_t *val)
+{
+	jsmntok_t newval = *val;
+	static bool warned;
+	int i;
+
+	cut_comma(map, &newval);
+	for (i = 0; msrmap[i].num; i++)
+		if (json_streq(map, &newval, msrmap[i].num))
+			return &msrmap[i];
+	if (!warned) {
+		warned = true;
+		pr_err("Unknown MSR in event file %.*s\n",
+			json_len(val), map + val->start);
+	}
+	return NULL;
+}
+
+/* Call func with each event in the json file */
+
+int json_events(const char *fn,
+	  int (*func)(void *data, char *name, char *event, char *desc),
+	  void *data)
+{
+	int err = -EIO;
+	size_t size;
+	jsmntok_t *tokens, *tok;
+	int i, j, len;
+	char *map;
+
+	tokens = parse_json(fn, &map, &size, &len);
+	if (!tokens)
+		return -EIO;
+	EXPECT(tokens->type == JSMN_ARRAY, tokens, "expected top level array");
+	tok = tokens + 1;
+	for (i = 0; i < tokens->size; i++) {
+		char *event = NULL, *desc = NULL, *name = NULL;
+		struct msrmap *msr = NULL;
+		jsmntok_t *msrval = NULL;
+		jsmntok_t *precise = NULL;
+		jsmntok_t *obj = tok++;
+
+		EXPECT(obj->type == JSMN_OBJECT, obj, "expected object");
+		for (j = 0; j < obj->size; j += 2) {
+			jsmntok_t *field, *val;
+			int nz;
+
+			field = tok + j;
+			EXPECT(field->type == JSMN_STRING, tok + j,
+			       "Expected field name");
+			val = tok + j + 1;
+			EXPECT(val->type == JSMN_STRING, tok + j + 1,
+			       "Expected string value");
+
+			nz = !json_streq(map, val, "0");
+			if (match_field(map, field, nz, &event, val)) {
+				/* ok */
+			} else if (json_streq(map, field, "EventName")) {
+				addfield(map, &name, "", "", val);
+			} else if (json_streq(map, field, "BriefDescription")) {
+				addfield(map, &desc, "", "", val);
+				fixdesc(desc);
+			} else if (json_streq(map, field, "PEBS") && nz &&
+				   !strstr(desc, "(Precise Event)")) {
+				precise = val;
+			} else if (json_streq(map, field, "MSRIndex") && nz) {
+				msr = lookup_msr(map, val);
+			} else if (json_streq(map, field, "MSRValue")) {
+				msrval = val;
+			} else if (json_streq(map, field, "Errata") &&
+				   !json_streq(map, val, "null")) {
+				addfield(map, &desc, ". ",
+					" Spec update: ", val);
+			} else if (json_streq(map, field, "Data_LA") && nz) {
+				addfield(map, &desc, ". ",
+					" Supports address when precise",
+					NULL);
+			}
+			/* ignore unknown fields */
+		}
+		if (precise) {
+			if (json_streq(map, precise, "2"))
+				addfield(map, &desc, " ", "(Must be precise)",
+						NULL);
+			else
+				addfield(map, &desc, " ",
+						"(Precise event)", NULL);
+		}
+		if (msr != NULL)
+			addfield(map, &event, ",", msr->pname, msrval);
+		fixname(name);
+		err = func(data, name, event, desc);
+		free(event);
+		free(desc);
+		free(name);
+		if (err)
+			break;
+		tok += j;
+	}
+	EXPECT(tok - tokens == len, tok, "unexpected objects at end");
+	err = 0;
+out_free:
+	free_json(map, size, tokens);
+	return err;
+}
diff --git a/tools/perf/util/jevents.h b/tools/perf/util/jevents.h
new file mode 100644
index 0000000..4c2b879
--- /dev/null
+++ b/tools/perf/util/jevents.h
@@ -0,0 +1,3 @@
+int json_events(const char *fn,
+		int (*func)(void *data, char *name, char *event, char *desc),
+		void *data);
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index baec090..9f154af 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -9,6 +9,9 @@
 #include "pmu.h"
 #include "parse-events.h"
 #include "cpumap.h"
+#include "jevents.h"
+
+const char *json_file;
 
 #define UNIT_MAX_LEN	31 /* max length for event unit name */
 
@@ -273,6 +276,14 @@ static int pmu_aliases_parse(char *dir, struct list_head *head)
 	return ret;
 }
 
+static int add_alias(void *data, char *name, char *event, char *desc)
+{
+	struct list_head *head = (struct list_head *)data;
+
+	__perf_pmu__new_alias(head, name, NULL, desc, event);
+	return 0;
+}
+
 /*
  * Reading the pmu event aliases definition, which should be located at:
  * /sys/bus/event_source/devices/<dev>/events as sysfs group attributes.
@@ -422,6 +433,9 @@ static struct perf_pmu *pmu_lookup(const char *name)
 	if (pmu_aliases(name, &aliases))
 		return NULL;
 
+	if (!strcmp(name, "cpu") && json_file)
+		json_events(json_file, add_alias, &aliases);
+
 	if (pmu_type(name, &type))
 		return NULL;
 
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index c14a543..583d21e 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -46,4 +46,6 @@ void print_pmu_events(const char *event_glob, bool name_only);
 bool pmu_have_event(const char *pname, const char *name);
 
 int perf_pmu__test(void);
+
+extern const char *json_file;
 #endif /* __PMU_H */
-- 
1.9.0


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

* [PATCH 4/9] perf, tools: Automatically look for event file name for cpu v2
  2014-05-12 22:51 perf: Add support for full Intel event lists v3 Andi Kleen
                   ` (2 preceding siblings ...)
  2014-05-12 22:51 ` [PATCH 3/9] perf, tools: Add support for reading JSON event files Andi Kleen
@ 2014-05-12 22:51 ` Andi Kleen
  2014-05-12 22:51 ` [PATCH 5/9] perf, tools: Add perf download to download event files v2 Andi Kleen
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Andi Kleen @ 2014-05-12 22:51 UTC (permalink / raw)
  To: acme; +Cc: linux-kernel, peterz, eranian, namhyung, jolsa, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

When no JSON event file is specified automatically look
for a suitable file in ~/.cache/pmu-events. A "perf download" can
automatically add files there for the current CPUs.

This does not include the actual event files with perf,
but they can be automatically downloaded instead
(implemented in the next patch)

This has the advantage that the events can be always uptodate,
because they are freshly downloaded. In oprofile we always
had problems with out of date or incomplete events files.

The event file format is per architecture, but can be
extended for other architectures.

v2: Supports XDG_CACHE_HOME and defaults to ~/.cache/pmu-events
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/arch/x86/Makefile      |  1 +
 tools/perf/arch/x86/util/cpustr.c | 34 ++++++++++++++++++++++++++++++++++
 tools/perf/util/jevents.c         | 31 +++++++++++++++++++++++++++++++
 tools/perf/util/jevents.h         |  1 +
 tools/perf/util/pmu.c             |  2 +-
 5 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/arch/x86/util/cpustr.c

diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
index 1641542..0efeb14 100644
--- a/tools/perf/arch/x86/Makefile
+++ b/tools/perf/arch/x86/Makefile
@@ -14,4 +14,5 @@ LIB_OBJS += $(OUTPUT)arch/$(ARCH)/tests/dwarf-unwind.o
 endif
 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o
 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/tsc.o
+LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/cpustr.o
 LIB_H += arch/$(ARCH)/util/tsc.h
diff --git a/tools/perf/arch/x86/util/cpustr.c b/tools/perf/arch/x86/util/cpustr.c
new file mode 100644
index 0000000..e1cd76c
--- /dev/null
+++ b/tools/perf/arch/x86/util/cpustr.c
@@ -0,0 +1,34 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include "../../util/jevents.h"
+
+char *get_cpu_str(void)
+{
+	char *line = NULL;
+	size_t llen = 0;
+	int found = 0, n;
+	char vendor[30];
+	int model, fam;
+	char *res = NULL;
+	FILE *f = fopen("/proc/cpuinfo", "r");
+
+	if (!f)
+		return NULL;
+	while (getline(&line, &llen, f) > 0) {
+		if (sscanf(line, "vendor_id : %30s", vendor) == 1)
+			found++;
+		else if (sscanf(line, "model : %d", &model) == 1)
+			found++;
+		else if (sscanf(line, "cpu family : %d", &fam) == 1)
+			found++;
+		if (found == 3) {
+			n = asprintf(&res, "%s-%d-%X-core", vendor, fam, model);
+			if (n < 0)
+				res = NULL;
+			break;
+		}
+	}
+	free(line);
+	fclose(f);
+	return res;
+}
diff --git a/tools/perf/util/jevents.c b/tools/perf/util/jevents.c
index 0173b68..89dada6 100644
--- a/tools/perf/util/jevents.c
+++ b/tools/perf/util/jevents.c
@@ -33,10 +33,39 @@
 #include <errno.h>
 #include <string.h>
 #include <ctype.h>
+#include "cache.h"
 #include "jsmn.h"
 #include "json.h"
 #include "jevents.h"
 
+__attribute__((weak)) char *get_cpu_str(void)
+{
+	return NULL;
+}
+
+static const char *json_default_name(void)
+{
+	char *cache = getenv("XDG_CACHE_HOME");
+	char *idstr = get_cpu_str();
+	char *res = NULL;
+	char *home = NULL;
+
+	if (!cache) {
+		home = getenv("HOME");
+		if (!home || asprintf(&cache, "%s/.cache", home) < 0)
+			goto out;
+	}
+	if (cache && idstr)
+		res = mkpath("%s/pmu-events/%s.json",
+			     cache,
+			     idstr);
+	if (home)
+		free(cache);
+out:
+	free(idstr);
+	return res;
+}
+
 static void addfield(char *map, char **dst, const char *sep,
 		     const char *a, jsmntok_t *bt)
 {
@@ -171,6 +200,8 @@ int json_events(const char *fn,
 	int i, j, len;
 	char *map;
 
+	if (!fn)
+		fn = json_default_name();
 	tokens = parse_json(fn, &map, &size, &len);
 	if (!tokens)
 		return -EIO;
diff --git a/tools/perf/util/jevents.h b/tools/perf/util/jevents.h
index 4c2b879..6a377a8 100644
--- a/tools/perf/util/jevents.h
+++ b/tools/perf/util/jevents.h
@@ -1,3 +1,4 @@
 int json_events(const char *fn,
 		int (*func)(void *data, char *name, char *event, char *desc),
 		void *data);
+char *get_cpu_str(void);
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 9f154af..fa21319 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -433,7 +433,7 @@ static struct perf_pmu *pmu_lookup(const char *name)
 	if (pmu_aliases(name, &aliases))
 		return NULL;
 
-	if (!strcmp(name, "cpu") && json_file)
+	if (!strcmp(name, "cpu"))
 		json_events(json_file, add_alias, &aliases);
 
 	if (pmu_type(name, &type))
-- 
1.9.0


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

* [PATCH 5/9] perf, tools: Add perf download to download event files v2
  2014-05-12 22:51 perf: Add support for full Intel event lists v3 Andi Kleen
                   ` (3 preceding siblings ...)
  2014-05-12 22:51 ` [PATCH 4/9] perf, tools: Automatically look for event file name for cpu v2 Andi Kleen
@ 2014-05-12 22:51 ` Andi Kleen
  2014-05-14  5:17   ` Namhyung Kim
  2014-05-12 22:51 ` [PATCH 6/9] perf, tools: Allow events with dot Andi Kleen
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Andi Kleen @ 2014-05-12 22:51 UTC (permalink / raw)
  To: acme; +Cc: linux-kernel, peterz, eranian, namhyung, jolsa, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

Add a downloader to automatically download the right
files from a download site.

This is implemented as a script calling wget, similar to
perf archive. The perf driver automatically calls the right
binary. The downloader is extensible, but currently only
implements an Intel event download.  It would be straightforward
to add other sites too for other vendors.

The downloaded event files are put into ~/.cache/pmu-events, where the
builtin event parser in util/* can find them automatically.

Cc: Use ~/.cache
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-download.txt | 28 ++++++++++++++++
 tools/perf/Documentation/perf-list.txt     | 12 ++++++-
 tools/perf/Makefile.perf                   |  5 ++-
 tools/perf/perf-download.sh                | 52 ++++++++++++++++++++++++++++++
 4 files changed, 95 insertions(+), 2 deletions(-)
 create mode 100644 tools/perf/Documentation/perf-download.txt
 create mode 100755 tools/perf/perf-download.sh

diff --git a/tools/perf/Documentation/perf-download.txt b/tools/perf/Documentation/perf-download.txt
new file mode 100644
index 0000000..84a1c14
--- /dev/null
+++ b/tools/perf/Documentation/perf-download.txt
@@ -0,0 +1,28 @@
+perf-download(1)
+===============
+
+NAME
+----
+perf-download - Download event files for current CPU.
+
+SYNOPSIS
+--------
+[verse]
+'perf download' [vendor-family-model]
+
+DESCRIPTION
+-----------
+This command automatically downloads event list for the current CPU and
+stores them in $XDG_CACHE_HOME/pmu-events (or $HOME/.cache/pmu-events).
+The other tools automatically look for them there. The CPU can be also
+specified at the command line.
+
+The downloading is done using http through wget, which needs
+to be installed.
+
+The user should regularly call this to download updated event lists
+for the current CPU.
+
+Note the downloaded files are stored per user, so if you use
+perf as normal user or with sudo you have to run perf download
+for both or specify --json-file for the users manually.
diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index d399e04..e8d5153 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -61,6 +61,16 @@ Sampling). Examples to use IBS:
  perf record -a -e r076:p ...          # same as -e cpu-cycles:p
  perf record -a -e r0C1:p ...          # use ibs op counting micro-ops
 
+PER CPU EVENT LISTS
+-------------------
+
+For some CPUs (particularly modern Intel CPUs) "perf download" can
+download additional CPU specific event definitions, which then
+become visible in perf list and available in the other perf tools.
+
+This obsoletes the raw event description method described below
+for most cases.
+
 RAW HARDWARE EVENT DESCRIPTOR
 -----------------------------
 Even when an event is not available in a symbolic form within perf right now,
@@ -123,6 +133,6 @@ types specified.
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-top[1],
-linkperf:perf-record[1],
+linkperf:perf-record[1], linkperf:perf-download[1],
 http://www.intel.com/Assets/PDF/manual/253669.pdf[Intel® 64 and IA-32 Architectures Software Developer's Manual Volume 3B: System Programming Guide],
 http://support.amd.com/us/Processor_TechDocs/24593_APM_v2.pdf[AMD64 Architecture Programmer’s Manual Volume 2: System Programming]
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 8800838..4dc1682 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -126,6 +126,7 @@ PYRF_OBJS =
 SCRIPT_SH =
 
 SCRIPT_SH += perf-archive.sh
+SCRIPT_SH += perf-download.sh
 
 grep-libs = $(filter -l%,$(1))
 strip-libs = $(filter-out -l%,$(1))
@@ -872,6 +873,8 @@ install-bin: all install-gtk
 		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)'
 	$(call QUIET_INSTALL, perf-archive) \
 		$(INSTALL) $(OUTPUT)perf-archive -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)'
+	$(call QUIET_INSTALL, perf-download) \
+		$(INSTALL) $(OUTPUT)perf-download -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)'
 ifndef NO_LIBPERL
 	$(call QUIET_INSTALL, perl-scripts) \
 		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/perl/Perf-Trace-Util/lib/Perf/Trace'; \
@@ -917,7 +920,7 @@ config-clean:
 	@$(MAKE) -C config/feature-checks clean >/dev/null
 
 clean: $(LIBTRACEEVENT)-clean $(LIBAPIKFS)-clean config-clean
-	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIB_OBJS) $(BUILTIN_OBJS) $(LIB_FILE) $(OUTPUT)perf-archive $(OUTPUT)perf.o $(LANG_BINDINGS) $(GTK_OBJS)
+	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIB_OBJS) $(BUILTIN_OBJS) $(LIB_FILE) $(OUTPUT)perf-archive $(OUTPUT)/perf-download $(OUTPUT)perf.o $(LANG_BINDINGS) $(GTK_OBJS)
 	$(call QUIET_CLEAN, core-progs) $(RM) $(ALL_PROGRAMS) perf
 	$(call QUIET_CLEAN, core-gen)   $(RM)  *.spec *.pyc *.pyo */*.pyc */*.pyo $(OUTPUT)common-cmds.h TAGS tags cscope* $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)PERF-CFLAGS $(OUTPUT)PERF-FEATURES $(OUTPUT)util/*-bison* $(OUTPUT)util/*-flex*
 	$(QUIET_SUBDIR0)Documentation $(QUIET_SUBDIR1) clean
diff --git a/tools/perf/perf-download.sh b/tools/perf/perf-download.sh
new file mode 100755
index 0000000..8f57a2a
--- /dev/null
+++ b/tools/perf/perf-download.sh
@@ -0,0 +1,52 @@
+#!/bin/bash
+# download event files for current cpu for perf
+
+WGETOPT=${WGETOPT:---no-verbose --timeout 5}
+
+set -e
+
+if [ "$1" == "" ] ; then
+	S=$(awk '
+/^vendor/     		{ V=$3 }
+/^model/ && $2 == ":" 	{ M=$3 }
+/^cpu family/ 		{ F = $4 }
+END	      { printf("%s-%s-%X", V, F, M) }' /proc/cpuinfo)
+else
+	S="$1"
+fi
+V=$(echo $S  | ( IFS=- read v f m ; echo $v) )
+
+CACHEDIR=${XDG_CACHE_HOME:-~/.cache}
+[ ! -d $CACHEDIR/pmu-events ] && mkdir -p $CACHEDIR/pmu-events
+cd $CACHEDIR/pmu-events
+
+case "$V" in
+GenuineIntel)
+	echo "Downloading models file"
+	URLBASE=${URLBASE:-https://download.01.org/perfmon}
+	MAPFILE=${MAPFILE:-mapfile.csv}
+	echo "Downloading readme.txt"
+	wget -N $WGETOPT $URLBASE/readme.txt
+	;;
+
+# Add more CPU vendors here
+
+*)
+	echo "Unsupported CPU vendor $V"
+	exit 1
+	;;
+esac
+
+wget -N $WGETOPT $URLBASE/$MAPFILE
+
+echo "Downloading events file"
+awk -v urlbase=$URLBASE -v cpu="$S" -F, \
+	'$1 == cpu && $4 == "core" { print urlbase $3; exit 0 }' \
+	$MAPFILE > url$$
+if [ -s url$$ ] ; then
+	wget $WGETOPT -q -i url$$ -O $S-core.json
+else
+	echo "CPU $S not found"
+fi
+rm -f url$$
+
-- 
1.9.0


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

* [PATCH 6/9] perf, tools: Allow events with dot
  2014-05-12 22:51 perf: Add support for full Intel event lists v3 Andi Kleen
                   ` (4 preceding siblings ...)
  2014-05-12 22:51 ` [PATCH 5/9] perf, tools: Add perf download to download event files v2 Andi Kleen
@ 2014-05-12 22:51 ` Andi Kleen
  2014-05-12 22:51 ` [PATCH 7/9] perf, tools: Query terminal width and use in perf list Andi Kleen
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Andi Kleen @ 2014-05-12 22:51 UTC (permalink / raw)
  To: acme; +Cc: linux-kernel, peterz, eranian, namhyung, jolsa, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

The Intel events use a dot to separate event name and unit mask.
Allow dot in names in the scanner, and remove special handling
of dot as EOF. Also remove the hack in jevents to replace dot
with underscore. This way dotted events can be specified
directly by the user.

I'm not fully sure this change to the scanner is correct
(what was the dot special case good for?), but I haven't
found anything that breaks with it so far at least.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/jevents.c      | 9 +--------
 tools/perf/util/parse-events.l | 3 +--
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/jevents.c b/tools/perf/util/jevents.c
index 89dada6..f86e510 100644
--- a/tools/perf/util/jevents.c
+++ b/tools/perf/util/jevents.c
@@ -87,15 +87,8 @@ static void addfield(char *map, char **dst, const char *sep,
 
 static void fixname(char *s)
 {
-	for (; *s; s++) {
+	for (; *s; s++)
 		*s = tolower(*s);
-		/*
-		 * Remove '.' for now, until the parser
-		 * can deal with it.
-		 */
-		if (*s == '.')
-			*s = '_';
-	}
 }
 
 static void fixdesc(char *s)
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 3432995..709fa3b 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -81,7 +81,7 @@ num_dec		[0-9]+
 num_hex		0x[a-fA-F0-9]+
 num_raw_hex	[a-fA-F0-9]+
 name		[a-zA-Z_*?][a-zA-Z0-9_*?]*
-name_minus	[a-zA-Z_*?][a-zA-Z0-9\-_*?]*
+name_minus	[a-zA-Z_*?][a-zA-Z0-9\-_*?.]*
 /* If you add a modifier you need to update check_modifier() */
 modifier_event	[ukhpGHSD]+
 modifier_bp	[rwx]{1,3}
@@ -119,7 +119,6 @@ modifier_bp	[rwx]{1,3}
 			return PE_EVENT_NAME;
 		}
 
-.		|
 <<EOF>>		{
 			BEGIN(INITIAL); yyless(0);
 		}
-- 
1.9.0


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

* [PATCH 7/9] perf, tools: Query terminal width and use in perf list
  2014-05-12 22:51 perf: Add support for full Intel event lists v3 Andi Kleen
                   ` (5 preceding siblings ...)
  2014-05-12 22:51 ` [PATCH 6/9] perf, tools: Allow events with dot Andi Kleen
@ 2014-05-12 22:51 ` Andi Kleen
  2014-05-12 22:51 ` [PATCH 8/9] perf, tools, test: Add test case for alias and JSON parsing Andi Kleen
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Andi Kleen @ 2014-05-12 22:51 UTC (permalink / raw)
  To: acme; +Cc: linux-kernel, peterz, eranian, namhyung, jolsa, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

Automatically adapt the now wider and word wrapped perf list
output to wider terminals. This requires querying the terminal
before the auto pager takes over, and exporting this
information from the pager subsystem.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/cache.h |  1 +
 tools/perf/util/pager.c | 15 +++++++++++++++
 tools/perf/util/pmu.c   | 12 ++----------
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 7b176dd..07527d6 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -31,6 +31,7 @@ extern void setup_pager(void);
 extern const char *pager_program;
 extern int pager_in_use(void);
 extern int pager_use_color;
+int pager_get_columns(void);
 
 char *alias_lookup(const char *alias);
 int split_cmdline(char *cmdline, const char ***argv);
diff --git a/tools/perf/util/pager.c b/tools/perf/util/pager.c
index 3322b84..60fa3fe 100644
--- a/tools/perf/util/pager.c
+++ b/tools/perf/util/pager.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "run-command.h"
 #include "sigchain.h"
+#include <sys/ioctl.h>
 
 /*
  * This is split up from the rest of git so that we can do
@@ -8,6 +9,7 @@
  */
 
 static int spawned_pager;
+static int pager_columns;
 
 static void pager_preexec(void)
 {
@@ -47,9 +49,12 @@ static void wait_for_pager_signal(int signo)
 void setup_pager(void)
 {
 	const char *pager = getenv("PERF_PAGER");
+	struct winsize sz;
 
 	if (!isatty(1))
 		return;
+	if (ioctl(1, TIOCGWINSZ, &sz) == 0)
+		pager_columns = sz.ws_col;
 	if (!pager) {
 		if (!pager_program)
 			perf_config(perf_default_config, NULL);
@@ -98,3 +103,13 @@ int pager_in_use(void)
 	env = getenv("PERF_PAGER_IN_USE");
 	return env ? perf_config_bool("PERF_PAGER_IN_USE", env) : 0;
 }
+
+int pager_get_columns(void)
+{
+	char *s;
+
+	s = getenv("COLUMNS");
+	if (s)
+		return atoi(s);
+	return (pager_columns ? pager_columns : 80) - 2;
+}
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index fa21319..8714f9a 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -9,6 +9,7 @@
 #include "pmu.h"
 #include "parse-events.h"
 #include "cpumap.h"
+#include "cache.h"
 #include "jevents.h"
 
 const char *json_file;
@@ -789,15 +790,6 @@ static void wordwrap(char *s, int start, int max, int corr)
 	}
 }
 
-static int get_columns(void)
-{
-	/*
-	 * Should ask the terminal with TIOCGWINSZ here, but we
-	 * need the original fd before the pager.
-	 */
-	return 79;
-}
-
 void print_pmu_events(const char *event_glob, bool name_only)
 {
 	struct perf_pmu *pmu;
@@ -807,7 +799,7 @@ void print_pmu_events(const char *event_glob, bool name_only)
 	int len, j;
 	struct pair *aliases;
 	int numdesc = 0;
-	int columns = get_columns();
+	int columns = pager_get_columns();
 
 	pmu = NULL;
 	len = 0;
-- 
1.9.0


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

* [PATCH 8/9] perf, tools, test: Add test case for alias and JSON parsing
  2014-05-12 22:51 perf: Add support for full Intel event lists v3 Andi Kleen
                   ` (6 preceding siblings ...)
  2014-05-12 22:51 ` [PATCH 7/9] perf, tools: Query terminal width and use in perf list Andi Kleen
@ 2014-05-12 22:51 ` Andi Kleen
  2014-05-14  5:27   ` Namhyung Kim
  2014-05-12 22:51 ` [PATCH 9/9] perf, tools, record: Always allow to overide default period v2 Andi Kleen
  2014-05-15 16:09 ` perf: Add support for full Intel event lists v3 Hagen Paul Pfeifer
  9 siblings, 1 reply; 21+ messages in thread
From: Andi Kleen @ 2014-05-12 22:51 UTC (permalink / raw)
  To: acme; +Cc: linux-kernel, peterz, eranian, namhyung, jolsa, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

Add a simple test case to perf test that runs perf download and parses
all the events. This needs adding an all event iterator to pmu.c

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Makefile.perf        |  1 +
 tools/perf/tests/aliases.c      | 52 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/builtin-test.c |  4 ++++
 tools/perf/tests/tests.h        |  1 +
 tools/perf/util/pmu.c           | 18 ++++++++++++++
 tools/perf/util/pmu.h           |  2 ++
 6 files changed, 78 insertions(+)
 create mode 100644 tools/perf/tests/aliases.c

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 4dc1682..3e6ed5a 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -417,6 +417,7 @@ endif
 LIB_OBJS += $(OUTPUT)tests/code-reading.o
 LIB_OBJS += $(OUTPUT)tests/sample-parsing.o
 LIB_OBJS += $(OUTPUT)tests/parse-no-sample-id-all.o
+LIB_OBJS += $(OUTPUT)tests/aliases.o
 ifndef NO_DWARF_UNWIND
 ifeq ($(ARCH),x86)
 LIB_OBJS += $(OUTPUT)tests/dwarf-unwind.o
diff --git a/tools/perf/tests/aliases.c b/tools/perf/tests/aliases.c
new file mode 100644
index 0000000..ec267d3
--- /dev/null
+++ b/tools/perf/tests/aliases.c
@@ -0,0 +1,52 @@
+/* Check if we can set up all aliases and can read JSON files */
+#include <stdlib.h>
+#include "tests.h"
+#include "pmu.h"
+#include "evlist.h"
+#include "parse-events.h"
+
+static struct perf_evlist *evlist;
+
+static int num_events;
+static int failed;
+
+static int test_event(const char *name)
+{
+	int ret = parse_events(evlist, name);
+
+	if (ret) {
+		/*
+		 * We only print on failure because common perf setups
+		 * have events that cannot be parsed.
+		 */
+		fprintf(stderr, "invalid or unsupported event: '%s'\n", name);
+		ret = 0;
+		failed++;
+	} else
+		num_events++;
+	return ret;
+}
+
+int test__aliases(void)
+{
+	int err;
+
+	/* Download JSON files */
+	/* XXX assumes perf is installed */
+	/* For now user must manually download */
+	if (0 && system("perf download > /dev/null") < 0) {
+		/* Don't error out for this for now */
+		fprintf(stderr, "perf download failed\n");
+	}
+
+	evlist = perf_evlist__new();
+	if (evlist == NULL)
+		return -ENOMEM;
+
+	err = pmu_iterate_events(test_event);
+	fprintf(stderr, " Parsed %d events :", num_events);
+	if (failed > 0)
+		fprintf(stderr, " %d events failed", num_events);
+	perf_evlist__delete(evlist);
+	return err;
+}
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 0d5afaf..d62601c 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -115,6 +115,10 @@ static struct test {
 		.desc = "Test parsing with no sample_id_all bit set",
 		.func = test__parse_no_sample_id_all,
 	},
+	{
+		.desc = "Test parsing JSON aliases",
+		.func = test__aliases,
+	},
 #if defined(__x86_64__) || defined(__i386__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
 	{
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index a9d7cb0..38e98df 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -44,6 +44,7 @@ int test__dwarf_unwind(void);
 int test__hists_filter(void);
 int test__mmap_thread_lookup(void);
 int test__thread_mg_share(void);
+int test__aliases(void);
 
 #if defined(__x86_64__) || defined(__i386__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 8714f9a..b87f520 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -869,3 +869,21 @@ bool pmu_have_event(const char *pname, const char *name)
 	}
 	return false;
 }
+
+int pmu_iterate_events(int (*func)(const char *name))
+{
+	int ret = 0;
+	struct perf_pmu *pmu;
+	struct perf_pmu_alias *alias;
+
+	perf_pmu__find("cpu"); /* Load PMUs */
+	pmu = NULL;
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		list_for_each_entry(alias, &pmu->aliases, list) {
+			ret = func(alias->name);
+			if (ret != 0)
+				break;
+		}
+	}
+	return ret;
+}
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 583d21e..a8ed283 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -47,5 +47,7 @@ bool pmu_have_event(const char *pname, const char *name);
 
 int perf_pmu__test(void);
 
+int pmu_iterate_events(int (*func)(const char *name));
+
 extern const char *json_file;
 #endif /* __PMU_H */
-- 
1.9.0


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

* [PATCH 9/9] perf, tools, record: Always allow to overide default period v2
  2014-05-12 22:51 perf: Add support for full Intel event lists v3 Andi Kleen
                   ` (7 preceding siblings ...)
  2014-05-12 22:51 ` [PATCH 8/9] perf, tools, test: Add test case for alias and JSON parsing Andi Kleen
@ 2014-05-12 22:51 ` Andi Kleen
  2014-05-14  5:50   ` Namhyung Kim
  2014-05-15 16:09 ` perf: Add support for full Intel event lists v3 Hagen Paul Pfeifer
  9 siblings, 1 reply; 21+ messages in thread
From: Andi Kleen @ 2014-05-12 22:51 UTC (permalink / raw)
  To: acme; +Cc: linux-kernel, peterz, eranian, namhyung, jolsa, Andi Kleen, fweisbec

From: Andi Kleen <ak@linux.intel.com>

Fix the logic to allow overriding event default periods with -c or -F
on the command line.  I'm not sure I understand this if() fully, but
this change makes all cases I tested work (tracepoint with default, default,
,-c, -F)

This fixed specifying -c / -F with json event list events,
which have a default period. It should do the same
for trace point events.

Cc: fweisbec@gmail.com
v2: Simplify, just change || to &&
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/evsel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 5c28d82..d9ceede 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -592,7 +592,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 	 * We default some events to a 1 default interval. But keep
 	 * it a weak assumption overridable by the user.
 	 */
-	if (!attr->sample_period || (opts->user_freq != UINT_MAX &&
+	if (!attr->sample_period && (opts->user_freq != UINT_MAX &&
 				     opts->user_interval != ULLONG_MAX)) {
 		if (opts->freq) {
 			perf_evsel__set_sample_bit(evsel, PERIOD);
-- 
1.9.0


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

* Re: [PATCH 1/9] perf, tools: Add jsmn `jasmine' JSON parser
  2014-05-12 22:51 ` [PATCH 1/9] perf, tools: Add jsmn `jasmine' JSON parser Andi Kleen
@ 2014-05-14  3:25   ` Namhyung Kim
  2014-05-14  5:00     ` Andi Kleen
  0 siblings, 1 reply; 21+ messages in thread
From: Namhyung Kim @ 2014-05-14  3:25 UTC (permalink / raw)
  To: Andi Kleen; +Cc: acme, linux-kernel, peterz, eranian, jolsa, Andi Kleen

Hi Andi,

On Mon, 12 May 2014 15:51:06 -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> I need a JSON parser. This adds the simplest JSON
> parser I could find -- Serge Zaitsev's jsmn `jasmine' --
> to the perf library. I merely converted it to (mostly)
> Linux style and added support for non 0 terminated input.

It seems like the original code now also supports it.

>
> The parser is quite straight forward and does not
> copy any data, just returns tokens with offsets
> into the input buffer. So it's relatively efficient
> and simple to use.
>
> The code is not fully checkpatch clean, but I didn't
> want to completely fork the upstream code.
>
> Original source: http://zserge.bitbucket.org/jsmn.html
>
> In addition I added a simple wrapper that mmaps a json
> file and provides some straight forward access functions.

I know you just ported the original source to perf, but there's some
nitpicks I'd like to point out anyway.. ;-)


>
> Used in follow-on patches to parse event files.
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/Makefile.perf |   4 +
>  tools/perf/util/jsmn.c   | 297 +++++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/jsmn.h   |  65 +++++++++++
>  tools/perf/util/json.c   | 155 +++++++++++++++++++++++++
>  tools/perf/util/json.h   |  13 +++
>  5 files changed, 534 insertions(+)
>  create mode 100644 tools/perf/util/jsmn.c
>  create mode 100644 tools/perf/util/jsmn.h
>  create mode 100644 tools/perf/util/json.c
>  create mode 100644 tools/perf/util/json.h
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 2baf61c..43d4109 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -300,6 +300,8 @@ LIB_H += ui/progress.h
>  LIB_H += ui/util.h
>  LIB_H += ui/ui.h
>  LIB_H += util/data.h
> +LIB_H += util/jsmn.h
> +LIB_H += util/json.h
>  
>  LIB_OBJS += $(OUTPUT)util/abspath.o
>  LIB_OBJS += $(OUTPUT)util/alias.o
> @@ -373,6 +375,8 @@ LIB_OBJS += $(OUTPUT)util/stat.o
>  LIB_OBJS += $(OUTPUT)util/record.o
>  LIB_OBJS += $(OUTPUT)util/srcline.o
>  LIB_OBJS += $(OUTPUT)util/data.o
> +LIB_OBJS += $(OUTPUT)util/jsmn.o
> +LIB_OBJS += $(OUTPUT)util/json.o
>  
>  LIB_OBJS += $(OUTPUT)ui/setup.o
>  LIB_OBJS += $(OUTPUT)ui/helpline.o
> diff --git a/tools/perf/util/jsmn.c b/tools/perf/util/jsmn.c
> new file mode 100644
> index 0000000..3e4b711
> --- /dev/null
> +++ b/tools/perf/util/jsmn.c
> @@ -0,0 +1,297 @@
> +/*
> + * Copyright (c) 2010 Serge A. Zaitsev
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + *
> + * Slightly modified by AK to not assume 0 terminated input.
> + */
> +
> +#include <stdlib.h>
> +#include "jsmn.h"
> +
> +/*
> + * Allocates a fresh unused token from the token pull.

s/pull/pool/ ?


> + */
> +static jsmntok_t *jsmn_alloc_token(jsmn_parser *parser,
> +				   jsmntok_t *tokens, size_t num_tokens)
> +{
> +	jsmntok_t *tok;
> +
> +	if ((unsigned)parser->toknext >= num_tokens)
> +		return NULL;
> +	tok = &tokens[parser->toknext++];
> +	tok->start = tok->end = -1;
> +	tok->size = 0;
> +	return tok;
> +}
> +
> +/*
> + * Fills token type and boundaries.
> + */
> +static void jsmn_fill_token(jsmntok_t *token, jsmntype_t type,
> +			    int start, int end)
> +{
> +	token->type = type;
> +	token->start = start;
> +	token->end = end;
> +	token->size = 0;

I think setting size to 0 is not needed here.

> +}
> +
> +/*
> + * Fills next available token with JSON primitive.
> + */
> +static jsmnerr_t jsmn_parse_primitive(jsmn_parser *parser, const char *js,
> +				      size_t len,
> +				      jsmntok_t *tokens, size_t num_tokens)
> +{
> +	jsmntok_t *token;
> +	int start;
> +
> +	start = parser->pos;
> +
> +	for (; parser->pos < len; parser->pos++) {
> +		switch (js[parser->pos]) {
> +#ifndef JSMN_STRICT

I guess we don't want to support strict mode..


> +		/*
> +		 * In strict mode primitive must be followed by ","
> +		 * or "}" or "]"
> +		 */
> +		case ':':
> +#endif
> +		case '\t':
> +		case '\r':
> +		case '\n':
> +		case ' ':
> +		case ',':
> +		case ']':
> +		case '}':
> +			goto found;
> +		default:
> +			break;
> +		}
> +		if (js[parser->pos] < 32 || js[parser->pos] >= 127) {
> +			parser->pos = start;
> +			return JSMN_ERROR_INVAL;
> +		}
> +	}
> +#ifdef JSMN_STRICT
> +	/*
> +	 * In strict mode primitive must be followed by a
> +	 * comma/object/array.
> +	 */
> +	parser->pos = start;
> +	return JSMN_ERROR_PART;
> +#endif
> +
> +found:
> +	token = jsmn_alloc_token(parser, tokens, num_tokens);
> +	if (token == NULL) {
> +		parser->pos = start;
> +		return JSMN_ERROR_NOMEM;
> +	}
> +	jsmn_fill_token(token, JSMN_PRIMITIVE, start, parser->pos);
> +	parser->pos--;

Why decrease the pos here?  Hmm.. looking at the code, it seems to make
sure that the parent sees closing braces.  So it'd better to add a
comment IMHO.


> +	return JSMN_SUCCESS;
> +}
> +
> +/*
> + * Fills next token with JSON string.
> + */
> +static jsmnerr_t jsmn_parse_string(jsmn_parser *parser, const char *js,
> +				   size_t len,
> +				   jsmntok_t *tokens, size_t num_tokens)
> +{
> +	jsmntok_t *token;
> +	int start = parser->pos;
> +
> +	parser->pos++;
> +
> +	/* Skip starting quote */

It'd better this comment to above the increment statement.


> +	for (; parser->pos < len; parser->pos++) {
> +		char c = js[parser->pos];
> +
> +		/* Quote: end of string */
> +		if (c == '\"') {
> +			token = jsmn_alloc_token(parser, tokens, num_tokens);
> +			if (token == NULL) {
> +				parser->pos = start;
> +				return JSMN_ERROR_NOMEM;
> +			}
> +			jsmn_fill_token(token, JSMN_STRING, start+1,
> +					parser->pos);
> +			return JSMN_SUCCESS;
> +		}
> +
> +		/* Backslash: Quoted symbol expected */
> +		if (c == '\\') {
> +			parser->pos++;
> +			switch (js[parser->pos]) {
> +				/* Allowed escaped symbols */
> +			case '\"':
> +			case '/':
> +			case '\\':
> +			case 'b':
> +			case 'f':
> +			case 'r':
> +			case 'n':
> +			case 't':
> +				break;
> +				/* Allows escaped symbol \uXXXX */
> +			case 'u':
> +				/* TODO */
> +				break;
> +				/* Unexpected symbol */
> +			default:
> +				parser->pos = start;
> +				return JSMN_ERROR_INVAL;
> +			}
> +		}
> +	}
> +	parser->pos = start;
> +	return JSMN_ERROR_PART;
> +}
> +
> +/*
> + * Parse JSON string and fill tokens.
> + */
> +jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len,
> +		     jsmntok_t *tokens,
> +		     unsigned int num_tokens)

Unnecessary line break.


> +{
> +	jsmnerr_t r;
> +	int i;
> +	jsmntok_t *token;
> +
> +	for (; parser->pos < len; parser->pos++) {
> +		char c;
> +		jsmntype_t type;
> +
> +		c = js[parser->pos];
> +		switch (c) {
> +		case '{':
> +		case '[':
> +			token = jsmn_alloc_token(parser, tokens, num_tokens);
> +			if (token == NULL)
> +				return JSMN_ERROR_NOMEM;
> +			if (parser->toksuper != -1)
> +				tokens[parser->toksuper].size++;

The names of 'toksuper' and 'size' look slightly confusing.  How about
changing them to 'parent' and 'nr_children'?


> +			token->type = (c == '{' ? JSMN_OBJECT : JSMN_ARRAY);
> +			token->start = parser->pos;
> +			parser->toksuper = parser->toknext - 1;
> +			break;
> +		case '}':
> +		case ']':
> +			type = (c == '}' ? JSMN_OBJECT : JSMN_ARRAY);
> +			for (i = parser->toknext - 1; i >= 0; i--) {
> +				token = &tokens[i];
> +				if (token->start != -1 && token->end == -1) {

I think it'd better to have this check as a function (or a macro).


> +					if (token->type != type)
> +						return JSMN_ERROR_INVAL;
> +					parser->toksuper = -1;
> +					token->end = parser->pos + 1;
> +					break;
> +				}
> +			}
> +			/* Error if unmatched closing bracket */
> +			if (i == -1)
> +				return JSMN_ERROR_INVAL;
> +			for (; i >= 0; i--) {
> +				token = &tokens[i];
> +				if (token->start != -1 && token->end == -1) {
> +					parser->toksuper = i;
> +					break;
> +				}
> +			}
> +			break;
> +		case '\"':
> +			r = jsmn_parse_string(parser, js, len, tokens,
> +					      num_tokens);
> +			if (r < 0)
> +				return r;
> +			if (parser->toksuper != -1)
> +				tokens[parser->toksuper].size++;
> +			break;
> +		case '\t':
> +		case '\r':
> +		case '\n':
> +		case ':':
> +		case ',':
> +		case ' ':
> +			break;
> +#ifdef JSMN_STRICT
> +			/*
> +			 * In strict mode primitives are:
> +			 * numbers and booleans.
> +			 */
> +		case '-':
> +		case '0':
> +		case '1':
> +		case '2':
> +		case '3':
> +		case '4':
> +		case '5':
> +		case '6':
> +		case '7':
> +		case '8':
> +		case '9':
> +		case 't':
> +		case 'f':
> +		case 'n':
> +#else
> +			/*
> +			 * In non-strict mode every unquoted value
> +			 * is a primitive.
> +			 */
> +		default:
> +#endif
> +			r = jsmn_parse_primitive(parser, js, len, tokens,
> +						 num_tokens);
> +			if (r < 0)
> +				return r;
> +			if (parser->toksuper != -1)
> +				tokens[parser->toksuper].size++;
> +			break;
> +
> +#ifdef JSMN_STRICT
> +			/* Unexpected char in strict mode */
> +		default:
> +			return JSMN_ERROR_INVAL;
> +#endif
> +		}
> +	}
> +
> +	for (i = parser->toknext - 1; i >= 0; i--) {
> +		/* Unmatched opened object or array */
> +		if (tokens[i].start != -1 && tokens[i].end == -1)
> +			return JSMN_ERROR_PART;
> +	}
> +
> +	return JSMN_SUCCESS;
> +}
> +
> +/*
> + * Creates a new parser based over a given  buffer with an array of tokens
> + * available.
> + */
> +void jsmn_init(jsmn_parser *parser)
> +{
> +	parser->pos = 0;
> +	parser->toknext = 0;
> +	parser->toksuper = -1;
> +}

I think the interface will be better if we pass num_tokens here and save
it in the parser.


> diff --git a/tools/perf/util/jsmn.h b/tools/perf/util/jsmn.h
> new file mode 100644
> index 0000000..8f432b0
> --- /dev/null
> +++ b/tools/perf/util/jsmn.h
> @@ -0,0 +1,65 @@
> +#ifndef __JSMN_H_
> +#define __JSMN_H_
> +
> +/*
> + * JSON type identifier. Basic types are:
> + *	o Object
> + *	o Array
> + *	o String
> + *	o Other primitive: number, boolean (true/false) or null
> + */
> +typedef enum {
> +	JSMN_PRIMITIVE = 0,
> +	JSMN_OBJECT = 1,
> +	JSMN_ARRAY = 2,
> +	JSMN_STRING = 3
> +} jsmntype_t;
> +
> +typedef enum {
> +	/* Not enough tokens were provided */
> +	JSMN_ERROR_NOMEM = -1,
> +	/* Invalid character inside JSON string */
> +	JSMN_ERROR_INVAL = -2,
> +	/* The string is not a full JSON packet, more bytes expected */
> +	JSMN_ERROR_PART = -3,
> +	/* Everything was fine */
> +	JSMN_SUCCESS = 0
> +} jsmnerr_t;

It'd be great if its also provide a strerror function to return the
above messages. :)

> +
> +/*
> + * JSON token description.
> + * @param		type	type (object, array, string etc.)
> + * @param		start	start position in JSON data string
> + * @param		end		end position in JSON data string
    * @param		size	number of child element (for object, array)

> + */
> +typedef struct {
> +	jsmntype_t type;
> +	int start;
> +	int end;
> +	int size;
> +} jsmntok_t;
> +
> +/*
> + * JSON parser. Contains an array of token blocks available. Also stores
> + * the string being parsed now and current position in that string
> + */
> +typedef struct {
> +	unsigned int pos; /* offset in the JSON string */
> +	int toknext; /* next token to allocate */
> +	int toksuper; /* superior token node, e.g parent object or array */
> +} jsmn_parser;
> +
> +/*
> + * Create JSON parser over an array of tokens
> + */
> +void jsmn_init(jsmn_parser *parser);
> +
> +/*
> + * Run JSON parser. It parses a JSON data string into and array of tokens,
> + * each describing a single JSON object.
> + */
> +jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js,
> +		     size_t len,
> +		     jsmntok_t *tokens, unsigned int num_tokens);
> +
> +#endif /* __JSMN_H_ */
> diff --git a/tools/perf/util/json.c b/tools/perf/util/json.c
> new file mode 100644
> index 0000000..e505da9
> --- /dev/null
> +++ b/tools/perf/util/json.c
> @@ -0,0 +1,155 @@
> +/* Parse JSON files using the JSMN parser. */
> +
> +/*
> + * Copyright (c) 2014, Intel Corporation
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright notice,
> + * this list of conditions and the following disclaimer.
> + *
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> + * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
> + * OF THE POSSIBILITY OF SUCH DAMAGE.
> +*/
> +
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <sys/fcntl.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include "jsmn.h"
> +#include "json.h"
> +#include <linux/kernel.h>
> +
> +static char *mapfile(const char *fn, size_t *size)
> +{
> +	unsigned ps = sysconf(_SC_PAGESIZE);

We can use the 'page_size' global variable for this.


> +	struct stat st;
> +	char *map = NULL;
> +	int err;
> +	int fd = open(fn, O_RDONLY);
> +
> +	if (fd < 0)
> +		return NULL;
> +	err = fstat(fd, &st);
> +	if (err < 0)
> +		goto out;
> +	*size = st.st_size;
> +	map = mmap(NULL,
> +		   (st.st_size + ps - 1) & ~(ps -1),

PERF_ALIGN(st.st_size, page_size)


> +		   PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
> +	if (map == (char *)MAP_FAILED)

Also the cast can be omitted IMHO.


> +		map = NULL;
> +out:
> +	close(fd);
> +	return map;
> +}
> +
> +static void unmapfile(char *map, size_t size)
> +{
> +	unsigned ps = sysconf(_SC_PAGESIZE);
> +	munmap(map, roundup(size, ps));

Ditto.  munmap(map, PERF_ALIGN(size, page_size)


> +}
> +
> +/*
> + * Parse json file using jsmn. Return array of tokens,
> + * and mapped file. Caller needs to free array.
> + */
> +jsmntok_t *parse_json(const char *fn, char **map, size_t *size, int *len)
> +{
> +	jsmn_parser parser;
> +	jsmntok_t *tokens;
> +	jsmnerr_t res;
> +	unsigned sz;
> +
> +	*map = mapfile(fn, size);
> +	if (!*map)
> +		return NULL;
> +	/* Heuristic */
> +	sz = *size * 16;

Is 16 sizeof(jsmntok_t) ?  If so, please use it instead of magic number.


> +	tokens = malloc(sz);
> +	if (!tokens)
> +		goto error;
> +	jsmn_init(&parser);
> +	res = jsmn_parse(&parser, *map, *size, tokens,
> +			 sz / sizeof(jsmntok_t));

Here you used it. :)


> +	if (res != JSMN_SUCCESS) {
> +		pr_err("%s: json error %d\n", fn, res);
> +		goto error_free;
> +	}
> +	if (len)
> +		*len = parser.toknext;
> +	return tokens;
> +error_free:
> +	free(tokens);
> +error:
> +	unmapfile(*map, *size);
> +	return NULL;
> +}
> +
> +void free_json(char *map, size_t size, jsmntok_t *tokens)
> +{
> +	free(tokens);
> +	unmapfile(map, size);
> +}
> +
> +static int countchar(char *map, char c, int end)
> +{
> +	int i;
> +	int count = 0;
> +	for (i = 0; i < end; i++)
> +		if (map[i] == c)
> +			count++;
> +	return count;
> +}
> +
> +/* Return line number of a jsmn token */
> +int json_line(char *map, jsmntok_t *t)
> +{
> +	return countchar(map, '\n', t->start) + 1;
> +}
> +
> +static const char *jsmn_types[] = {
> +	[JSMN_PRIMITIVE] = "primitive",
> +	[JSMN_ARRAY] = "array",
> +	[JSMN_OBJECT] = "object",
> +	[JSMN_STRING] = "string"
> +};
> +
> +#define LOOKUP(a, i) ((i) < (sizeof(a)/sizeof(*(a))) ? ((a)[i]) : "?")
> +
> +/* Return type name of a jsmn token */
> +const char *json_name(jsmntok_t *t)
> +{
> +	return LOOKUP(jsmn_types, t->type);
> +}
> +
> +int json_len(jsmntok_t *t)
> +{
> +	return t->end - t->start;
> +}
> +
> +/* Is string t equal to s? */
> +int json_streq(char *map, jsmntok_t *t, const char *s)
> +{
> +	unsigned len = t->end - t->start;

	unsigned len = json_len(t);

Thanks,
Namhyung


> +	return len == strlen(s) && !strncasecmp(map + t->start, s, len);
> +}
> diff --git a/tools/perf/util/json.h b/tools/perf/util/json.h
> new file mode 100644
> index 0000000..37dd9e9
> --- /dev/null
> +++ b/tools/perf/util/json.h
> @@ -0,0 +1,13 @@
> +#ifndef JSON_H
> +#define JSON_H 1
> +
> +#include "jsmn.h"
> +
> +jsmntok_t *parse_json(const char *fn, char **map, size_t *size, int *len);
> +void free_json(char *map, size_t size, jsmntok_t *tokens);
> +int json_line(char *map, jsmntok_t *t);
> +const char *json_name(jsmntok_t *t);
> +int json_streq(char *map, jsmntok_t *t, const char *s);
> +int json_len(jsmntok_t *t);
> +
> +#endif

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

* Re: [PATCH 1/9] perf, tools: Add jsmn `jasmine' JSON parser
  2014-05-14  3:25   ` Namhyung Kim
@ 2014-05-14  5:00     ` Andi Kleen
  0 siblings, 0 replies; 21+ messages in thread
From: Andi Kleen @ 2014-05-14  5:00 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Andi Kleen, acme, linux-kernel, peterz, eranian, jolsa

On Wed, May 14, 2014 at 12:25:44PM +0900, Namhyung Kim wrote:
> Hi Andi,
> 
> On Mon, 12 May 2014 15:51:06 -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> >
> > I need a JSON parser. This adds the simplest JSON
> > parser I could find -- Serge Zaitsev's jsmn `jasmine' --
> > to the perf library. I merely converted it to (mostly)
> > Linux style and added support for non 0 terminated input.
> 
> It seems like the original code now also supports it.

Sorry I don't want to fork this code.

I think we're better of staying near upstream than trying
to fix all those (unimportant) nit-picks.

Please only point out real errors. Also the code is intentionally
not trying to have a lot of dependencies on perf infrastructure.

-Andi

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

* Re: [PATCH 3/9] perf, tools: Add support for reading JSON event files
  2014-05-12 22:51 ` [PATCH 3/9] perf, tools: Add support for reading JSON event files Andi Kleen
@ 2014-05-14  5:10   ` Namhyung Kim
  2014-05-15 20:18     ` Andi Kleen
  0 siblings, 1 reply; 21+ messages in thread
From: Namhyung Kim @ 2014-05-14  5:10 UTC (permalink / raw)
  To: Andi Kleen; +Cc: acme, linux-kernel, peterz, eranian, jolsa, Andi Kleen

On Mon, 12 May 2014 15:51:08 -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> Add a parser for Intel style JSON event files. This allows
> to use an Intel event list directly with perf. The Intel
> event lists can be quite large and are too big to store
> in unswappable kernel memory.
>
> The parser code knows how to convert the JSON fields
> to perf fields. The conversion code is straight forward.
> It knows (very little) Intel specific information, and can be easily
> extended to handle fields for other CPUs.
>
> The parser code is partially shared with an independent parsing
> library, which is 2-clause BSD licenced. To avoid any conflicts I marked
> those files as BSD licenced too. As part of perf they become GPLv2.
>
> The events are handled using the existing alias machinery.
>
> We output the BriefDescription in perf list.
>
> Right now the json file can be specified as an argument
> to perf stat/record/list. Followon patches will automate this.

I think it'd be good to show an example json event file content in
the changelog and/or comment.

>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/Documentation/perf-list.txt   |   6 +
>  tools/perf/Documentation/perf-record.txt |   3 +
>  tools/perf/Documentation/perf-stat.txt   |   3 +
>  tools/perf/Makefile.perf                 |   2 +
>  tools/perf/builtin-list.c                |   2 +
>  tools/perf/builtin-record.c              |   3 +
>  tools/perf/builtin-stat.c                |   2 +
>  tools/perf/util/jevents.c                | 248 +++++++++++++++++++++++++++++++
>  tools/perf/util/jevents.h                |   3 +
>  tools/perf/util/pmu.c                    |  14 ++
>  tools/perf/util/pmu.h                    |   2 +
>  11 files changed, 288 insertions(+)
>  create mode 100644 tools/perf/util/jevents.c
>  create mode 100644 tools/perf/util/jevents.h
>
> diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> index 6fce6a6..d399e04 100644
> --- a/tools/perf/Documentation/perf-list.txt
> +++ b/tools/perf/Documentation/perf-list.txt
> @@ -15,6 +15,12 @@ DESCRIPTION
>  This command displays the symbolic event types which can be selected in the
>  various perf commands with the -e option.
>  
> +OPTIONS
> +-------
> +--json-file=::

Hmm.. I think the option name should not be tied to the implementation.
How about --event-file or --event-alias then?


> +Specify JSON event list file to use for parsing events.
> +
> +
>  [[EVENT_MODIFIERS]]
>  EVENT MODIFIERS
>  ---------------
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index c71b0f3..8a1e0d9 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -213,6 +213,9 @@ if combined with -a or -C options.
>  After starting the program, wait msecs before measuring. This is useful to
>  filter out the startup phase of the program, which is often very different.
>  
> +--json-file=::
> +Specify JSON event list file to use for parsing events.
> +
>  SEE ALSO
>  --------
>  linkperf:perf-stat[1], linkperf:perf-list[1]
> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> index 29ee857..a946283 100644
> --- a/tools/perf/Documentation/perf-stat.txt
> +++ b/tools/perf/Documentation/perf-stat.txt
> @@ -142,6 +142,9 @@ filter out the startup phase of the program, which is often very different.
>  
>  Print statistics of transactional execution if supported.
>  
> +--json-file=::
> +Specify JSON event list file to use for parsing events.
> +
>  EXAMPLES
>  --------
>  
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 43d4109..8800838 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -302,6 +302,7 @@ LIB_H += ui/ui.h
>  LIB_H += util/data.h
>  LIB_H += util/jsmn.h
>  LIB_H += util/json.h
> +LIB_H += util/jevents.h
>  
>  LIB_OBJS += $(OUTPUT)util/abspath.o
>  LIB_OBJS += $(OUTPUT)util/alias.o
> @@ -377,6 +378,7 @@ LIB_OBJS += $(OUTPUT)util/srcline.o
>  LIB_OBJS += $(OUTPUT)util/data.o
>  LIB_OBJS += $(OUTPUT)util/jsmn.o
>  LIB_OBJS += $(OUTPUT)util/json.o
> +LIB_OBJS += $(OUTPUT)util/jevents.o
>  
>  LIB_OBJS += $(OUTPUT)ui/setup.o
>  LIB_OBJS += $(OUTPUT)ui/helpline.o
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 011195e..ca3dd18 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -20,6 +20,8 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
>  {
>  	int i;
>  	const struct option list_options[] = {
> +		OPT_STRING(0, "json-file", &json_file, "json file",
> +			   "Read event json file"),
>  		OPT_END()
>  	};
>  	const char * const list_usage[] = {
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index e4c85b8..e4a5be0 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -25,6 +25,7 @@
>  #include "util/cpumap.h"
>  #include "util/thread_map.h"
>  #include "util/data.h"
> +#include "util/pmu.h"
>  
>  #include <unistd.h>
>  #include <sched.h>
> @@ -871,6 +872,8 @@ const struct option record_options[] = {
>  		    "sample transaction flags (special events only)"),
>  	OPT_BOOLEAN(0, "per-thread", &record.opts.target.per_thread,
>  		    "use per-thread mmaps"),
> +	OPT_STRING(0, "json-file", &json_file, "json file",
> +		   "Read event json file"),
>  	OPT_END()
>  };
>  
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 65a151e..461ff0a 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -1668,6 +1668,8 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
>  		     "aggregate counts per physical processor core", AGGR_CORE),
>  	OPT_UINTEGER('D', "delay", &initial_delay,
>  		     "ms to wait before starting measurement after program start"),
> +	OPT_STRING(0, "json-file", &json_file, "json file",
> +		   "Read event json file"),
>  	OPT_END()
>  	};
>  	const char * const stat_usage[] = {
> diff --git a/tools/perf/util/jevents.c b/tools/perf/util/jevents.c
> new file mode 100644
> index 0000000..0173b68
> --- /dev/null
> +++ b/tools/perf/util/jevents.c
> @@ -0,0 +1,248 @@
> +/* Parse event JSON files */
> +
> +/*
> + * Copyright (c) 2014, Intel Corporation
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright notice,
> + * this list of conditions and the following disclaimer.
> + *
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> + * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
> + * OF THE POSSIBILITY OF SUCH DAMAGE.
> +*/
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <ctype.h>
> +#include "jsmn.h"
> +#include "json.h"
> +#include "jevents.h"
> +
> +static void addfield(char *map, char **dst, const char *sep,
> +		     const char *a, jsmntok_t *bt)
> +{
> +	unsigned len = strlen(a) + 1 + strlen(sep);
> +	int olen = *dst ? strlen(*dst) : 0;
> +	int blen = bt ? json_len(bt) : 0;
> +
> +	*dst = realloc(*dst, len + olen + blen);
> +	if (!*dst)
> +		exit(ENOMEM);

Hmm.. wouldn't it be better to return an error code?  In general, the
perf tools try to terminate it gracefully in case of error.


> +	if (!olen)
> +		*(*dst) = 0;
> +	else
> +		strcat(*dst, sep);
> +	strcat(*dst, a);
> +	if (bt)
> +		strncat(*dst, map + bt->start, blen);
> +}
> +
> +static void fixname(char *s)
> +{
> +	for (; *s; s++) {
> +		*s = tolower(*s);
> +		/*
> +		 * Remove '.' for now, until the parser
> +		 * can deal with it.
> +		 */
> +		if (*s == '.')
> +			*s = '_';
> +	}
> +}
> +
> +static void fixdesc(char *s)
> +{
> +	char *e = s + strlen(s);
> +
> +	/* Remove trailing dots that look ugly in perf list */
> +	--e;
> +	while (e >= s && isspace(*e))
> +		--e;
> +	if (*e == '.')
> +		*e = 0;
> +}
> +
> +#define EXPECT(e, t, m) do { if (!(e)) {			\
> +	jsmntok_t *loc = (t);					\
> +	if (!(t)->start && (t) > tokens)			\
> +		loc = (t) - 1;					\
> +	fprintf(stderr, "%s:%d: " m ", got %s\n", fn,		\
> +		json_line(map, loc),				\
> +		json_name(t));					\
> +	goto out_free;						\
> +} } while (0)

It looks like the EXPECT macro can only be used in json_events()
function.  If so, please move it next to the function.

Also please use pr_err() to print an error message.

> +
> +static struct msrmap {
> +	const char *num;
> +	const char *pname;
> +} msrmap[] = {
> +	{ "0x3F6", "ldlat=" },
> +	{ "0x1A6", "offcore_rsp=" },
> +	{ "0x1A7", "offcore_rsp=" },
> +	{ NULL, NULL }
> +};
> +
> +static struct field {
> +	const char *field;
> +	const char *kernel;
> +} fields[] = {
> +	{ "EventCode",	"event=" },
> +	{ "UMask",	"umask=" },
> +	{ "CounterMask", "cmask=" },
> +	{ "Invert",	"inv=" },
> +	{ "AnyThread",	"any=" },
> +	{ "EdgeDetect",	"edge=" },
> +	{ "SampleAfterValue", "period=" },
> +	{ NULL, NULL }
> +};
> +
> +static void cut_comma(char *map, jsmntok_t *newval)
> +{
> +	int i;
> +
> +	/* Cut off everything after comma */
> +	for (i = newval->start; i < newval->end; i++) {
> +		if (map[i] == ',')
> +			newval->end = i;
> +	}
> +}
> +
> +static int match_field(char *map, jsmntok_t *field, int nz,
> +		       char **event, jsmntok_t *val)
> +{
> +	struct field *f;
> +	jsmntok_t newval = *val;
> +
> +	for (f = fields; f->field; f++)
> +		if (json_streq(map, field, f->field) && nz) {
> +			cut_comma(map, &newval);
> +			addfield(map, event, ",", f->kernel, &newval);
> +			return 1;
> +		}
> +	return 0;
> +}
> +
> +static struct msrmap *lookup_msr(char *map, jsmntok_t *val)
> +{
> +	jsmntok_t newval = *val;
> +	static bool warned;
> +	int i;
> +
> +	cut_comma(map, &newval);
> +	for (i = 0; msrmap[i].num; i++)
> +		if (json_streq(map, &newval, msrmap[i].num))
> +			return &msrmap[i];
> +	if (!warned) {
> +		warned = true;
> +		pr_err("Unknown MSR in event file %.*s\n",
> +			json_len(val), map + val->start);
> +	}
> +	return NULL;
> +}
> +
> +/* Call func with each event in the json file */
> +
> +int json_events(const char *fn,
> +	  int (*func)(void *data, char *name, char *event, char *desc),
> +	  void *data)
> +{
> +	int err = -EIO;
> +	size_t size;
> +	jsmntok_t *tokens, *tok;
> +	int i, j, len;
> +	char *map;
> +
> +	tokens = parse_json(fn, &map, &size, &len);
> +	if (!tokens)
> +		return -EIO;
> +	EXPECT(tokens->type == JSMN_ARRAY, tokens, "expected top level array");
> +	tok = tokens + 1;
> +	for (i = 0; i < tokens->size; i++) {
> +		char *event = NULL, *desc = NULL, *name = NULL;
> +		struct msrmap *msr = NULL;
> +		jsmntok_t *msrval = NULL;
> +		jsmntok_t *precise = NULL;
> +		jsmntok_t *obj = tok++;
> +
> +		EXPECT(obj->type == JSMN_OBJECT, obj, "expected object");
> +		for (j = 0; j < obj->size; j += 2) {
> +			jsmntok_t *field, *val;
> +			int nz;
> +
> +			field = tok + j;
> +			EXPECT(field->type == JSMN_STRING, tok + j,
> +			       "Expected field name");
> +			val = tok + j + 1;
> +			EXPECT(val->type == JSMN_STRING, tok + j + 1,
> +			       "Expected string value");
> +
> +			nz = !json_streq(map, val, "0");
> +			if (match_field(map, field, nz, &event, val)) {
> +				/* ok */
> +			} else if (json_streq(map, field, "EventName")) {
> +				addfield(map, &name, "", "", val);
> +			} else if (json_streq(map, field, "BriefDescription")) {
> +				addfield(map, &desc, "", "", val);
> +				fixdesc(desc);
> +			} else if (json_streq(map, field, "PEBS") && nz &&
> +				   !strstr(desc, "(Precise Event)")) {

I can't understand this.  Why did you check 'desc' to have such string?
It seems that it's only set after the loop.  And what if this PEBS line
comes first (having desc set NULL)?

And more generally, can we extend these if-else's to a generic loop to
check a table like in match_field() so that it can be added easily?


> +				precise = val;
> +			} else if (json_streq(map, field, "MSRIndex") && nz) {
> +				msr = lookup_msr(map, val);
> +			} else if (json_streq(map, field, "MSRValue")) {
> +				msrval = val;
> +			} else if (json_streq(map, field, "Errata") &&
> +				   !json_streq(map, val, "null")) {
> +				addfield(map, &desc, ". ",
> +					" Spec update: ", val);
> +			} else if (json_streq(map, field, "Data_LA") && nz) {
> +				addfield(map, &desc, ". ",
> +					" Supports address when precise",
> +					NULL);
> +			}
> +			/* ignore unknown fields */
> +		}
> +		if (precise) {
> +			if (json_streq(map, precise, "2"))
> +				addfield(map, &desc, " ", "(Must be precise)",
> +						NULL);
> +			else
> +				addfield(map, &desc, " ",
> +						"(Precise event)", NULL);
> +		}
> +		if (msr != NULL)
> +			addfield(map, &event, ",", msr->pname, msrval);
> +		fixname(name);
> +		err = func(data, name, event, desc);
> +		free(event);
> +		free(desc);
> +		free(name);
> +		if (err)
> +			break;
> +		tok += j;
> +	}
> +	EXPECT(tok - tokens == len, tok, "unexpected objects at end");
> +	err = 0;
> +out_free:
> +	free_json(map, size, tokens);
> +	return err;
> +}
> diff --git a/tools/perf/util/jevents.h b/tools/perf/util/jevents.h
> new file mode 100644
> index 0000000..4c2b879
> --- /dev/null
> +++ b/tools/perf/util/jevents.h
> @@ -0,0 +1,3 @@
> +int json_events(const char *fn,
> +		int (*func)(void *data, char *name, char *event, char *desc),
> +		void *data);
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index baec090..9f154af 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -9,6 +9,9 @@
>  #include "pmu.h"
>  #include "parse-events.h"
>  #include "cpumap.h"
> +#include "jevents.h"
> +
> +const char *json_file;
>  
>  #define UNIT_MAX_LEN	31 /* max length for event unit name */
>  
> @@ -273,6 +276,14 @@ static int pmu_aliases_parse(char *dir, struct list_head *head)
>  	return ret;
>  }
>  
> +static int add_alias(void *data, char *name, char *event, char *desc)
> +{
> +	struct list_head *head = (struct list_head *)data;
> +
> +	__perf_pmu__new_alias(head, name, NULL, desc, event);
> +	return 0;
> +}
> +
>  /*
>   * Reading the pmu event aliases definition, which should be located at:
>   * /sys/bus/event_source/devices/<dev>/events as sysfs group attributes.
> @@ -422,6 +433,9 @@ static struct perf_pmu *pmu_lookup(const char *name)
>  	if (pmu_aliases(name, &aliases))
>  		return NULL;
>  
> +	if (!strcmp(name, "cpu") && json_file)
> +		json_events(json_file, add_alias, &aliases);
> +
>  	if (pmu_type(name, &type))
>  		return NULL;
>  
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index c14a543..583d21e 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -46,4 +46,6 @@ void print_pmu_events(const char *event_glob, bool name_only);
>  bool pmu_have_event(const char *pname, const char *name);
>  
>  int perf_pmu__test(void);
> +
> +extern const char *json_file;
>  #endif /* __PMU_H */

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

* Re: [PATCH 5/9] perf, tools: Add perf download to download event files v2
  2014-05-12 22:51 ` [PATCH 5/9] perf, tools: Add perf download to download event files v2 Andi Kleen
@ 2014-05-14  5:17   ` Namhyung Kim
  2014-05-15 20:10     ` Andi Kleen
  0 siblings, 1 reply; 21+ messages in thread
From: Namhyung Kim @ 2014-05-14  5:17 UTC (permalink / raw)
  To: Andi Kleen; +Cc: acme, linux-kernel, peterz, eranian, jolsa, Andi Kleen

On Mon, 12 May 2014 15:51:10 -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> Add a downloader to automatically download the right
> files from a download site.
>
> This is implemented as a script calling wget, similar to
> perf archive. The perf driver automatically calls the right
> binary. The downloader is extensible, but currently only
> implements an Intel event download.  It would be straightforward
> to add other sites too for other vendors.
>
> The downloaded event files are put into ~/.cache/pmu-events, where the
> builtin event parser in util/* can find them automatically.
>
> Cc: Use ~/.cache

???


[SNIP]
> @@ -0,0 +1,52 @@
> +#!/bin/bash
> +# download event files for current cpu for perf
> +
> +WGETOPT=${WGETOPT:---no-verbose --timeout 5}

Could you please extend this script to support curl also?  It seems the
wget is not installed by default on my system.

> +
> +set -e
> +
> +if [ "$1" == "" ] ; then
> +	S=$(awk '
> +/^vendor/     		{ V=$3 }
> +/^model/ && $2 == ":" 	{ M=$3 }
> +/^cpu family/ 		{ F = $4 }

Inconsistency in using whitespace..

Thanks,
Namhyung


> +END	      { printf("%s-%s-%X", V, F, M) }' /proc/cpuinfo)
> +else
> +	S="$1"
> +fi
> +V=$(echo $S  | ( IFS=- read v f m ; echo $v) )
> +
> +CACHEDIR=${XDG_CACHE_HOME:-~/.cache}
> +[ ! -d $CACHEDIR/pmu-events ] && mkdir -p $CACHEDIR/pmu-events
> +cd $CACHEDIR/pmu-events
> +
> +case "$V" in
> +GenuineIntel)
> +	echo "Downloading models file"
> +	URLBASE=${URLBASE:-https://download.01.org/perfmon}
> +	MAPFILE=${MAPFILE:-mapfile.csv}
> +	echo "Downloading readme.txt"
> +	wget -N $WGETOPT $URLBASE/readme.txt
> +	;;
> +
> +# Add more CPU vendors here
> +
> +*)
> +	echo "Unsupported CPU vendor $V"
> +	exit 1
> +	;;
> +esac
> +
> +wget -N $WGETOPT $URLBASE/$MAPFILE
> +
> +echo "Downloading events file"
> +awk -v urlbase=$URLBASE -v cpu="$S" -F, \
> +	'$1 == cpu && $4 == "core" { print urlbase $3; exit 0 }' \
> +	$MAPFILE > url$$
> +if [ -s url$$ ] ; then
> +	wget $WGETOPT -q -i url$$ -O $S-core.json
> +else
> +	echo "CPU $S not found"
> +fi
> +rm -f url$$
> +

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

* Re: [PATCH 8/9] perf, tools, test: Add test case for alias and JSON parsing
  2014-05-12 22:51 ` [PATCH 8/9] perf, tools, test: Add test case for alias and JSON parsing Andi Kleen
@ 2014-05-14  5:27   ` Namhyung Kim
  0 siblings, 0 replies; 21+ messages in thread
From: Namhyung Kim @ 2014-05-14  5:27 UTC (permalink / raw)
  To: Andi Kleen; +Cc: acme, linux-kernel, peterz, eranian, jolsa, Andi Kleen

On Mon, 12 May 2014 15:51:13 -0700, Andi Kleen wrote:

[SNIP]
> +int test__aliases(void)

Just a nitpick.  Please use a more descriptive name for the testcase
like test__event_aliases().

> +{
> +	int err;
> +
> +	/* Download JSON files */
> +	/* XXX assumes perf is installed */
> +	/* For now user must manually download */
> +	if (0 && system("perf download > /dev/null") < 0) {
> +		/* Don't error out for this for now */
> +		fprintf(stderr, "perf download failed\n");
> +	}
> +
> +	evlist = perf_evlist__new();
> +	if (evlist == NULL)
> +		return -ENOMEM;
> +
> +	err = pmu_iterate_events(test_event);
> +	fprintf(stderr, " Parsed %d events :", num_events);
> +	if (failed > 0)
> +		fprintf(stderr, " %d events failed", num_events);

s/num_events/failed/ ?

And this kind of detailed information is usually printed when -v option
is given via pr_debug().

Thanks,
Namhyung


> +	perf_evlist__delete(evlist);
> +	return err;
> +}

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

* Re: [PATCH 9/9] perf, tools, record: Always allow to overide default period v2
  2014-05-12 22:51 ` [PATCH 9/9] perf, tools, record: Always allow to overide default period v2 Andi Kleen
@ 2014-05-14  5:50   ` Namhyung Kim
  2014-05-15 20:51     ` Andi Kleen
  0 siblings, 1 reply; 21+ messages in thread
From: Namhyung Kim @ 2014-05-14  5:50 UTC (permalink / raw)
  To: Andi Kleen
  Cc: acme, linux-kernel, peterz, eranian, jolsa, Andi Kleen, fweisbec

On Mon, 12 May 2014 15:51:14 -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> Fix the logic to allow overriding event default periods with -c or -F
> on the command line.  I'm not sure I understand this if() fully, but
> this change makes all cases I tested work (tracepoint with default, default,
> ,-c, -F)
>
> This fixed specifying -c / -F with json event list events,
> which have a default period. It should do the same
> for trace point events.

Hmm.. doesn't that require both -c and -F are given at the same time?

Anyway, there was a similar discussion, but I suggested an other way.
Please see the line below.

https://lkml.org/lkml/2014/2/4/807

Thanks,
Namhyung

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

* Re: perf: Add support for full Intel event lists v3
  2014-05-12 22:51 perf: Add support for full Intel event lists v3 Andi Kleen
                   ` (8 preceding siblings ...)
  2014-05-12 22:51 ` [PATCH 9/9] perf, tools, record: Always allow to overide default period v2 Andi Kleen
@ 2014-05-15 16:09 ` Hagen Paul Pfeifer
  2014-05-15 19:12   ` Andi Kleen
  9 siblings, 1 reply; 21+ messages in thread
From: Hagen Paul Pfeifer @ 2014-05-15 16:09 UTC (permalink / raw)
  To: Andi Kleen; +Cc: acme, linux-kernel, peterz, eranian, namhyung, jolsa

On 13 May 2014 00:51, Andi Kleen <andi@firstfloor.org> wrote:

> I implemented an automatic downloader to get the event file for the
> current CPU.  The events are stored in ~/.events.
> Then perf adds a parser that converts the JSON format into perf event
> aliases, which then can be used directly as any other perf event.

Should the name of the file not more specific? I mean .events can mean
everything? Every program may have a demand for this generic name. Why
not ~/.perf-events? Even more standard [1]:
$XDG_CACHE_HOME/perf/events (i.e. ~/.cache/perf/events)

[1] http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html

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

* Re: perf: Add support for full Intel event lists v3
  2014-05-15 16:09 ` perf: Add support for full Intel event lists v3 Hagen Paul Pfeifer
@ 2014-05-15 19:12   ` Andi Kleen
  0 siblings, 0 replies; 21+ messages in thread
From: Andi Kleen @ 2014-05-15 19:12 UTC (permalink / raw)
  To: Hagen Paul Pfeifer
  Cc: Andi Kleen, acme, linux-kernel, peterz, eranian, namhyung, jolsa

On Thu, May 15, 2014 at 06:09:20PM +0200, Hagen Paul Pfeifer wrote:
> On 13 May 2014 00:51, Andi Kleen <andi@firstfloor.org> wrote:
> 
> > I implemented an automatic downloader to get the event file for the
> > current CPU.  The events are stored in ~/.events.
> > Then perf adds a parser that converts the JSON format into perf event
> > aliases, which then can be used directly as any other perf event.
> 
> Should the name of the file not more specific? I mean .events can mean
> everything? Every program may have a demand for this generic name. Why
> not ~/.perf-events? Even more standard [1]:
> $XDG_CACHE_HOME/perf/events (i.e. ~/.cache/perf/events)

It's already pmu-events, but I forgot to update the description.

-Andi

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

* Re: [PATCH 5/9] perf, tools: Add perf download to download event files v2
  2014-05-14  5:17   ` Namhyung Kim
@ 2014-05-15 20:10     ` Andi Kleen
  0 siblings, 0 replies; 21+ messages in thread
From: Andi Kleen @ 2014-05-15 20:10 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Andi Kleen, acme, linux-kernel, peterz, eranian, jolsa, Andi Kleen

> Could you please extend this script to support curl also?  It seems the
> wget is not installed by default on my system.

I looked at it, but it's a major pain because curl is totally
incompatible. I don't think it's a major burden to install wget?

> > +set -e
> > +
> > +if [ "$1" == "" ] ; then
> > +	S=$(awk '
> > +/^vendor/     		{ V=$3 }
> > +/^model/ && $2 == ":" 	{ M=$3 }
> > +/^cpu family/ 		{ F = $4 }
> 
> Inconsistency in using whitespace..

awk scripts always start at column 0

-Andi

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

* Re: [PATCH 3/9] perf, tools: Add support for reading JSON event files
  2014-05-14  5:10   ` Namhyung Kim
@ 2014-05-15 20:18     ` Andi Kleen
  0 siblings, 0 replies; 21+ messages in thread
From: Andi Kleen @ 2014-05-15 20:18 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Andi Kleen, acme, linux-kernel, peterz, eranian, jolsa, Andi Kleen

> And more generally, can we extend these if-else's to a generic loop to
> check a table like in match_field() so that it can be added easily?

The simple cases are all already handled in a table above.

The if() only contains cases that need special code. I don't think
callbacks would be better here.

-Andi

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

* Re: [PATCH 9/9] perf, tools, record: Always allow to overide default period v2
  2014-05-14  5:50   ` Namhyung Kim
@ 2014-05-15 20:51     ` Andi Kleen
  0 siblings, 0 replies; 21+ messages in thread
From: Andi Kleen @ 2014-05-15 20:51 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Andi Kleen, acme, linux-kernel, peterz, eranian, jolsa,
	Andi Kleen, fweisbec

On Wed, May 14, 2014 at 02:50:47PM +0900, Namhyung Kim wrote:
> On Mon, 12 May 2014 15:51:14 -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> >
> > Fix the logic to allow overriding event default periods with -c or -F
> > on the command line.  I'm not sure I understand this if() fully, but
> > this change makes all cases I tested work (tracepoint with default, default,
> > ,-c, -F)
> >
> > This fixed specifying -c / -F with json event list events,
> > which have a default period. It should do the same
> > for trace point events.
> 
> Hmm.. doesn't that require both -c and -F are given at the same time?

I don't think so.

> 
> Anyway, there was a similar discussion, but I suggested an other way.
> Please see the line below.

I had tried a couple of combinations earlier, this was the one that
worked in all cases.

-Andi

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

end of thread, other threads:[~2014-05-15 20:51 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-12 22:51 perf: Add support for full Intel event lists v3 Andi Kleen
2014-05-12 22:51 ` [PATCH 1/9] perf, tools: Add jsmn `jasmine' JSON parser Andi Kleen
2014-05-14  3:25   ` Namhyung Kim
2014-05-14  5:00     ` Andi Kleen
2014-05-12 22:51 ` [PATCH 2/9] perf, tools: Add support for text descriptions of events and alias add Andi Kleen
2014-05-12 22:51 ` [PATCH 3/9] perf, tools: Add support for reading JSON event files Andi Kleen
2014-05-14  5:10   ` Namhyung Kim
2014-05-15 20:18     ` Andi Kleen
2014-05-12 22:51 ` [PATCH 4/9] perf, tools: Automatically look for event file name for cpu v2 Andi Kleen
2014-05-12 22:51 ` [PATCH 5/9] perf, tools: Add perf download to download event files v2 Andi Kleen
2014-05-14  5:17   ` Namhyung Kim
2014-05-15 20:10     ` Andi Kleen
2014-05-12 22:51 ` [PATCH 6/9] perf, tools: Allow events with dot Andi Kleen
2014-05-12 22:51 ` [PATCH 7/9] perf, tools: Query terminal width and use in perf list Andi Kleen
2014-05-12 22:51 ` [PATCH 8/9] perf, tools, test: Add test case for alias and JSON parsing Andi Kleen
2014-05-14  5:27   ` Namhyung Kim
2014-05-12 22:51 ` [PATCH 9/9] perf, tools, record: Always allow to overide default period v2 Andi Kleen
2014-05-14  5:50   ` Namhyung Kim
2014-05-15 20:51     ` Andi Kleen
2014-05-15 16:09 ` perf: Add support for full Intel event lists v3 Hagen Paul Pfeifer
2014-05-15 19:12   ` Andi Kleen

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