All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline
@ 2015-10-17 10:48 Wang Nan
  2015-10-17 10:48 ` [RFC PATCH 1/7] perf tools: Add API to config maps in bpf object Wang Nan
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Wang Nan @ 2015-10-17 10:48 UTC (permalink / raw)
  To: acme, ast, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu, Wang Nan

This patch set is based on commit d2f820885d8e ("perf tools: Pass
available CPU number to clang compiler") in my git tree [1]. It replaces
the old four patches in the git tree with improved solution.

In these 7 patches:

 1. perf is able to put values into map:
  # perf record -e mybpf.c/maps.values.value=1234/ ...

 2. perf is able to control different slots in a map separately:
  # perf record -e mybpf.c/maps.values.value[1,4-6]=1234,maps.values.value[0,2-3]=5678/ ...

 3. The second syntax can be applied to perf event also:
  # perf record -v -a -e evt=cycles -e mybpf.c/maps.pmu_map.event[0]=evt/ ...

 4. Compatible with the old syntax:
  # perf record -v -a -e evt=cycles -e mybpf.c/maps.pmu_map.event=evt/ ...

[1] git://git.kernel.org/pub/scm/linux/kernel/git/pi3orama/linux.git perf/ebpf

He Kuang (1):
  perf record: Apply config to BPF objects before recording

Wang Nan (6):
  perf tools: Add API to config maps in bpf object
  perf tools: Add API to apply config to BPF map
  perf tools: Enable BPF object configure syntax
  perf tools: Support setting different slots in a BPF map separately
  perf tools: Enable indics setting syntax for BPF maps
  perf tools: Enable passing event to BPF object

 tools/perf/builtin-record.c    |  11 +
 tools/perf/util/bpf-loader.c   | 515 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/bpf-loader.h   |  42 ++++
 tools/perf/util/parse-events.c |  60 ++++-
 tools/perf/util/parse-events.h |   5 +-
 tools/perf/util/parse-events.l |  11 +
 tools/perf/util/parse-events.y | 113 ++++++++-
 7 files changed, 748 insertions(+), 9 deletions(-)

-- 
1.8.3.4


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

* [RFC PATCH 1/7] perf tools: Add API to config maps in bpf object
  2015-10-17 10:48 [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Wang Nan
@ 2015-10-17 10:48 ` Wang Nan
  2015-11-20  8:13   ` Wangnan (F)
  2015-11-23 11:20   ` Wangnan (F)
  2015-10-17 10:48 ` [RFC PATCH 2/7] perf tools: Add API to apply config to BPF map Wang Nan
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 18+ messages in thread
From: Wang Nan @ 2015-10-17 10:48 UTC (permalink / raw)
  To: acme, ast, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu, Wang Nan, Arnaldo Carvalho de Melo

bpf__config_obj() is introduced as a core API to config BPF object
after loading. One configuration option of maps is introduced. After
this patch BPF object can accept configuration like:

 maps.my_map.value=1234

This patch is more complex than the work it really does because the
consideration of extension. In designing of BPF map configuration,
following things should be considered:

 1. Array indics selection: perf should allow user setting different
    value to different slots in an array, with syntax like:
    maps.my_map.value[0,3-6]=1234;

 2. Type of value: integer is not the only valid value type. Perf
    event can also be put into a map after commit 35578d7984003097af2b1e3
    (bpf: Implement function bpf_perf_event_read() that get the selected
    hardware PMU conuter);

 3. For hash table, it is possible to use string or other as key;

 4. It is possible that map configuration is unable to be setup
    during parsing. Perf event is an example.

Therefore, this patch does tie following thing for extension:

 1. Instead of update map element during parsing, this patch stores
    map config options in 'struct bpf_map_priv'. Following patches
    would apply those configs at proper time;

 2. Make 'struct bpf_map_priv' extensible so following patches can
    add new key and value operations;

 3. Use bpf_config_map_funcs array to support more maps configuration.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David Ahern <dsahern@gmail.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kaixu Xia <xiakaixu@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/n/ebpf-36xcrahy9n0ayc05mu7aajpk@git.kernel.org
---
 tools/perf/util/bpf-loader.c | 180 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/bpf-loader.h |  27 +++++++
 2 files changed, 207 insertions(+)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 73ff9a9..a5a1c36 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -10,11 +10,13 @@
 #include <linux/err.h>
 #include "perf.h"
 #include "debug.h"
+#include "util.h"
 #include "bpf-loader.h"
 #include "bpf-prologue.h"
 #include "llvm-utils.h"
 #include "probe-event.h"
 #include "probe-finder.h" // for MAX_PROBES
+#include "parse-events.h"
 #include "llvm-utils.h"
 
 #define DEFINE_PRINT_FN(name, level) \
@@ -633,6 +635,170 @@ int bpf__foreach_tev(struct bpf_object *obj,
 	return 0;
 }
 
+enum bpf_map_priv_key_type {
+	BPF_MAP_PRIV_KEY_ALL,
+};
+
+enum bpf_map_priv_value_type {
+	BPF_MAP_PRIV_VAL_VALUE,
+};
+
+struct bpf_map_priv {
+	struct {
+		enum bpf_map_priv_key_type type;
+	} key;
+
+	struct {
+		enum bpf_map_priv_value_type type;
+		union {
+			u64 val;
+		};
+	} value;
+};
+
+static void
+bpf_map_priv__clear(struct bpf_map *map __maybe_unused,
+		    void *_priv)
+{
+	struct bpf_map_priv *priv = _priv;
+
+	free(priv);
+}
+
+static int
+bpf__config_obj_map_array_value(struct bpf_map *map,
+				struct parse_events_term *term)
+{
+	struct bpf_map_priv *priv;
+	struct bpf_map_def def;
+	const char *map_name;
+	int err;
+
+	map_name = bpf_map__get_name(map);
+
+	err = bpf_map__get_def(map, &def);
+	if (err) {
+		pr_debug("Unable to get map definition from '%s'\n",
+			 map_name);
+		return -EINVAL;
+	}
+
+	if (def.type != BPF_MAP_TYPE_ARRAY) {
+		pr_debug("Map %s type is not BPF_MAP_TYPE_ARRAY\n",
+			 map_name);
+		return -EBADF;
+	}
+	if (def.key_size < sizeof(unsigned int)) {
+		pr_debug("Map %s has incorrect key size\n", map_name);
+		return -EINVAL;
+	}
+	switch (def.value_size) {
+	case 1:
+	case 2:
+	case 4:
+	case 8:
+		break;
+	default:
+		pr_debug("Map %s has incorrect value size\n", map_name);
+		return -EINVAL;
+	}
+
+	priv = zalloc(sizeof(*priv));
+	if (!priv) {
+		pr_debug("No enough memory to alloc map private\n");
+		return -ENOMEM;
+	}
+
+	priv->key.type = BPF_MAP_PRIV_KEY_ALL;
+	priv->value.type = BPF_MAP_PRIV_VAL_VALUE;
+	priv->value.val = term->val.num;
+	return bpf_map__set_private(map, priv, bpf_map_priv__clear);
+}
+
+static int
+bpf__config_obj_map_value(struct bpf_map *map,
+			  struct parse_events_term *term,
+			  struct perf_evlist *evlist __maybe_unused)
+{
+	if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
+		return bpf__config_obj_map_array_value(map, term);
+
+	pr_debug("ERROR: wrong value type\n");
+	return -EINVAL;
+}
+
+struct bpf_config_map_func {
+	const char *config_opt;
+	int (*config_func)(struct bpf_map *, struct parse_events_term *,
+			   struct perf_evlist *);
+};
+
+struct bpf_config_map_func bpf_config_map_funcs[] = {
+	{"value", bpf__config_obj_map_value},
+};
+
+static int
+bpf__config_obj_map(struct bpf_object *obj,
+		    struct parse_events_term *term,
+		    struct perf_evlist *evlist)
+{
+	/* key is "maps.<mapname>.<config opt>" */
+	char *map_name = strdup(term->config + sizeof("maps.") - 1);
+	struct bpf_map *map;
+	int err = -ENOENT;
+	char *map_opt;
+	size_t i;
+
+	if (!map_name)
+		return -ENOMEM;
+
+	map_opt = strchr(map_name, '.');
+	if (!map_opt) {
+		pr_debug("ERROR: Invalid map config: %s\n", map_name);
+		goto out;
+	}
+
+	*map_opt++ = '\0';
+	if (*map_opt == '\0') {
+		pr_debug("ERROR: Invalid map option: %s\n", term->config);
+		goto out;
+	}
+
+	map = bpf_object__get_map_by_name(obj, map_name);
+	if (!map) {
+		pr_debug("ERROR: Map %s doesn't exist\n", map_name);
+		goto out;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(bpf_config_map_funcs); i++) {
+		struct bpf_config_map_func *func = &bpf_config_map_funcs[i];
+
+		if (strcmp(map_opt, func->config_opt) == 0) {
+			err = func->config_func(map, term, evlist);
+			goto out;
+		}
+	}
+
+	pr_debug("ERROR: invalid config option '%s' for maps\n",
+		 map_opt);
+	err = -ENOENT;
+out:
+	free(map_name);
+	return err;
+}
+
+int bpf__config_obj(struct bpf_object *obj,
+		    struct parse_events_term *term,
+		    struct perf_evlist *evlist)
+{
+	if (!obj || !term || !term->config)
+		return -ENODEV;
+
+	if (!prefixcmp(term->config, "maps."))
+		return bpf__config_obj_map(obj, term, evlist);
+	return -ENODEV;
+}
+
 #define bpf__strerror_head(err, buf, size) \
 	char sbuf[STRERR_BUFSIZE], *emsg;\
 	if (!size)\
@@ -675,3 +841,17 @@ int bpf__strerror_load(struct bpf_object *obj __maybe_unused,
 	bpf__strerror_end(buf, size);
 	return 0;
 }
+
+int bpf__strerror_config_obj(struct bpf_object *obj __maybe_unused,
+			     struct parse_events_term *term,
+			     struct perf_evlist *evlist __maybe_unused,
+			     int err, char *buf, size_t size)
+{
+	bpf__strerror_head(err, buf, size);
+	bpf__strerror_entry(ENODEV, "Invalid config option: '%s'", term->config)
+	bpf__strerror_entry(ENOENT, "Config target in '%s' is invalid", term->config)
+	bpf__strerror_entry(EBADF,  "Map type mismatch in '%s'", term->config)
+	bpf__strerror_entry(EINVAL, "Invalid config value")
+	bpf__strerror_end(buf, size);
+	return 0;
+}
diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
index d8f1945..dfec9b8 100644
--- a/tools/perf/util/bpf-loader.h
+++ b/tools/perf/util/bpf-loader.h
@@ -9,9 +9,11 @@
 #include <linux/err.h>
 #include <string.h>
 #include "probe-event.h"
+#include "evlist.h"
 #include "debug.h"
 
 struct bpf_object;
+struct parse_events_term;
 #define PERF_BPF_PROBE_GROUP "perf_bpf_probe"
 
 typedef int (*bpf_prog_iter_callback_t)(struct probe_trace_event *tev,
@@ -34,6 +36,13 @@ int bpf__strerror_load(struct bpf_object *obj, int err,
 		       char *buf, size_t size);
 int bpf__foreach_tev(struct bpf_object *obj,
 		     bpf_prog_iter_callback_t func, void *arg);
+
+int bpf__config_obj(struct bpf_object *obj, struct parse_events_term *term,
+		    struct perf_evlist *evlist);
+int bpf__strerror_config_obj(struct bpf_object *obj,
+			     struct parse_events_term *term,
+			     struct perf_evlist *evlist,
+			     int err, char *buf, size_t size);
 #else
 static inline struct bpf_object *
 bpf__prepare_load(const char *filename __maybe_unused,
@@ -65,6 +74,14 @@ bpf__foreach_tev(struct bpf_object *obj __maybe_unused,
 }
 
 static inline int
+bpf__config_obj(struct bpf_object *obj __maybe_unused,
+		struct parse_events_term *term __maybe_unused,
+		struct perf_evlist *evlist __maybe_unused)
+{
+	return 0;
+}
+
+static inline int
 __bpf_strerror(char *buf, size_t size)
 {
 	if (!size)
@@ -90,5 +107,15 @@ static inline int bpf__strerror_load(struct bpf_object *obj __maybe_unused,
 {
 	return __bpf_strerror(buf, size);
 }
+
+static inline int
+bpf__strerror_config_obj(struct bpf_object *obj __maybe_unused,
+			 struct parse_events_term *term __maybe_unused,
+			 struct perf_evlist *evlist __maybe_unused,
+			 int err __maybe_unused,
+			 char *buf, size_t size)
+{
+	return __bpf_strerror(buf, size);
+}
 #endif
 #endif
-- 
1.8.3.4


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

* [RFC PATCH 2/7] perf tools: Add API to apply config to BPF map
  2015-10-17 10:48 [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Wang Nan
  2015-10-17 10:48 ` [RFC PATCH 1/7] perf tools: Add API to config maps in bpf object Wang Nan
