From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Christy Lee <christylee@fb.com>
Cc: andrii@kernel.org, christyc.y.lee@gmail.com, bpf@vger.kernel.org,
kernel-team@fb.com, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH bpf-next 3/5] perf: stop using bpf_map__def() API
Date: Thu, 6 Jan 2022 17:24:51 -0300 [thread overview]
Message-ID: <YddQE5ZIgkarkRNL@kernel.org> (raw)
In-Reply-To: <20220105230057.853163-4-christylee@fb.com>
Em Wed, Jan 05, 2022 at 03:00:55PM -0800, Christy Lee escreveu:
> libbpf bpf_map__def() API is being deprecated, replace bpftool's
> usage with the appropriate getters and setters.
This log message is for perf, right?
- Arnaldo
> Signed-off-by: Christy Lee <christylee@fb.com>
> ---
> tools/perf/util/bpf-loader.c | 58 ++++++++++++++++--------------------
> tools/perf/util/bpf_map.c | 28 ++++++++---------
> 2 files changed, 39 insertions(+), 47 deletions(-)
>
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 528aeb0ab79d..ea5ccf0aed1b 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -1002,24 +1002,22 @@ __bpf_map__config_value(struct bpf_map *map,
> {
> struct bpf_map_op *op;
> const char *map_name = bpf_map__name(map);
> - const struct bpf_map_def *def = bpf_map__def(map);
>
> - if (IS_ERR(def)) {
> - pr_debug("Unable to get map definition from '%s'\n",
> - map_name);
> + if (!map) {
> + pr_debug("Map '%s' is invalid\n", map_name);
> return -BPF_LOADER_ERRNO__INTERNAL;
> }
>
> - if (def->type != BPF_MAP_TYPE_ARRAY) {
> + if (bpf_map__type(map) != BPF_MAP_TYPE_ARRAY) {
> pr_debug("Map %s type is not BPF_MAP_TYPE_ARRAY\n",
> map_name);
> return -BPF_LOADER_ERRNO__OBJCONF_MAP_TYPE;
> }
> - if (def->key_size < sizeof(unsigned int)) {
> + if (bpf_map__key_size(map) < sizeof(unsigned int)) {
> pr_debug("Map %s has incorrect key size\n", map_name);
> return -BPF_LOADER_ERRNO__OBJCONF_MAP_KEYSIZE;
> }
> - switch (def->value_size) {
> + switch (bpf_map__value_size(map)) {
> case 1:
> case 2:
> case 4:
> @@ -1061,7 +1059,6 @@ __bpf_map__config_event(struct bpf_map *map,
> struct parse_events_term *term,
> struct evlist *evlist)
> {
> - const struct bpf_map_def *def;
> struct bpf_map_op *op;
> const char *map_name = bpf_map__name(map);
> struct evsel *evsel = evlist__find_evsel_by_str(evlist, term->val.str);
> @@ -1072,18 +1069,16 @@ __bpf_map__config_event(struct bpf_map *map,
> return -BPF_LOADER_ERRNO__OBJCONF_MAP_NOEVT;
> }
>
> - def = bpf_map__def(map);
> - if (IS_ERR(def)) {
> - pr_debug("Unable to get map definition from '%s'\n",
> - map_name);
> - return PTR_ERR(def);
> + if (!map) {
> + pr_debug("Map '%s' is invalid\n", map_name);
> + return PTR_ERR(map);
> }
>
> /*
> * No need to check key_size and value_size:
> * kernel has already checked them.
> */
> - if (def->type != BPF_MAP_TYPE_PERF_EVENT_ARRAY) {
> + if (bpf_map__type(map) != BPF_MAP_TYPE_PERF_EVENT_ARRAY) {
> pr_debug("Map %s type is not BPF_MAP_TYPE_PERF_EVENT_ARRAY\n",
> map_name);
> return -BPF_LOADER_ERRNO__OBJCONF_MAP_TYPE;
> @@ -1132,7 +1127,6 @@ config_map_indices_range_check(struct parse_events_term *term,
> const char *map_name)
> {
> struct parse_events_array *array = &term->array;
> - const struct bpf_map_def *def;
> unsigned int i;
>
> if (!array->nr_ranges)
> @@ -1143,10 +1137,8 @@ config_map_indices_range_check(struct parse_events_term *term,
> return -BPF_LOADER_ERRNO__INTERNAL;
> }
>
> - def = bpf_map__def(map);
> - if (IS_ERR(def)) {
> - pr_debug("ERROR: Unable to get map definition from '%s'\n",
> - map_name);
> + if (!map) {
> + pr_debug("Map '%s' is invalid\n", map_name);
> return -BPF_LOADER_ERRNO__INTERNAL;
> }
>
> @@ -1155,7 +1147,7 @@ config_map_indices_range_check(struct parse_events_term *term,
> size_t length = array->ranges[i].length;
> unsigned int idx = start + length - 1;
>
> - if (idx >= def->max_entries) {
> + if (idx >= bpf_map__max_entries(map)) {
> pr_debug("ERROR: index %d too large\n", idx);
> return -BPF_LOADER_ERRNO__OBJCONF_MAP_IDX2BIG;
> }
> @@ -1248,21 +1240,21 @@ int bpf__config_obj(struct bpf_object *obj,
> }
>
> typedef int (*map_config_func_t)(const char *name, int map_fd,
> - const struct bpf_map_def *pdef,
> + const struct bpf_map *map,
> struct bpf_map_op *op,
> void *pkey, void *arg);
>
> static int
> foreach_key_array_all(map_config_func_t func,
> void *arg, const char *name,
> - int map_fd, const struct bpf_map_def *pdef,
> + int map_fd, const struct bpf_map *map,
> struct bpf_map_op *op)
> {
> unsigned int i;
> int err;
>
> - for (i = 0; i < pdef->max_entries; i++) {
> - err = func(name, map_fd, pdef, op, &i, arg);
> + for (i = 0; i < bpf_map__max_entries(map); i++) {
> + err = func(name, map_fd, map, op, &i, arg);
> if (err) {
> pr_debug("ERROR: failed to insert value to %s[%u]\n",
> name, i);
> @@ -1275,7 +1267,7 @@ foreach_key_array_all(map_config_func_t func,
> static int
> foreach_key_array_ranges(map_config_func_t func, void *arg,
> const char *name, int map_fd,
> - const struct bpf_map_def *pdef,
> + const struct bpf_map *map,
> struct bpf_map_op *op)
> {
> unsigned int i, j;
> @@ -1288,7 +1280,7 @@ foreach_key_array_ranges(map_config_func_t func, void *arg,
> for (j = 0; j < length; j++) {
> unsigned int idx = start + j;
>
> - err = func(name, map_fd, pdef, op, &idx, arg);
> + err = func(name, map_fd, map, op, &idx, arg);
> if (err) {
> pr_debug("ERROR: failed to insert value to %s[%u]\n",
> name, idx);
> @@ -1304,7 +1296,7 @@ bpf_map_config_foreach_key(struct bpf_map *map,
> map_config_func_t func,
> void *arg)
> {
> - int err, map_fd;
> + int err, map_fd, type;
> struct bpf_map_op *op;
> const struct bpf_map_def *def;
> const char *name = bpf_map__name(map);
> @@ -1330,19 +1322,19 @@ bpf_map_config_foreach_key(struct bpf_map *map,
> return map_fd;
> }
>
> + type = bpf_map__type(map);
> list_for_each_entry(op, &priv->ops_list, list) {
> - switch (def->type) {
> + switch (type) {
> case BPF_MAP_TYPE_ARRAY:
> case BPF_MAP_TYPE_PERF_EVENT_ARRAY:
> switch (op->key_type) {
> case BPF_MAP_KEY_ALL:
> err = foreach_key_array_all(func, arg, name,
> - map_fd, def, op);
> + map_fd, map, op);
> break;
> case BPF_MAP_KEY_RANGES:
> err = foreach_key_array_ranges(func, arg, name,
> - map_fd, def,
> - op);
> + map_fd, map, op);
> break;
> default:
> pr_debug("ERROR: keytype for map '%s' invalid\n",
> @@ -1451,7 +1443,7 @@ apply_config_evsel_for_key(const char *name, int map_fd, void *pkey,
>
> static int
> apply_obj_config_map_for_key(const char *name, int map_fd,
> - const struct bpf_map_def *pdef,
> + const struct bpf_map *map,
> struct bpf_map_op *op,
> void *pkey, void *arg __maybe_unused)
> {
> @@ -1460,7 +1452,7 @@ apply_obj_config_map_for_key(const char *name, int map_fd,
> switch (op->op_type) {
> case BPF_MAP_OP_SET_VALUE:
> err = apply_config_value_for_key(map_fd, pkey,
> - pdef->value_size,
> + bpf_map__value_size(map),
> op->v.value);
> break;
> case BPF_MAP_OP_SET_EVSEL:
> diff --git a/tools/perf/util/bpf_map.c b/tools/perf/util/bpf_map.c
> index eb853ca67cf4..c863ae0c5cb5 100644
> --- a/tools/perf/util/bpf_map.c
> +++ b/tools/perf/util/bpf_map.c
> @@ -9,25 +9,25 @@
> #include <stdlib.h>
> #include <unistd.h>
>
> -static bool bpf_map_def__is_per_cpu(const struct bpf_map_def *def)
> +static bool bpf_map__is_per_cpu(enum bpf_map_type type)
> {
> - return def->type == BPF_MAP_TYPE_PERCPU_HASH ||
> - def->type == BPF_MAP_TYPE_PERCPU_ARRAY ||
> - def->type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
> - def->type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE;
> + return type == BPF_MAP_TYPE_PERCPU_HASH ||
> + type == BPF_MAP_TYPE_PERCPU_ARRAY ||
> + type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
> + type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE;
> }
>
> -static void *bpf_map_def__alloc_value(const struct bpf_map_def *def)
> +static void *bpf_map__alloc_value(const struct bpf_map *map)
> {
> - if (bpf_map_def__is_per_cpu(def))
> - return malloc(round_up(def->value_size, 8) * sysconf(_SC_NPROCESSORS_CONF));
> + if (bpf_map__is_per_cpu(bpf_map__type(map)))
> + return malloc(round_up(bpf_map__value_size(map), 8) *
> + sysconf(_SC_NPROCESSORS_CONF));
>
> - return malloc(def->value_size);
> + return malloc(bpf_map__value_size(map));
> }
>
> int bpf_map__fprintf(struct bpf_map *map, FILE *fp)
> {
> - const struct bpf_map_def *def = bpf_map__def(map);
> void *prev_key = NULL, *key, *value;
> int fd = bpf_map__fd(map), err;
> int printed = 0;
> @@ -35,15 +35,15 @@ int bpf_map__fprintf(struct bpf_map *map, FILE *fp)
> if (fd < 0)
> return fd;
>
> - if (IS_ERR(def))
> - return PTR_ERR(def);
> + if (!map)
> + return PTR_ERR(map);
>
> err = -ENOMEM;
> - key = malloc(def->key_size);
> + key = malloc(bpf_map__key_size(map));
> if (key == NULL)
> goto out;
>
> - value = bpf_map_def__alloc_value(def);
> + value = bpf_map__alloc_value(map);
> if (value == NULL)
> goto out_free_key;
>
> --
> 2.30.2
--
- Arnaldo
next prev parent reply other threads:[~2022-01-06 20:25 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-05 23:00 [PATCH bpf-next 0/5] libbpf 1.0: deprecate bpf_map__def() API Christy Lee
2022-01-05 23:00 ` [PATCH bpf-next 1/5] samples/bpf: stop using " Christy Lee
2022-01-05 23:00 ` [PATCH bpf-next 2/5] bpftool: " Christy Lee
2022-01-05 23:00 ` [PATCH bpf-next 3/5] perf: " Christy Lee
2022-01-06 0:20 ` Andrii Nakryiko
2022-01-06 20:24 ` Arnaldo Carvalho de Melo [this message]
2022-01-05 23:00 ` [PATCH bpf-next 4/5] selftests/bpf: " Christy Lee
2022-01-05 23:00 ` [PATCH bpf-next 5/5] libbpf: deprecate " Christy Lee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YddQE5ZIgkarkRNL@kernel.org \
--to=acme@kernel.org \
--cc=andrii@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=christyc.y.lee@gmail.com \
--cc=christylee@fb.com \
--cc=kernel-team@fb.com \
--cc=linux-perf-users@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).