@ 2015-10-17 10:48 ` Wang Nan
  2015-10-17 10:48 ` [RFC PATCH 3/7] perf record: Apply config to BPF objects before recording Wang Nan
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Wang Nan @ 2015-10-17 10:48 UTC (permalink / raw)
  To: acme, ast, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu, Wang Nan, Arnaldo Carvalho de Melo

bpf__apply_config() is introduced as the core API to apply config
options to all BPF objects. This patch also does the real work for
setting values for BPF_MAP_TYPE_PERF_ARRAY maps by inserting value
stored in map's private field into the BPF map.

This patch is required because we are not always able to set all
BPF config during parsing. Further patch will set events created
by perf to BPF_MAP_TYPE_PERF_EVENT_ARRAY maps, which is not exist
until perf_evsel__open().

bpf_map_foreach_key() is introduced to iterate over each key
needs to be configured. This function would be extended to support
more map types and different key settings.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David Ahern <dsahern@gmail.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kaixu Xia <xiakaixu@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/n/ebpf-tmg65cm1zaf1zxs7zmvxmxp4@git.kernel.org
---
 tools/perf/util/bpf-loader.c | 163 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/bpf-loader.h |  15 ++++
 2 files changed, 178 insertions(+)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index a5a1c36..15cf27a 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -7,6 +7,7 @@
 
 #include <linux/bpf.h>
 #include <bpf/libbpf.h>
+#include <bpf/bpf.h>
 #include <linux/err.h>
 #include "perf.h"
 #include "debug.h"
@@ -666,6 +667,69 @@ bpf_map_priv__clear(struct bpf_map *map __maybe_unused,
 }
 
 static int
+bpf_map_config_foreach_key(struct bpf_map *map,
+			   int (*func)(const char *name,
+				       int map_fd,
+				       struct bpf_map_def *pdef,
+				       struct bpf_map_priv *priv,
+				       void *pkey, void *arg),
+			   void *arg)
+{
+	unsigned int i;
+	int err, map_fd;
+	const char *name;
+	struct bpf_map_def def;
+	struct bpf_map_priv *priv;
+
+	name = bpf_map__get_name(map);
+
+	err = bpf_map__get_private(map, (void **)&priv);
+	if (err) {
+		pr_debug("ERROR: failed to get private from map %s\n", name);
+		return -EINVAL;
+	}
+	if (!priv) {
+		pr_debug("INFO: nothing to config for map %s\n", name);
+		return 0;
+	}
+
+	err = bpf_map__get_def(map, &def);
+	if (err) {
+		pr_debug("ERROR: failed to get definition from map %s\n", name);
+		return -EINVAL;
+	}
+	map_fd = bpf_map__get_fd(map);
+	if (map_fd < 0) {
+		pr_debug("ERROR: failed to get fd from map %s\n", name);
+		return map_fd;
+	}
+
+	switch (def.type) {
+	case BPF_MAP_TYPE_ARRAY:
+		switch (priv->key.type) {
+		case BPF_MAP_PRIV_KEY_ALL:
+			for (i = 0; i < def.max_entries; i++) {
+				err = (*func)(name, map_fd, &def,
+					      priv, &i, arg);
+				if (err) {
+					pr_debug("ERROR: failed to insert value to %s[%u]\n",
+						 name, i);
+					return err;
+				}
+			}
+			return 0;
+		default:
+			pr_debug("ERROR: keytype for map '%s' invalid\n", name);
+			return -EINVAL;
+		}
+	default:
+		pr_debug("ERROR: type of '%s' incorrect\n", name);
+		return -EINVAL;
+	}
+}
+
+
+static int
 bpf__config_obj_map_array_value(struct bpf_map *map,
 				struct parse_events_term *term)
 {
@@ -799,6 +863,97 @@ int bpf__config_obj(struct bpf_object *obj,
 	return -ENODEV;
 }
 
+static int
+bpf__apply_config_value_for_key(int map_fd, void *pkey,
+				size_t val_size, u64 val)
+{
+	int err = 0;
+
+	switch (val_size) {
+	case 1: {
+		u8 _val = (u8)(val);
+		err = bpf_map_update_elem(map_fd, pkey, &_val, BPF_ANY);
+		break;
+	}
+	case 2: {
+		u16 _val = (u16)(val);
+		err = bpf_map_update_elem(map_fd, pkey, &_val, BPF_ANY);
+		break;
+	}
+	case 4: {
+		u32 _val = (u32)(val);
+		err = bpf_map_update_elem(map_fd, pkey, &_val, BPF_ANY);
+		break;
+	}
+	case 8: {
+		err = bpf_map_update_elem(map_fd, pkey, &val, BPF_ANY);
+		break;
+	}
+	default:
+		pr_debug("ERROR: internal error: invalid value size\n");
+		return -EINVAL;
+	}
+	if (err && errno)
+		err = -errno;
+	return err;
+}
+
+static int
+bpf__apply_config_map_for_key(const char *name, int map_fd,
+			      struct bpf_map_def *pdef __maybe_unused,
+			      struct bpf_map_priv *priv,
+			      void *pkey, void *arg __maybe_unused)
+{
+	int err;
+
+	switch (priv->value.type) {
+	case BPF_MAP_PRIV_VAL_VALUE:
+		err = bpf__apply_config_value_for_key(map_fd, pkey,
+						      pdef->value_size,
+						      priv->value.val);
+		break;
+	default:
+		pr_debug("ERROR: unknown value type for '%s'\n", name);
+		err = -EINVAL;
+	}
+	return err;
+}
+
+static int
+bpf__apply_config_map(struct bpf_map *map)
+{
+	return bpf_map_config_foreach_key(map, bpf__apply_config_map_for_key,
+					  NULL);
+}
+
+static int
+bpf__apply_config_object(struct bpf_object *obj)
+{
+	struct bpf_map *map;
+	int err;
+
+	bpf_map__for_each(map, obj) {
+		err = bpf__apply_config_map(map);
+		if (err)
+			return err;
+	}
+	return 0;
+}
+
+int bpf__apply_config(void)
+{
+	struct bpf_object *obj, *tmp;
+	int err;
+
+	bpf_object__for_each_safe(obj, tmp) {
+		err = bpf__apply_config_object(obj);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 #define bpf__strerror_head(err, buf, size) \
 	char sbuf[STRERR_BUFSIZE], *emsg;\
 	if (!size)\
@@ -855,3 +1010,11 @@ int bpf__strerror_config_obj(struct bpf_object *obj __maybe_unused,
 	bpf__strerror_end(buf, size);
 	return 0;
 }
+
+int bpf__strerror_apply_config(int err, char *buf, size_t size)
+{
+	bpf__strerror_head(err, buf, size);
+	bpf__strerror_entry(EINVAL, "Invalid option for map, add -v to see detail");
+	bpf__strerror_end(buf, size);
+	return 0;
+}
diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
index dfec9b8..c9c515e 100644
--- a/tools/perf/util/bpf-loader.h
+++ b/tools/perf/util/bpf-loader.h
@@ -43,6 +43,8 @@ int bpf__strerror_config_obj(struct bpf_object *obj,
 			     struct parse_events_term *term,
 			     struct perf_evlist *evlist,
 			     int err, char *buf, size_t size);
+int bpf__apply_config(void);
+int bpf__strerror_apply_config(int err, char *buf, size_t size);
 #else
 static inline struct bpf_object *
 bpf__prepare_load(const char *filename __maybe_unused,
@@ -82,6 +84,12 @@ bpf__config_obj(struct bpf_object *obj __maybe_unused,
 }
 
 static inline int
+bpf__apply_config(void)
+{
+	return 0;
+}
+
+static inline int
 __bpf_strerror(char *buf, size_t size)
 {
 	if (!size)
@@ -117,5 +125,12 @@ bpf__strerror_config_obj(struct bpf_object *obj __maybe_unused,
 {
 	return __bpf_strerror(buf, size);
 }
+
+static inline int
+bpf__strerror_apply_config(int err __maybe_unused,
+			   char *buf, size_t size)
+{
+	return __bpf_strerror(buf, size);
+}
 #endif
 #endif
-- 
1.8.3.4


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

* [RFC PATCH 3/7] perf record: Apply config to BPF objects before recording
  2015-10-17 10:48 [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Wang Nan
  2015-10-17 10:48 ` [RFC PATCH 1/7] perf tools: Add API to config maps in bpf object Wang Nan
  2015-10-17 10:48 ` [RFC PATCH 2/7] perf tools: Add API to apply config to BPF map Wang Nan
@ 2015-10-17 10:48 ` Wang Nan
  2015-10-17 10:48 ` [RFC PATCH 4/7] perf tools: Enable BPF object configure syntax Wang Nan
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Wang Nan @ 2015-10-17 10:48 UTC (permalink / raw)
  To: acme, ast, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu, Wang Nan, Arnaldo Carvalho de Melo

From: He Kuang <hekuang@huawei.com>

In perf record, before start recording, call bpf__apply_config() to
turn on all BPF config options.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David Ahern <dsahern@gmail.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kaixu Xia <xiakaixu@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/n/ebpf-ziazd5s4t9j96d01t5bdbtat@git.kernel.org
---
 tools/perf/builtin-record.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 200f221..b1c1324 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -32,6 +32,7 @@
 #include "util/parse-branch-options.h"
 #include "util/parse-regs-options.h"
 #include "util/llvm-utils.h"
+#include "util/bpf-loader.h"
 
 #include <unistd.h>
 #include <sched.h>
@@ -524,6 +525,16 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		goto out_child;
 	}
 
+	err = bpf__apply_config();
+	if (err) {
+		char errbuf[BUFSIZ];
+
+		bpf__strerror_apply_config(err, errbuf, sizeof(errbuf));
+		pr_err("ERROR: Apply config to BPF failed: %s\n",
+			 errbuf);
+		goto out_child;
+	}
+
 	/*
 	 * Normally perf_session__new would do this, but it doesn't have the
 	 * evlist.
-- 
1.8.3.4


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

* [RFC PATCH 4/7] perf tools: Enable BPF object configure syntax
  2015-10-17 10:48 [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Wang Nan
                   ` (2 preceding siblings ...)
  2015-10-17 10:48 ` [RFC PATCH 3/7] perf record: Apply config to BPF objects before recording Wang Nan
@ 2015-10-17 10:48 ` Wang Nan
  2015-10-23  4:48   ` Wangnan (F)
  2015-10-17 10:48 ` [RFC PATCH 5/7] perf tools: Support setting different slots in a BPF map separately Wang Nan
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Wang Nan @ 2015-10-17 10:48 UTC (permalink / raw)
  To: acme, ast, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu, Wang Nan, Arnaldo Carvalho de Melo

This patch adds the final step for BPF map configuration. A new syntax
is appended into parser so user can config BPF objects through '/' '/'
enclosed config terms.

After this patch, BPF programs for perf are able to get value through
perf cmdline like:

 # perf record -e bpf_file.c/maps.mymap.value=123/ ...

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David Ahern <dsahern@gmail.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kaixu Xia <xiakaixu@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/n/ebpf-2mjd96mowgzslkj8jrwbnwg7@git.kernel.org
---
 tools/perf/util/parse-events.c | 54 +++++++++++++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.h |  3 ++-
 tools/perf/util/parse-events.y | 21 ++++++++++++----
 3 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 06ba5a6..9f081a1 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -624,17 +624,62 @@ errout:
 	return err;
 }
 
+static int
+parse_events_config_bpf(struct parse_events_evlist *data,
+		       struct bpf_object *obj,
+		       struct list_head *head_config)
+{
+	struct parse_events_term *term;
+
+	if (!head_config || list_empty(head_config))
+		return 0;
+
+	list_for_each_entry(term, head_config, list) {
+		char errbuf[BUFSIZ];
+		int err;
+
+		if (term->type_term != PARSE_EVENTS__TERM_TYPE_USER) {
+			snprintf(errbuf, sizeof(errbuf),
+				 "Invalid config term for BPF object");
+			errbuf[BUFSIZ - 1] = '\0';
+
+			data->error->idx = term->err_term;
+			data->error->str = strdup(errbuf);
+			return -EINVAL;
+		}
+
+		err = bpf__config_obj(obj, term, data->evlist);
+		if (err) {
+			bpf__strerror_config_obj(obj, term, data->evlist,
+						 err, errbuf, sizeof(errbuf));
+			data->error->help = strdup(
+"Hint:\tValid config term:\n"
+"     \tmaps.<mapname>.value\n"
+"     \t(add -v to see detail)");
+			data->error->str = strdup(errbuf);
+			if (err == -EINVAL)
+				data->error->idx = term->err_val;
+			else
+				data->error->idx = term->err_term;
+			return err;
+		}
+	}
+	return 0;
+
+}
+
 int parse_events_load_bpf(struct parse_events_evlist *data,
 			  struct list_head *list,
 			  char *bpf_file_name,
-			  bool source)
+			  bool source,
+			  struct list_head *head_config)
 {
 	struct bpf_object *obj;
+	int err;
 
 	obj = bpf__prepare_load(bpf_file_name, source);
 	if (IS_ERR(obj) || !obj) {
 		char errbuf[BUFSIZ];
-		int err;
 
 		err = obj ? PTR_ERR(obj) : -EINVAL;
 
@@ -651,7 +696,10 @@ int parse_events_load_bpf(struct parse_events_evlist *data,
 		return err;
 	}
 
-	return parse_events_load_bpf_obj(data, list, obj);
+	err = parse_events_load_bpf_obj(data, list, obj);
+	if (err)
+		return err;
+	return parse_events_config_bpf(data, obj, head_config);
 }
 
 static int
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index b525353..d4aa88e 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -125,7 +125,8 @@ int parse_events_add_tracepoint(struct list_head *list, int *idx,
 int parse_events_load_bpf(struct parse_events_evlist *data,
 			  struct list_head *list,
 			  char *bpf_file_name,
-			  bool source);
+			  bool source,
+			  struct list_head *head_config);
 /* Provide this function for perf test */
 struct bpf_object;
 int parse_events_load_bpf_obj(struct parse_events_evlist *data,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 90e382f..255387a 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -64,6 +64,7 @@ static inc_group_count(struct list_head *list,
 %type <str> PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT
 %type <num> value_sym
 %type <head> event_config
+%type <head> event_bpf_config
 %type <term> event_term
 %type <head> event_pmu
 %type <head> event_legacy_symbol
@@ -468,27 +469,39 @@ PE_RAW
 }
 
 event_bpf_file:
-PE_BPF_OBJECT
+PE_BPF_OBJECT event_bpf_config
 {
 	struct parse_events_evlist *data = _data;
 	struct parse_events_error *error = data->error;
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_load_bpf(data, list, $1, false));
+	ABORT_ON(parse_events_load_bpf(data, list, $1, false, $2));
+	parse_events__free_terms($2);
 	$$ = list;
 }
 |
-PE_BPF_SOURCE
+PE_BPF_SOURCE event_bpf_config
 {
 	struct parse_events_evlist *data = _data;
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_load_bpf(data, list, $1, true));
+	ABORT_ON(parse_events_load_bpf(data, list, $1, true, $2));
+	parse_events__free_terms($2);
 	$$ = list;
 }
 
+event_bpf_config:
+'/' event_config '/'
+{
+	$$ = $2;
+}
+|
+{
+	$$ = NULL;
+}
+
 start_terms: event_config
 {
 	struct parse_events_terms *data = _data;
-- 
1.8.3.4


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

* [RFC PATCH 5/7] perf tools: Support setting different slots in a BPF map separately
  2015-10-17 10:48 [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Wang Nan
                   ` (3 preceding siblings ...)
  2015-10-17 10:48 ` [RFC PATCH 4/7] perf tools: Enable BPF object configure syntax Wang Nan
@ 2015-10-17 10:48 ` Wang Nan
  2015-11-20 13:25   ` Wangnan (F)
  2015-10-17 10:48 ` [RFC PATCH 6/7] perf tools: Enable indics setting syntax for BPF maps Wang Nan
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Wang Nan @ 2015-10-17 10:48 UTC (permalink / raw)
  To: acme, ast, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu, Wang Nan, Arnaldo Carvalho de Melo

This patch introduces basic facilities to support config different
slots in a BPF map one by one.

nr_indics and indics are introduced into 'struct parse_events_term',
where indics is an array of indics which will be configured by this
config term, nr_indics is the size of the array. The array is passed
to 'struct bpf_map_priv'. To indicate the new type of configuration,
BPF_MAP_PRIV_KEY_INDICS is added as a new key type.
bpf_map_config_foreach_key() is extended to iterate over those indics
instead of all possible keys.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David Ahern <dsahern@gmail.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kaixu Xia <xiakaixu@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/util/bpf-loader.c   | 68 +++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/parse-events.c |  4 ++-
 tools/perf/util/parse-events.h |  2 ++
 3 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 15cf27a..023fc12 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -638,6 +638,7 @@ int bpf__foreach_tev(struct bpf_object *obj,
 
 enum bpf_map_priv_key_type {
 	BPF_MAP_PRIV_KEY_ALL,
+	BPF_MAP_PRIV_KEY_INDICS,
 };
 
 enum bpf_map_priv_value_type {
@@ -647,6 +648,12 @@ enum bpf_map_priv_value_type {
 struct bpf_map_priv {
 	struct {
 		enum bpf_map_priv_key_type type;
+		union {
+			struct {
+				size_t nr_indics;
+				u64 *indics;
+			} indics;
+		};
 	} key;
 
 	struct {
@@ -663,6 +670,8 @@ bpf_map_priv__clear(struct bpf_map *map __maybe_unused,
 {
 	struct bpf_map_priv *priv = _priv;
 
+	if (priv->key.type == BPF_MAP_PRIV_KEY_INDICS)
+		zfree(&priv->key.indics.indics);
 	free(priv);
 }
 
@@ -718,6 +727,20 @@ bpf_map_config_foreach_key(struct bpf_map *map,
 				}
 			}
 			return 0;
+		case BPF_MAP_PRIV_KEY_INDICS:
+			for (i = 0; i < priv->key.indics.nr_indics; i++) {
+				u64 _idx = priv->key.indics.indics[i];
+				unsigned int idx = (unsigned int)(_idx);
+
+				err = (*func)(name, map_fd, &def,
+					      priv, &idx, arg);
+				if (err) {
+					pr_debug("ERROR: failed to insert value to %s[%u]\n",
+						 name, idx);
+					return err;
+				}
+			}
+			return 0;
 		default:
 			pr_debug("ERROR: keytype for map '%s' invalid\n", name);
 			return -EINVAL;
@@ -728,6 +751,28 @@ bpf_map_config_foreach_key(struct bpf_map *map,
 	}
 }
 
+static int
+bpf_map_priv_setkey(struct bpf_map_priv *priv,
+		    struct parse_events_term *term,
+		    const char *map_name)
+{
+	if (!term->nr_indics)
+		priv->key.type = BPF_MAP_PRIV_KEY_ALL;
+	else {
+		size_t memsz = term->nr_indics * sizeof(term->indics[0]);
+
+		priv->key.indics.indics = malloc(memsz);
+		if (!priv->key.indics.indics) {
+			pr_debug("No enough memory to alloc indics for %s\n",
+				 map_name);
+			return -ENOMEM;
+		}
+		memcpy(priv->key.indics.indics, term->indics, memsz);
+		priv->key.type = BPF_MAP_PRIV_KEY_INDICS;
+		priv->key.indics.nr_indics = term->nr_indics;
+	}
+	return 0;
+}
 
 static int
 bpf__config_obj_map_array_value(struct bpf_map *map,
@@ -773,7 +818,9 @@ bpf__config_obj_map_array_value(struct bpf_map *map,
 		return -ENOMEM;
 	}
 
-	priv->key.type = BPF_MAP_PRIV_KEY_ALL;
+	err = bpf_map_priv_setkey(priv, term, map_name);
+	if (err)
+		return err;
 	priv->value.type = BPF_MAP_PRIV_VAL_VALUE;
 	priv->value.val = term->val.num;
 	return bpf_map__set_private(map, priv, bpf_map_priv__clear);
@@ -834,6 +881,24 @@ bpf__config_obj_map(struct bpf_object *obj,
 		goto out;
 	}
 
+	if (term->nr_indics) {
+		struct bpf_map_def def;
+
+		err = bpf_map__get_def(map, &def);
+		if (err) {
+			pr_debug("ERROR: Unable to get map definition from '%s'\n",
+				 map_name);
+			goto out;
+		}
+		for (i = 0; i < term->nr_indics; i++)
+			if (term->indics[i] >= def.max_entries) {
+				pr_debug("ERROR: index %d too large\n",
+					 (int)term->indics[i]);
+				err = -E2BIG;
+				goto out;
+			}
+	}
+
 	for (i = 0; i < ARRAY_SIZE(bpf_config_map_funcs); i++) {
 		struct bpf_config_map_func *func = &bpf_config_map_funcs[i];
 
@@ -1004,6 +1069,7 @@ int bpf__strerror_config_obj(struct bpf_object *obj __maybe_unused,
 {
 	bpf__strerror_head(err, buf, size);
 	bpf__strerror_entry(ENODEV, "Invalid config option: '%s'", term->config)
+	bpf__strerror_entry(E2BIG,  "Index in '%s' too big", term->config)
 	bpf__strerror_entry(ENOENT, "Config target in '%s' is invalid", term->config)
 	bpf__strerror_entry(EBADF,  "Map type mismatch in '%s'", term->config)
 	bpf__strerror_entry(EINVAL, "Invalid config value")
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 9f081a1..42ac1cb 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2123,8 +2123,10 @@ void parse_events__free_terms(struct list_head *terms)
 {
 	struct parse_events_term *term, *h;
 
-	list_for_each_entry_safe(term, h, terms, list)
+	list_for_each_entry_safe(term, h, terms, list) {
+		free(term->indics);
 		free(term);
+	}
 }
 
 void parse_events_evlist_error(struct parse_events_evlist *data,
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index d4aa88e..5ba1d3e 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -71,6 +71,8 @@ enum {
 
 struct parse_events_term {
 	char *config;
+	size_t nr_indics;
+	u64 *indics;
 	union {
 		char *str;
 		u64  num;
-- 
1.8.3.4


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

* [RFC PATCH 6/7] perf tools: Enable indics setting syntax for BPF maps
  2015-10-17 10:48 [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Wang Nan
                   ` (4 preceding siblings ...)
  2015-10-17 10:48 ` [RFC PATCH 5/7] perf tools: Support setting different slots in a BPF map separately Wang Nan
@ 2015-10-17 10:48 ` Wang Nan
  2015-10-17 10:48 ` [RFC PATCH 7/7] perf tools: Enable passing event to BPF object Wang Nan
  2015-10-17 20:35 ` [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Alexei Starovoitov
  7 siblings, 0 replies; 18+ messages in thread
From: Wang Nan @ 2015-10-17 10:48 UTC (permalink / raw)
  To: acme, ast, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu, Wang Nan, Arnaldo Carvalho de Melo

This patch introduce a new syntax to perf event parser:

 # perf record -e bpf_file.c/maps.mymap.value[0,3-5,7]=1234/ ...

By utilizing the basic facilities in bpf-loader.c which allow setting
different slots in a BPF map separately, the newly introduced syntax
allows perf to control specific elements in a BPF map.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David Ahern <dsahern@gmail.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kaixu Xia <xiakaixu@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/util/parse-events.c |  4 +-
 tools/perf/util/parse-events.l | 11 +++++
 tools/perf/util/parse-events.y | 92 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 42ac1cb..a2af5a8 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -654,7 +654,9 @@ parse_events_config_bpf(struct parse_events_evlist *data,
 						 err, errbuf, sizeof(errbuf));
 			data->error->help = strdup(
 "Hint:\tValid config term:\n"
-"     \tmaps.<mapname>.value\n"
+"     \tmaps.<mapname>.value<indics>\n"
+"\n"
+"     \twhere <indics> is something like [0,3-4]\n"
 "     \t(add -v to see detail)");
 			data->error->str = strdup(errbuf);
 			if (err == -EINVAL)
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index eeea4e1..0d92a4d 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -111,6 +111,7 @@ do {							\
 %x mem
 %s config
 %x event
+%x array
 
 group		[^,{}/]*[{][^}]*[}][^,{}/]*
 event_pmu	[^,{}/]+[/][^/]*[/][^,{}/]*
@@ -176,6 +177,14 @@ modifier_bp	[rwx]{1,3}
 
 }
 
+<array>{
+"]"			{ BEGIN(config); return ']'; }
+{num_dec}		{ return value(yyscanner, 10); }
+{num_hex}		{ return value(yyscanner, 16); }
+,			{ return ','; }
+-			{ return '-'; }
+}
+
 <config>{
 	/*
 	 * Please update parse_events_formats_error_string any time
@@ -194,6 +203,8 @@ stack-size		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_STACKSIZE); }
 ,			{ return ','; }
 "/"			{ BEGIN(INITIAL); return '/'; }
 {name_minus}		{ return str(yyscanner, PE_NAME); }
+\[all\]			{ return PE_ARRAY_ALL; }
+"["			{ BEGIN(array); return '['; }
 }
 
 <mem>{
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 255387a..81fe702 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -48,6 +48,7 @@ static inc_group_count(struct list_head *list,
 %token PE_PREFIX_MEM PE_PREFIX_RAW PE_PREFIX_GROUP
 %token PE_ERROR
 %token PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT
+%token PE_ARRAY_ALL
 %type <num> PE_VALUE
 %type <num> PE_VALUE_SYM_HW
 %type <num> PE_VALUE_SYM_SW
@@ -84,6 +85,9 @@ static inc_group_count(struct list_head *list,
 %type <head> group_def
 %type <head> group
 %type <head> groups
+%type <array> array
+%type <array> array_term
+%type <array> array_terms
 
 %union
 {
@@ -95,6 +99,10 @@ static inc_group_count(struct list_head *list,
 		char *sys;
 		char *event;
 	} tracepoint_name;
+	struct array {
+		size_t nr_indics;
+		u64 *indics;
+	} array;
 }
 %%
 
@@ -599,6 +607,90 @@ PE_TERM
 	ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, 1, &@1, NULL));
 	$$ = term;
 }
+|
+PE_NAME array '=' PE_NAME
+{
+	struct parse_events_term *term;
+	int i;
+
+	ABORT_ON(parse_events_term__str(&term, PARSE_EVENTS__TERM_TYPE_USER,
+					$1, $4, &@1, &@4));
+
+	term->nr_indics = $2.nr_indics;
+	term->indics = $2.indics;
+	$$ = term;
+}
+|
+PE_NAME array '=' PE_VALUE
+{
+	struct parse_events_term *term;
+
+	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
+					$1, $4, &@1, &@4));
+	term->nr_indics = $2.nr_indics;
+	term->indics = $2.indics;
+	$$ = term;
+}
+
+array:
+'[' array_terms ']'
+{
+	$$ = $2;
+}
+|
+PE_ARRAY_ALL
+{
+	$$.nr_indics = 0;
+	$$.indics = NULL;
+}
+
+array_terms:
+array_terms ',' array_term
+{
+	struct array array = $1;
+	struct array new_array = $3;
+	u64 *new_indics;
+
+	new_indics = realloc(array.indics,
+			     (array.nr_indics + new_array.nr_indics) * sizeof(u64));
+	ABORT_ON(!new_indics);
+
+	memcpy(&new_indics[array.nr_indics], new_array.indics,
+	       sizeof(u64) * new_array.nr_indics);
+
+	array.nr_indics += new_array.nr_indics;
+	array.indics = new_indics;
+	free(new_array.indics);
+	$$ = array;
+}
+|
+array_term
+
+array_term:
+PE_VALUE
+{
+	struct array array;
+	array.nr_indics = 1;
+	array.indics = malloc(sizeof(u64));
+	ABORT_ON(!array.indics);
+	array.indics[0] = $1;
+	$$ = array;
+}
+|
+PE_VALUE '-' PE_VALUE
+{
+	struct array array;
+	int i;
+
+	ABORT_ON($3 < $1);
+	array.nr_indics = $3 - $1 + 1;
+	array.indics = malloc(sizeof(u64) * array.nr_indics);
+	ABORT_ON(!array.indics);
+
+	for (i = 0; i < array.nr_indics; i++)
+		array.indics[i] = $1 + i;
+	$$ = array;
+}
 
 sep_dc: ':' |
 
-- 
1.8.3.4


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

* [RFC PATCH 7/7] perf tools: Enable passing event to BPF object
  2015-10-17 10:48 [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Wang Nan
                   ` (5 preceding siblings ...)
  2015-10-17 10:48 ` [RFC PATCH 6/7] perf tools: Enable indics setting syntax for BPF maps Wang Nan
@ 2015-10-17 10:48 ` Wang Nan
  2015-10-17 20:35 ` [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Alexei Starovoitov
  7 siblings, 0 replies; 18+ messages in thread
From: Wang Nan @ 2015-10-17 10:48 UTC (permalink / raw)
  To: acme, ast, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu, Wang Nan, Arnaldo Carvalho de Melo

A new syntax is appended into parser so user can pass predefined perf
events into BPF objects.

After this patch, BPF programs for perf are finally able to utilize
bpf_perf_event_read() introduced in commit 35578d7984003097af2b1e3
(bpf: Implement function bpf_perf_event_read() that get the selected
hardware PMU conuter) in following way:

 ===== BPF program bpf_program.c =====

 struct bpf_map_def SEC("maps") pmu_map = {
     .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
     .key_size = sizeof(int),
     .value_size = sizeof(u32),
     .max_entries = __NR_CPUS__,
 };

 SEC("func_write=sys_write")
 int func_write(void *ctx)
 {
     unsigned long long val;
     char fmt[] = "sys_write:        pmu=%llu\n";
     val = bpf_perf_event_read(&pmu_map, bpf_get_smp_processor_id());
     bpf_trace_printk(fmt, sizeof(fmt), val);
     return 0;
 }

 SEC("func_write_return=sys_write%return")
 int func_write_return(void *ctx)
 {
     unsigned long long val = 0;
     char fmt[] = "sys_write_return: pmu=%llu\n";
     val = bpf_perf_event_read(&pmu_map, bpf_get_smp_processor_id());
     bpf_trace_printk(fmt, sizeof(fmt), val);
     return 0;
 }

 ===== cmdline =====
 # echo "" > /sys/kernel/debug/tracing/trace
 # perf record -e evt=cycles/period=0x7fffffffffffffff/ \
               -e bpf_program.c/maps.pmu_map.event[all]=evt/
               -a ls
 # cat /sys/kernel/debug/tracing/trace | grep ls
              ls-3363  [003] d... 75475.056190: : sys_write:        pmu=3961415
              ls-3363  [003] dN.. 75475.056212: : sys_write_return: pmu=4051390
              ls-3363  [003] d... 75475.056216: : sys_write:        pmu=4065447
              ls-3363  [003] dN.. 75475.056227: : sys_write_return: pmu=4109760
              ls-3363  [003] d... 75475.056230: : sys_write:        pmu=4120776
              ls-3363  [003] dN.. 75475.056245: : sys_write_return: pmu=4178441
              ...
 # perf report --stdio
 Error:
 The perf.data file has no samples!

Where, setting period of cycles Set a very large value to period
of cycles event because we want to use this event as a counter
only, don't need sampling.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David Ahern <dsahern@gmail.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kaixu Xia <xiakaixu@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/n/ebpf-2mjd96mowgzslkj8jrwbnwg7@git.kernel.org
---
 tools/perf/util/bpf-loader.c   | 106 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/parse-events.c |   2 +-
 2 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 023fc12..e185fb6 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -643,6 +643,7 @@ enum bpf_map_priv_key_type {
 
 enum bpf_map_priv_value_type {
 	BPF_MAP_PRIV_VAL_VALUE,
+	BPF_MAP_PRIV_VAL_EVSEL,
 };
 
 struct bpf_map_priv {
@@ -660,6 +661,7 @@ struct bpf_map_priv {
 		enum bpf_map_priv_value_type type;
 		union {
 			u64 val;
+			struct perf_evsel *evsel;
 		};
 	} value;
 };
@@ -715,6 +717,7 @@ bpf_map_config_foreach_key(struct bpf_map *map,
 
 	switch (def.type) {
 	case BPF_MAP_TYPE_ARRAY:
+	case BPF_MAP_TYPE_PERF_EVENT_ARRAY:
 		switch (priv->key.type) {
 		case BPF_MAP_PRIV_KEY_ALL:
 			for (i = 0; i < def.max_entries; i++) {
@@ -838,6 +841,69 @@ bpf__config_obj_map_value(struct bpf_map *map,
 	return -EINVAL;
 }
 
+static int
+bpf__config_obj_map_array_event(struct bpf_map *map,
+				struct parse_events_term *term,
+				struct perf_evlist *evlist)
+{
+	struct bpf_map_priv *priv;
+	struct perf_evsel *evsel;
+	struct bpf_map_def def;
+	const char *map_name;
+	int err;
+
+	map_name = bpf_map__get_name(map);
+	evsel = perf_evlist__find_evsel_by_alias(evlist, term->val.str);
+	if (!evsel) {
+		pr_debug("Event (for '%s') '%s' doesn't exist\n",
+			 map_name, term->val.str);
+		return -EINVAL;
+	}
+
+	err = bpf_map__get_def(map, &def);
+	if (err) {
+		pr_debug("Unable to get map definition from '%s'\n",
+			 map_name);
+		return -EINVAL;
+	}
+	
+	/*
+	 * No need to check key_size and value_size:
+	 * kernel has already checked them.
+	 */
+	if (def.type != BPF_MAP_TYPE_PERF_EVENT_ARRAY) {
+		pr_debug("Map %s type is not BPF_MAP_TYPE_PERF_EVENT_ARRAY\n",
+			 map_name);
+		return -EINVAL;
+	}
+
+	priv = zalloc(sizeof(*priv));
+	if (!priv) {
+		pr_debug("No enough memory for map private\n");
+		return -ENOMEM;
+	}
+
+	err = bpf_map_priv_setkey(priv, term, map_name);
+	if (err)
+		return err;
+
+	priv->value.type = BPF_MAP_PRIV_VAL_EVSEL;
+	priv->value.evsel = evsel;
+	return bpf_map__set_private(map, priv, bpf_map_priv__clear);
+}
+
+static int
+bpf__config_obj_map_event(struct bpf_map *map,
+			  struct parse_events_term *term,
+			  struct perf_evlist *evlist)
+{
+	if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
+		return bpf__config_obj_map_array_event(map, term, evlist);
+
+	pr_debug("ERROR: wrong value type\n");
+	return -EINVAL;
+}
+
 struct bpf_config_map_func {
 	const char *config_opt;
 	int (*config_func)(struct bpf_map *, struct parse_events_term *,
@@ -846,6 +912,7 @@ struct bpf_config_map_func {
 
 struct bpf_config_map_func bpf_config_map_funcs[] = {
 	{"value", bpf__config_obj_map_value},
+	{"event", bpf__config_obj_map_event},
 };
 
 static int
@@ -964,6 +1031,40 @@ bpf__apply_config_value_for_key(int map_fd, void *pkey,
 }
 
 static int
+bpf__apply_config_evsel_for_key(const char *name, int map_fd, void *pkey,
+				struct perf_evsel *evsel)
+{
+	struct xyarray *xy = evsel->fd;
+	unsigned int key, events;
+	int *evt_fd;
+	int err;
+
+	if (!xy) {
+		pr_debug("ERROR: evsel not ready for map %s\n", name);
+		return -EINVAL;
+	}
+
+	if (xy->row_size / xy->entry_size != 1) {
+		pr_debug("ERROR: Dimension of target event is incorrect for map %s\n",
+			 name);
+		return -EINVAL;
+	}
+
+	events = xy->entries / (xy->row_size / xy->entry_size);
+	key = *((unsigned int *)pkey);
+	if (key >= events) {
+		pr_debug("ERROR: there is no event %d for map %s\n",
+			 key, name);
+		return -E2BIG;
+	}
+	evt_fd = xyarray__entry(xy, key, 0);
+	err = bpf_map_update_elem(map_fd, pkey, evt_fd, BPF_ANY);
+	if (err && errno)
+		err = -errno;
+	return err;
+}
+
+static int
 bpf__apply_config_map_for_key(const char *name, int map_fd,
 			      struct bpf_map_def *pdef __maybe_unused,
 			      struct bpf_map_priv *priv,
@@ -977,6 +1078,10 @@ bpf__apply_config_map_for_key(const char *name, int map_fd,
 						      pdef->value_size,
 						      priv->value.val);
 		break;
+	case BPF_MAP_PRIV_VAL_EVSEL:
+		err = bpf__apply_config_evsel_for_key(name, map_fd, pkey,
+						      priv->value.evsel);
+		break;
 	default:
 		pr_debug("ERROR: unknown value type for '%s'\n", name);
 		err = -EINVAL;
@@ -1081,6 +1186,7 @@ int bpf__strerror_apply_config(int err, char *buf, size_t size)
 {
 	bpf__strerror_head(err, buf, size);
 	bpf__strerror_entry(EINVAL, "Invalid option for map, add -v to see detail");
+	bpf__strerror_entry(E2BIG,  "Array index too big, add -v to see detail");
 	bpf__strerror_end(buf, size);
 	return 0;
 }
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index a2af5a8..ae973cd 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -654,7 +654,7 @@ parse_events_config_bpf(struct parse_events_evlist *data,
 						 err, errbuf, sizeof(errbuf));
 			data->error->help = strdup(
 "Hint:\tValid config term:\n"
-"     \tmaps.<mapname>.value<indics>\n"
+"     \tmaps.<mapname>.[value|event]<indics>\n"
 "\n"
 "     \twhere <indics> is something like [0,3-4]\n"
 "     \t(add -v to see detail)");
-- 
1.8.3.4


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

* Re: [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline
  2015-10-17 10:48 [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Wang Nan
                   ` (6 preceding siblings ...)
  2015-10-17 10:48 ` [RFC PATCH 7/7] perf tools: Enable passing event to BPF object Wang Nan
@ 2015-10-17 20:35 ` Alexei Starovoitov
  2015-10-17 23:58   ` Wangnan (F)
  7 siblings, 1 reply; 18+ messages in thread
From: Alexei Starovoitov @ 2015-10-17 20:35 UTC (permalink / raw)
  To: Wang Nan, acme, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu

On 10/17/15 3:48 AM, Wang Nan wrote:
> In these 7 patches:
>
>   1. perf is able to put values into map:
>    # perf record -e mybpf.c/maps.values.value=1234/ ...
>
>   2. perf is able to control different slots in a map separately:
>    # perf record -e mybpf.c/maps.values.value[1,4-6]=1234,maps.values.value[0,2-3]=5678/ ...
>
>   3. The second syntax can be applied to perf event also:
>    # perf record -v -a -e evt=cycles -e mybpf.c/maps.pmu_map.event[0]=evt/ ...
>
>   4. Compatible with the old syntax:
>    # perf record -v -a -e evt=cycles -e mybpf.c/maps.pmu_map.event=evt/ ...

The concept looks good and solves real need.
No opinion on implementation.


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

* Re: [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline
  2015-10-17 20:35 ` [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Alexei Starovoitov
@ 2015-10-17 23:58   ` Wangnan (F)
  2015-10-18  0:07     ` Alexei Starovoitov
  0 siblings, 1 reply; 18+ messages in thread
From: Wangnan (F) @ 2015-10-17 23:58 UTC (permalink / raw)
  To: Alexei Starovoitov, acme, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu



On 2015/10/18 4:35, Alexei Starovoitov wrote:
> On 10/17/15 3:48 AM, Wang Nan wrote:
>> In these 7 patches:
>>
>>   1. perf is able to put values into map:
>>    # perf record -e mybpf.c/maps.values.value=1234/ ...
>>
>>   2. perf is able to control different slots in a map separately:
>>    # perf record -e 
>> mybpf.c/maps.values.value[1,4-6]=1234,maps.values.value[0,2-3]=5678/ ...
>>
>>   3. The second syntax can be applied to perf event also:
>>    # perf record -v -a -e evt=cycles -e 
>> mybpf.c/maps.pmu_map.event[0]=evt/ ...
>>
>>   4. Compatible with the old syntax:
>>    # perf record -v -a -e evt=cycles -e 
>> mybpf.c/maps.pmu_map.event=evt/ ...
>
> The concept looks good and solves real need.
> No opinion on implementation.
>
Can I translate these words into an acked-by?

And what's your PERF_COUNT_SW_BPF_OUTPUT going on? I think based on this
patchset you can test it with perf now.

Thank you.


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

* Re: [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline
  2015-10-17 23:58   ` Wangnan (F)
@ 2015-10-18  0:07     ` Alexei Starovoitov
  0 siblings, 0 replies; 18+ messages in thread
From: Alexei Starovoitov @ 2015-10-18  0:07 UTC (permalink / raw)
  To: Wangnan (F), acme, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu

On 10/17/15 4:58 PM, Wangnan (F) wrote:
> Can I translate these words into an acked-by?

sorry nope. acks for implementation, but I didn't review it yet.

> And what's your PERF_COUNT_SW_BPF_OUTPUT going on? I think based on this
> patchset you can test it with perf now.

few more lines would still be needed in perf.
I'm testing it.


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

* Re: [RFC PATCH 4/7] perf tools: Enable BPF object configure syntax
  2015-10-17 10:48 ` [RFC PATCH 4/7] perf tools: Enable BPF object configure syntax Wang Nan
@ 2015-10-23  4:48   ` Wangnan (F)
  0 siblings, 0 replies; 18+ messages in thread
From: Wangnan (F) @ 2015-10-23  4:48 UTC (permalink / raw)
  To: acme, ast, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu, Arnaldo Carvalho de Melo



On 2015/10/17 18:48, Wang Nan wrote:

[SNIP]
>   
>   event_bpf_file:
> -PE_BPF_OBJECT
> +PE_BPF_OBJECT event_bpf_config
>   {
>   	struct parse_events_evlist *data = _data;
>   	struct parse_events_error *error = data->error;
>   	struct list_head *list;
>   
>   	ALLOC_LIST(list);
> -	ABORT_ON(parse_events_load_bpf(data, list, $1, false));
> +	ABORT_ON(parse_events_load_bpf(data, list, $1, false, $2));
> +	parse_events__free_terms($2);

Here found a bug that $2 is possible to be NULL, but parse_events_free_terms
can't accept NULL param.

Will be fixed in next version.

Thank you.

>   	$$ = list;
>   }
>   |
> -PE_BPF_SOURCE
> +PE_BPF_SOURCE event_bpf_config
>   {
>   	struct parse_events_evlist *data = _data;
>   	struct list_head *list;
>   
>   	ALLOC_LIST(list);
> -	ABORT_ON(parse_events_load_bpf(data, list, $1, true));
> +	ABORT_ON(parse_events_load_bpf(data, list, $1, true, $2));
> +	parse_events__free_terms($2);
>   	$$ = list;
>   }
>   
> +event_bpf_config:
> +'/' event_config '/'
> +{
> +	$$ = $2;
> +}
> +|
> +{
> +	$$ = NULL;
> +}
> +
>   start_terms: event_config
>   {
>   	struct parse_events_terms *data = _data;



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

* Re: [RFC PATCH 1/7] perf tools: Add API to config maps in bpf object
  2015-10-17 10:48 ` [RFC PATCH 1/7] perf tools: Add API to config maps in bpf object Wang Nan
@ 2015-11-20  8:13   ` Wangnan (F)
  2015-11-23 11:20   ` Wangnan (F)
  1 sibling, 0 replies; 18+ messages in thread
From: Wangnan (F) @ 2015-11-20  8:13 UTC (permalink / raw)
  To: acme, ast, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu, Arnaldo Carvalho de Melo



On 2015/10/17 18:48, Wang Nan wrote:
> bpf__config_obj() is introduced as a core API to config BPF object
> after loading. One configuration option of maps is introduced. After
> this patch BPF object can accept configuration like:
>
>   maps.my_map.value=1234

There's an inconvience in this syntax.

In following cmdline:

  # perf record -e mybpf.c/maps.channel.value=1234/ ls

because of the greedy manner of flex, mybpf.c/maps.c would
be expressed as path of a BPF source file (and yes, it is a valid
path).

If flex has a non-greedy mode then it would be fixed easily. However,
the official flex docs reveals its policy that it doesn't and would not
provide non-greedy matching. Even if we have non-greedy matching,
we are unable to prohibit user to put their BPF object into path
like

/home/user/mybpf.c/thefile.c

Fortunately this patch has not beed merged, so we have a chance to fix
it at very beginning. I will replace all '.' in object config string
to ':', so the above cmdline becomes:

  # perf record -e mybpf.c/maps:channel:value=1234/ ls

[1] 
http://flex.sourceforge.net/manual/Why-doesn_0027t-flex-have-non_002dgreedy-operators-like-perl-does_003f.html



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

* Re: [RFC PATCH 5/7] perf tools: Support setting different slots in a BPF map separately
  2015-10-17 10:48 ` [RFC PATCH 5/7] perf tools: Support setting different slots in a BPF map separately Wang Nan
@ 2015-11-20 13:25   ` Wangnan (F)
  2015-11-20 15:34     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 18+ messages in thread
From: Wangnan (F) @ 2015-11-20 13:25 UTC (permalink / raw)
  To: acme, ast, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu, Arnaldo Carvalho de Melo



On 2015/10/17 18:48, Wang Nan wrote:
> This patch introduces basic facilities to support config different
> slots in a BPF map one by one.
>
> nr_indics and indics are introduced into 'struct parse_events_term',
> where indics is an array of indics which will be configured by this
> config term, nr_indics is the size of the array. The array is passed
> to 'struct bpf_map_priv'. To indicate the new type of configuration,
> BPF_MAP_PRIV_KEY_INDICS is added as a new key type.
> bpf_map_config_foreach_key() is extended to iterate over those indics
> instead of all possible keys.
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Alexei Starovoitov <ast@plumgrid.com>
> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: He Kuang <hekuang@huawei.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kaixu Xia <xiakaixu@huawei.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: pi3orama@163.com
> ---
>   tools/perf/util/bpf-loader.c   | 68 +++++++++++++++++++++++++++++++++++++++++-
>   tools/perf/util/parse-events.c |  4 ++-
>   tools/perf/util/parse-events.h |  2 ++
>   3 files changed, 72 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 15cf27a..023fc12 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -638,6 +638,7 @@ int bpf__foreach_tev(struct bpf_object *obj,
>   
>   enum bpf_map_priv_key_type {
>   	BPF_MAP_PRIV_KEY_ALL,
> +	BPF_MAP_PRIV_KEY_INDICS,
>   };
>   
>   enum bpf_map_priv_value_type {
> @@ -647,6 +648,12 @@ enum bpf_map_priv_value_type {
>   struct bpf_map_priv {
>   	struct {
>   		enum bpf_map_priv_key_type type;
> +		union {
> +			struct {
> +				size_t nr_indics;
> +				u64 *indics;
> +			} indics;
> +		};
>   	} key;
>   
>   	struct {
> @@ -663,6 +670,8 @@ bpf_map_priv__clear(struct bpf_map *map __maybe_unused,
>   {
>   	struct bpf_map_priv *priv = _priv;
>   
> +	if (priv->key.type == BPF_MAP_PRIV_KEY_INDICS)
> +		zfree(&priv->key.indics.indics);
>   	free(priv);
>   }
>   
> @@ -718,6 +727,20 @@ bpf_map_config_foreach_key(struct bpf_map *map,
>   				}
>   			}
>   			return 0;
> +		case BPF_MAP_PRIV_KEY_INDICS:
> +			for (i = 0; i < priv->key.indics.nr_indics; i++) {
> +				u64 _idx = priv->key.indics.indics[i];
> +				unsigned int idx = (unsigned int)(_idx);
> +
> +				err = (*func)(name, map_fd, &def,
> +					      priv, &idx, arg);
> +				if (err) {
> +					pr_debug("ERROR: failed to insert value to %s[%u]\n",
> +						 name, idx);
> +					return err;
> +				}
> +			}

This for-loop has a potential problem that, if perf's user want to
set a very big array using indices, for example:

  # perf record -e 
mybpf.c/maps:mymap:values[1,2,3,10-100000,200000-400000]=3/ 
mybpf.c/maps:mymap:values[100000-200000]=3/ ...

Perf would alloc nearly 300000 slots for indices array, consume too much 
memory.

I will fix this problem by reinterprete indices array, makes negative
value represent range start and use next slot to store range size. For
example, the above perf cmdline can be converted to:

{1,2,3,-10, 99991,-200000,200001} and {-100000,100001}.

Thank you.


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

* Re: [RFC PATCH 5/7] perf tools: Support setting different slots in a BPF map separately
  2015-11-20 13:25   ` Wangnan (F)
@ 2015-11-20 15:34     ` Arnaldo Carvalho de Melo
  2015-11-23  2:01       ` Wangnan (F)
  0 siblings, 1 reply; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-20 15:34 UTC (permalink / raw)
  To: Wangnan (F)
  Cc: ast, brendan.d.gregg, a.p.zijlstra, daniel, dsahern, hekuang,
	jolsa, lizefan, Ingo Molnar, masami.hiramatsu.pt, namhyung,
	paulus, linux-kernel, pi3orama, xiakaixu

Em Fri, Nov 20, 2015 at 09:25:36PM +0800, Wangnan (F) escreveu:
> >+		case BPF_MAP_PRIV_KEY_INDICS:
> >+			for (i = 0; i < priv->key.indics.nr_indics; i++) {
> >+				u64 _idx = priv->key.indics.indics[i];
> >+				unsigned int idx = (unsigned int)(_idx);
> >+
> >+				err = (*func)(name, map_fd, &def,
> >+					      priv, &idx, arg);
> >+				if (err) {
> >+					pr_debug("ERROR: failed to insert value to %s[%u]\n",
> >+						 name, idx);
> >+					return err;
> >+				}
> >+			}
> 
> This for-loop has a potential problem that, if perf's user want to
> set a very big array using indices, for example:
> 
>  # perf record -e
> mybpf.c/maps:mymap:values[1,2,3,10-100000,200000-400000]=3/
> mybpf.c/maps:mymap:values[100000-200000]=3/ ...
> 
> Perf would alloc nearly 300000 slots for indices array, consume too much
> memory.
> 
> I will fix this problem by reinterprete indices array, makes negative
> value represent range start and use next slot to store range size. For
> example, the above perf cmdline can be converted to:
> 
> {1,2,3,-10, 99991,-200000,200001} and {-100000,100001}.

Why is that changing the way you specify what entries should be set to
a value will make it not allocate too much memory?

I found the first form of representing ( start-end ) to be better than (
-start, size ), but I would use what the C language uses for expressing
ranges in switch case ranges, which is familiar and doesn't reuses the
minus arithmetic operator to express a range, i.e.:

 # perf record -e \
   mybpf.c/maps:mymap:values[1,2,3,10..100000,200000..400000]=3/

 # perf record -e \
   mybpf.c/maps:mymap:values[100000..200000]=3/ ...

- Arnaldo

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

* Re: [RFC PATCH 5/7] perf tools: Support setting different slots in a BPF map separately
  2015-11-20 15:34     ` Arnaldo Carvalho de Melo
@ 2015-11-23  2:01       ` Wangnan (F)
  2015-11-23  5:45         ` Wangnan (F)
  0 siblings, 1 reply; 18+ messages in thread
From: Wangnan (F) @ 2015-11-23  2:01 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: ast, brendan.d.gregg, a.p.zijlstra, daniel, dsahern, hekuang,
	jolsa, lizefan, Ingo Molnar, masami.hiramatsu.pt, namhyung,
	paulus, linux-kernel, pi3orama, xiakaixu



On 2015/11/20 23:34, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 20, 2015 at 09:25:36PM +0800, Wangnan (F) escreveu:
>>> +		case BPF_MAP_PRIV_KEY_INDICS:
>>> +			for (i = 0; i < priv->key.indics.nr_indics; i++) {
>>> +				u64 _idx = priv->key.indics.indics[i];
>>> +				unsigned int idx = (unsigned int)(_idx);
>>> +
>>> +				err = (*func)(name, map_fd, &def,
>>> +					      priv, &idx, arg);
>>> +				if (err) {
>>> +					pr_debug("ERROR: failed to insert value to %s[%u]\n",
>>> +						 name, idx);
>>> +					return err;
>>> +				}
>>> +			}
>> This for-loop has a potential problem that, if perf's user want to
>> set a very big array using indices, for example:
>>
>>   # perf record -e
>> mybpf.c/maps:mymap:values[1,2,3,10-100000,200000-400000]=3/
>> mybpf.c/maps:mymap:values[100000-200000]=3/ ...
>>
>> Perf would alloc nearly 300000 slots for indices array, consume too much
>> memory.
>>
>> I will fix this problem by reinterprete indices array, makes negative
>> value represent range start and use next slot to store range size. For
>> example, the above perf cmdline can be converted to:
>>
>> {1,2,3,-10, 99991,-200000,200001} and {-100000,100001}.
> Why is that changing the way you specify what entries should be set to
> a value will make it not allocate too much memory?

It is actually a problem in the next patch, in which it expand all range
into a series of indices. If user wants 1-10000, it creates an array as
[1,2,3,4,...10000], so user is possible to use a simple cmdline to consume
all of available memory.

However, the method I described above is not the best way to solve this 
probelm.
I thought yesterday that we should not insist on indices array. We can
make parser always return ranges. For example, [1,2,3-5] can be represent
using [(1,1), (2,1), (3,3)], so we don't need the above ugly negative
indicators.

> I found the first form of representing ( start-end ) to be better than (
> -start, size ), but I would use what the C language uses for expressing
> ranges in switch case ranges, which is familiar and doesn't reuses the
> minus arithmetic operator to express a range, i.e.:
>
>   # perf record -e \
>     mybpf.c/maps:mymap:values[1,2,3,10..100000,200000..400000]=3/
>
>   # perf record -e \
>     mybpf.c/maps:mymap:values[100000..200000]=3/ ...

'..' is better.

Thank you.

> - Arnaldo



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

* Re: [RFC PATCH 5/7] perf tools: Support setting different slots in a BPF map separately
  2015-11-23  2:01       ` Wangnan (F)
@ 2015-11-23  5:45         ` Wangnan (F)
  0 siblings, 0 replies; 18+ messages in thread
From: Wangnan (F) @ 2015-11-23  5:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: ast, brendan.d.gregg, a.p.zijlstra, daniel, dsahern, hekuang,
	jolsa, lizefan, Ingo Molnar, masami.hiramatsu.pt, namhyung,
	paulus, linux-kernel, pi3orama, xiakaixu



On 2015/11/23 10:01, Wangnan (F) wrote:
>
>
> On 2015/11/20 23:34, Arnaldo Carvalho de Melo wrote:
>> Em Fri, Nov 20, 2015 at 09:25:36PM +0800, Wangnan (F) escreveu:
>>>> +        case BPF_MAP_PRIV_KEY_INDICS:
>>>> +            for (i = 0; i < priv->key.indics.nr_indics; i++) {
>>>> +                u64 _idx = priv->key.indics.indics[i];
>>>> +                unsigned int idx = (unsigned int)(_idx);
>>>> +
>>>> +                err = (*func)(name, map_fd, &def,
>>>> +                          priv, &idx, arg);
>>>> +                if (err) {
>>>> +                    pr_debug("ERROR: failed to insert value to 
>>>> %s[%u]\n",
>>>> +                         name, idx);
>>>> +                    return err;
>>>> +                }
>>>> +            }
>>> This for-loop has a potential problem that, if perf's user want to
>>> set a very big array using indices, for example:
>>>
>>>   # perf record -e
>>> mybpf.c/maps:mymap:values[1,2,3,10-100000,200000-400000]=3/
>>> mybpf.c/maps:mymap:values[100000-200000]=3/ ...
>>>
>>> Perf would alloc nearly 300000 slots for indices array, consume too 
>>> much
>>> memory.
>>>
>>> I will fix this problem by reinterprete indices array, makes negative
>>> value represent range start and use next slot to store range size. For
>>> example, the above perf cmdline can be converted to:
>>>
>>> {1,2,3,-10, 99991,-200000,200001} and {-100000,100001}.
>> Why is that changing the way you specify what entries should be set to
>> a value will make it not allocate too much memory?
>
> It is actually a problem in the next patch, in which it expand all range
> into a series of indices. If user wants 1-10000, it creates an array as
> [1,2,3,4,...10000], so user is possible to use a simple cmdline to 
> consume
> all of available memory.
>
> However, the method I described above is not the best way to solve 
> this probelm.
> I thought yesterday that we should not insist on indices array. We can
> make parser always return ranges. For example, [1,2,3-5] can be represent
> using [(1,1), (2,1), (3,3)], so we don't need the above ugly negative
> indicators.
>
>> I found the first form of representing ( start-end ) to be better than (
>> -start, size ), but I would use what the C language uses for expressing
>> ranges in switch case ranges, which is familiar and doesn't reuses the
>> minus arithmetic operator to express a range, i.e.:
>>
>>   # perf record -e \
>> mybpf.c/maps:mymap:values[1,2,3,10..100000,200000..400000]=3/
>>
>>   # perf record -e \
>>     mybpf.c/maps:mymap:values[100000..200000]=3/ ...
>
> '..' is better.
>

One problem: the case range syntax is introduced by gcc extension, not a
part of standard, and should be '...'.

Please see: https://gcc.gnu.org/onlinedocs/gcc/Case-Ranges.html

So I'll use '...' also.

Thank you.



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



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

* Re: [RFC PATCH 1/7] perf tools: Add API to config maps in bpf object
  2015-10-17 10:48 ` [RFC PATCH 1/7] perf tools: Add API to config maps in bpf object Wang Nan
  2015-11-20  8:13   ` Wangnan (F)
@ 2015-11-23 11:20   ` Wangnan (F)
  1 sibling, 0 replies; 18+ messages in thread
From: Wangnan (F) @ 2015-11-23 11:20 UTC (permalink / raw)
  To: acme, ast, brendan.d.gregg
  Cc: a.p.zijlstra, daniel, dsahern, hekuang, jolsa, lizefan,
	masami.hiramatsu.pt, namhyung, paulus, linux-kernel, pi3orama,
	xiakaixu, Arnaldo Carvalho de Melo



On 2015/10/17 18:48, Wang Nan wrote:
> bpf__config_obj() is introduced as a core API to config BPF object
> after loading. One configuration option of maps is introduced. After
> this patch BPF object can accept configuration like:
>
>   maps.my_map.value=1234
>
> This patch is more complex than the work it really does because the
> consideration of extension. In designing of BPF map configuration,
> following things should be considered:
>
>   1. Array indics selection: perf should allow user setting different
>      value to different slots in an array, with syntax like:
>      maps.my_map.value[0,3-6]=1234;
>
>   2. Type of value: integer is not the only valid value type. Perf
>      event can also be put into a map after commit 35578d7984003097af2b1e3
>      (bpf: Implement function bpf_perf_event_read() that get the selected
>      hardware PMU conuter);
>
>   3. For hash table, it is possible to use string or other as key;
>
>   4. It is possible that map configuration is unable to be setup
>      during parsing. Perf event is an example.
>
> Therefore, this patch does tie following thing for extension:
>
>   1. Instead of update map element during parsing, this patch stores
>      map config options in 'struct bpf_map_priv'. Following patches
>      would apply those configs at proper time;

Because of this delay-updating manner, current implementation forbid
setting a map with different configuration.
For example:

  # perf record -e 
test_bpf.c/maps:channel:value[0...9]=1,maps:channel:value[10...19]=2/ ...

is equal to
  # perf record -e test_bpf.c/maps:channel:value[10...19]=2/ ...

because [see follow]

>   2. Make 'struct bpf_map_priv' extensible so following patches can
>      add new key and value operations;
>
>   3. Use bpf_config_map_funcs array to support more maps configuration.
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: He Kuang <hekuang@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Alexei Starovoitov <ast@plumgrid.com>
> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: He Kuang <hekuang@huawei.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kaixu Xia <xiakaixu@huawei.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: pi3orama@163.com
> Link: http://lkml.kernel.org/n/ebpf-36xcrahy9n0ayc05mu7aajpk@git.kernel.org
> ---
>   

[SNIP]

>   
> +enum bpf_map_priv_key_type {
> +	BPF_MAP_PRIV_KEY_ALL,
> +};
> +
> +enum bpf_map_priv_value_type {
> +	BPF_MAP_PRIV_VAL_VALUE,
> +};
> +
> +struct bpf_map_priv {
> +	struct {
> +		enum bpf_map_priv_key_type type;
> +	} key;
> +
> +	struct {
> +		enum bpf_map_priv_value_type type;
> +		union {
> +			u64 val;
> +		};
> +	} value;
> +};
> +

... because this structure holds only one config term.

In next version I'd like to save multiple setting operations in
this structure.

Thank you.


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

end of thread, other threads:[~2015-11-23 11:21 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-17 10:48 [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Wang Nan
2015-10-17 10:48 ` [RFC PATCH 1/7] perf tools: Add API to config maps in bpf object Wang Nan
2015-11-20  8:13   ` Wangnan (F)
2015-11-23 11:20   ` Wangnan (F)
2015-10-17 10:48 ` [RFC PATCH 2/7] perf tools: Add API to apply config to BPF map Wang Nan
2015-10-17 10:48 ` [RFC PATCH 3/7] perf record: Apply config to BPF objects before recording Wang Nan
2015-10-17 10:48 ` [RFC PATCH 4/7] perf tools: Enable BPF object configure syntax Wang Nan
2015-10-23  4:48   ` Wangnan (F)
2015-10-17 10:48 ` [RFC PATCH 5/7] perf tools: Support setting different slots in a BPF map separately Wang Nan
2015-11-20 13:25   ` Wangnan (F)
2015-11-20 15:34     ` Arnaldo Carvalho de Melo
2015-11-23  2:01       ` Wangnan (F)
2015-11-23  5:45         ` Wangnan (F)
2015-10-17 10:48 ` [RFC PATCH 6/7] perf tools: Enable indics setting syntax for BPF maps Wang Nan
2015-10-17 10:48 ` [RFC PATCH 7/7] perf tools: Enable passing event to BPF object Wang Nan
2015-10-17 20:35 ` [RFC PATCH 0/7] perf tools: Config BPF maps through perf cmdline Alexei Starovoitov
2015-10-17 23:58   ` Wangnan (F)
2015-10-18  0:07     ` Alexei Starovoitov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.