[PATCHv3 0/2] perf/bpf: Replace deprecated code

[PATCHv3 0/2] perf/bpf: Replace deprecated code

[Jiri Olsa]

hi,
the original patchset [1] removed the whole perf functionality
with the hope nobody's using that. But it turned out there's
actually bpf script using prologue functionality, so there
might be users of this.

v3 changes:
  - sending priv related changes, because they can be already
    merged, the rest will need more discussion and work

  - this version gets rid of and adds workaround (and keeps the
    current functionality) for following deprecated libbpf functions:

      bpf_program__set_priv
      bpf_program__priv
      bpf_map__set_priv
      bpf_map__priv

    Basically it implements workarounds suggested by Andrii in [2].

Also available in here:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  bpf/depre

thanks,
jirka


[1] https://lore.kernel.org/linux-perf-users/YgoPxhE3OEEmZqla@krava/T/#t
[2] https://lore.kernel.org/linux-perf-users/YgoPxhE3OEEmZqla@krava/T/#md3ccab9fe70a4583e94603b1a562e369bd67b17d
---
Jiri Olsa (2):
      perf tools: Remove bpf_program__set_priv/bpf_program__priv usage
      perf tools: Remove bpf_map__set_priv/bpf_map__priv usage

 tools/perf/util/bpf-loader.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 141 insertions(+), 23 deletions(-)

[PATCH 1/2] perf tools: Remove bpf_program__set_priv/bpf_program__priv usage

[Jiri Olsa]

Both bpf_program__set_priv/bpf_program__priv are deprecated
and will be eventually removed.

Using hashmap to replace that functionality.

Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/bpf-loader.c | 98 ++++++++++++++++++++++++++++++------
 1 file changed, 82 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index efd9c703b5cc..b9d4278895ec 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -26,6 +26,7 @@
 #include "util.h"
 #include "llvm-utils.h"
 #include "c++/clang-c.h"
+#include "hashmap.h"
 
 #include <internal/xyarray.h>
 
@@ -55,6 +56,7 @@ struct bpf_perf_object {
 };
 
 static LIST_HEAD(bpf_objects_list);
+static struct hashmap *bpf_program_hash;
 
 static struct bpf_perf_object *
 bpf_perf_object__next(struct bpf_perf_object *prev)
@@ -173,6 +175,35 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
 	return obj;
 }
 
+static void
+clear_prog_priv(const struct bpf_program *prog __maybe_unused,
+		void *_priv)
+{
+	struct bpf_prog_priv *priv = _priv;
+
+	cleanup_perf_probe_events(&priv->pev, 1);
+	zfree(&priv->insns_buf);
+	zfree(&priv->type_mapping);
+	zfree(&priv->sys_name);
+	zfree(&priv->evt_name);
+	free(priv);
+}
+
+static void bpf_program_hash_free(void)
+{
+	struct hashmap_entry *cur;
+	size_t bkt;
+
+	if (IS_ERR_OR_NULL(bpf_program_hash))
+		return;
+
+	hashmap__for_each_entry(bpf_program_hash, cur, bkt)
+		clear_prog_priv(cur->key, cur->value);
+
+	hashmap__free(bpf_program_hash);
+	bpf_program_hash = NULL;
+}
+
 void bpf__clear(void)
 {
 	struct bpf_perf_object *perf_obj, *tmp;
@@ -181,20 +212,55 @@ void bpf__clear(void)
 		bpf__unprobe(perf_obj->obj);
 		bpf_perf_object__close(perf_obj);
 	}
+
+	bpf_program_hash_free();
 }
 
-static void
-clear_prog_priv(struct bpf_program *prog __maybe_unused,
-		void *_priv)
+static size_t ptr_hash(const void *__key, void *ctx __maybe_unused)
 {
-	struct bpf_prog_priv *priv = _priv;
+	return (size_t) __key;
+}
 
-	cleanup_perf_probe_events(&priv->pev, 1);
-	zfree(&priv->insns_buf);
-	zfree(&priv->type_mapping);
-	zfree(&priv->sys_name);
-	zfree(&priv->evt_name);
-	free(priv);
+static bool ptr_equal(const void *key1, const void *key2,
+			  void *ctx __maybe_unused)
+{
+	return key1 == key2;
+}
+
+static void *program_priv(const struct bpf_program *prog)
+{
+	void *priv;
+
+	if (IS_ERR_OR_NULL(bpf_program_hash))
+		return NULL;
+	if (!hashmap__find(bpf_program_hash, prog, &priv))
+		return NULL;
+	return priv;
+}
+
+static int program_set_priv(struct bpf_program *prog, void *priv)
+{
+	void *old_priv;
+
+	/*
+	 * Should not happen, we warn about it in the
+	 * caller function - config_bpf_program
+	 */
+	if (IS_ERR(bpf_program_hash))
+		return PTR_ERR(bpf_program_hash);
+
+	if (!bpf_program_hash) {
+		bpf_program_hash = hashmap__new(ptr_hash, ptr_equal, NULL);
+		if (IS_ERR(bpf_program_hash))
+			return PTR_ERR(bpf_program_hash);
+	}
+
+	old_priv = program_priv(prog);
+	if (old_priv) {
+		clear_prog_priv(prog, old_priv);
+		return hashmap__set(bpf_program_hash, prog, priv, NULL, NULL);
+	}
+	return hashmap__add(bpf_program_hash, prog, priv);
 }
 
 static int
@@ -438,7 +504,7 @@ config_bpf_program(struct bpf_program *prog)
 	pr_debug("bpf: config '%s' is ok\n", config_str);
 
 set_priv:
-	err = bpf_program__set_priv(prog, priv, clear_prog_priv);
+	err = program_set_priv(prog, priv);
 	if (err) {
 		pr_debug("Failed to set priv for program '%s'\n", config_str);
 		goto errout;
@@ -479,7 +545,7 @@ preproc_gen_prologue(struct bpf_program *prog, int n,
 		     struct bpf_insn *orig_insns, int orig_insns_cnt,
 		     struct bpf_prog_prep_result *res)
 {
-	struct bpf_prog_priv *priv = bpf_program__priv(prog);
+	struct bpf_prog_priv *priv = program_priv(prog);
 	struct probe_trace_event *tev;
 	struct perf_probe_event *pev;
 	struct bpf_insn *buf;
@@ -630,7 +696,7 @@ static int map_prologue(struct perf_probe_event *pev, int *mapping,
 
 static int hook_load_preprocessor(struct bpf_program *prog)
 {
-	struct bpf_prog_priv *priv = bpf_program__priv(prog);
+	struct bpf_prog_priv *priv = program_priv(prog);
 	struct perf_probe_event *pev;
 	bool need_prologue = false;
 	int err, i;
@@ -706,7 +772,7 @@ int bpf__probe(struct bpf_object *obj)
 		if (err)
 			goto out;
 
-		priv = bpf_program__priv(prog);
+		priv = program_priv(prog);
 		if (IS_ERR_OR_NULL(priv)) {
 			if (!priv)
 				err = -BPF_LOADER_ERRNO__INTERNAL;
@@ -758,7 +824,7 @@ int bpf__unprobe(struct bpf_object *obj)
 	struct bpf_program *prog;
 
 	bpf_object__for_each_program(prog, obj) {
-		struct bpf_prog_priv *priv = bpf_program__priv(prog);
+		struct bpf_prog_priv *priv = program_priv(prog);
 		int i;
 
 		if (IS_ERR_OR_NULL(priv) || priv->is_tp)
@@ -814,7 +880,7 @@ int bpf__foreach_event(struct bpf_object *obj,
 	int err;
 
 	bpf_object__for_each_program(prog, obj) {
-		struct bpf_prog_priv *priv = bpf_program__priv(prog);
+		struct bpf_prog_priv *priv = program_priv(prog);
 		struct probe_trace_event *tev;
 		struct perf_probe_event *pev;
 		int i, fd;
-- 
2.35.1

[PATCH 2/2] perf tools: Remove bpf_map__set_priv/bpf_map__priv usage

[Jiri Olsa]

Both bpf_map__set_priv/bpf_map__priv are deprecated
and will be eventually removed.

Using hashmap to replace that functionality.

Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/bpf-loader.c | 66 ++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index b9d4278895ec..4f6173756a9d 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -27,6 +27,7 @@
 #include "llvm-utils.h"
 #include "c++/clang-c.h"
 #include "hashmap.h"
+#include "asm/bug.h"
 
 #include <internal/xyarray.h>
 
@@ -57,6 +58,7 @@ struct bpf_perf_object {
 
 static LIST_HEAD(bpf_objects_list);
 static struct hashmap *bpf_program_hash;
+static struct hashmap *bpf_map_hash;
 
 static struct bpf_perf_object *
 bpf_perf_object__next(struct bpf_perf_object *prev)
@@ -204,6 +206,8 @@ static void bpf_program_hash_free(void)
 	bpf_program_hash = NULL;
 }
 
+static void bpf_map_hash_free(void);
+
 void bpf__clear(void)
 {
 	struct bpf_perf_object *perf_obj, *tmp;
@@ -214,6 +218,7 @@ void bpf__clear(void)
 	}
 
 	bpf_program_hash_free();
+	bpf_map_hash_free();
 }
 
 static size_t ptr_hash(const void *__key, void *ctx __maybe_unused)
@@ -976,7 +981,7 @@ bpf_map_priv__purge(struct bpf_map_priv *priv)
 }
 
 static void
-bpf_map_priv__clear(struct bpf_map *map __maybe_unused,
+bpf_map_priv__clear(const struct bpf_map *map __maybe_unused,
 		    void *_priv)
 {
 	struct bpf_map_priv *priv = _priv;
@@ -985,6 +990,53 @@ bpf_map_priv__clear(struct bpf_map *map __maybe_unused,
 	free(priv);
 }
 
+static void *map_priv(const struct bpf_map *map)
+{
+	void *priv;
+
+	if (IS_ERR_OR_NULL(bpf_map_hash))
+		return NULL;
+	if (!hashmap__find(bpf_map_hash, map, &priv))
+		return NULL;
+	return priv;
+}
+
+static void bpf_map_hash_free(void)
+{
+	struct hashmap_entry *cur;
+	size_t bkt;
+
+	if (IS_ERR_OR_NULL(bpf_map_hash))
+		return;
+
+	hashmap__for_each_entry(bpf_map_hash, cur, bkt)
+		bpf_map_priv__clear(cur->key, cur->value);
+
+	hashmap__free(bpf_map_hash);
+	bpf_map_hash = NULL;
+}
+
+static int map_set_priv(struct bpf_map *map, void *priv)
+{
+	void *old_priv;
+
+	if (WARN_ON_ONCE(IS_ERR(bpf_map_hash)))
+		return PTR_ERR(bpf_program_hash);
+
+	if (!bpf_map_hash) {
+		bpf_map_hash = hashmap__new(ptr_hash, ptr_equal, NULL);
+		if (IS_ERR(bpf_map_hash))
+			return PTR_ERR(bpf_map_hash);
+	}
+
+	old_priv = map_priv(map);
+	if (old_priv) {
+		bpf_map_priv__clear(map, old_priv);
+		return hashmap__set(bpf_map_hash, map, priv, NULL, NULL);
+	}
+	return hashmap__add(bpf_map_hash, map, priv);
+}
+
 static int
 bpf_map_op_setkey(struct bpf_map_op *op, struct parse_events_term *term)
 {
@@ -1084,7 +1136,7 @@ static int
 bpf_map__add_op(struct bpf_map *map, struct bpf_map_op *op)
 {
 	const char *map_name = bpf_map__name(map);
-	struct bpf_map_priv *priv = bpf_map__priv(map);
+	struct bpf_map_priv *priv = map_priv(map);
 
 	if (IS_ERR(priv)) {
 		pr_debug("Failed to get private from map %s\n", map_name);
@@ -1099,7 +1151,7 @@ bpf_map__add_op(struct bpf_map *map, struct bpf_map_op *op)
 		}
 		INIT_LIST_HEAD(&priv->ops_list);
 
-		if (bpf_map__set_priv(map, priv, bpf_map_priv__clear)) {
+		if (map_set_priv(map, priv)) {
 			free(priv);
 			return -BPF_LOADER_ERRNO__INTERNAL;
 		}
@@ -1440,7 +1492,7 @@ bpf_map_config_foreach_key(struct bpf_map *map,
 	struct bpf_map_op *op;
 	const struct bpf_map_def *def;
 	const char *name = bpf_map__name(map);
-	struct bpf_map_priv *priv = bpf_map__priv(map);
+	struct bpf_map_priv *priv = map_priv(map);
 
 	if (IS_ERR(priv)) {
 		pr_debug("ERROR: failed to get private from map %s\n", name);
@@ -1660,7 +1712,7 @@ struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
 	bool need_init = false;
 
 	bpf__perf_for_each_map_named(map, perf_obj, tmp, name) {
-		struct bpf_map_priv *priv = bpf_map__priv(map);
+		struct bpf_map_priv *priv = map_priv(map);
 
 		if (IS_ERR(priv))
 			return ERR_PTR(-BPF_LOADER_ERRNO__INTERNAL);
@@ -1696,7 +1748,7 @@ struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
 	}
 
 	bpf__perf_for_each_map_named(map, perf_obj, tmp, name) {
-		struct bpf_map_priv *priv = bpf_map__priv(map);
+		struct bpf_map_priv *priv = map_priv(map);
 
 		if (IS_ERR(priv))
 			return ERR_PTR(-BPF_LOADER_ERRNO__INTERNAL);
@@ -1708,7 +1760,7 @@ struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
 			if (!priv)
 				return ERR_PTR(-ENOMEM);
 
-			err = bpf_map__set_priv(map, priv, bpf_map_priv__clear);
+			err = map_set_priv(map, priv);
 			if (err) {
 				bpf_map_priv__clear(map, priv);
 				return ERR_PTR(err);
-- 
2.35.1

Re: [PATCH 2/2] perf tools: Remove bpf_map__set_priv/bpf_map__priv usage

[Andrii Nakryiko]

On Thu, Feb 24, 2022 at 7:53 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Both bpf_map__set_priv/bpf_map__priv are deprecated
> and will be eventually removed.
>
> Using hashmap to replace that functionality.
>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/bpf-loader.c | 66 ++++++++++++++++++++++++++++++++----
>  1 file changed, 59 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index b9d4278895ec..4f6173756a9d 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -27,6 +27,7 @@
>  #include "llvm-utils.h"
>  #include "c++/clang-c.h"
>  #include "hashmap.h"
> +#include "asm/bug.h"
>
>  #include <internal/xyarray.h>
>
> @@ -57,6 +58,7 @@ struct bpf_perf_object {
>
>  static LIST_HEAD(bpf_objects_list);
>  static struct hashmap *bpf_program_hash;
> +static struct hashmap *bpf_map_hash;
>
>  static struct bpf_perf_object *
>  bpf_perf_object__next(struct bpf_perf_object *prev)
> @@ -204,6 +206,8 @@ static void bpf_program_hash_free(void)
>         bpf_program_hash = NULL;
>  }
>
> +static void bpf_map_hash_free(void);
> +
>  void bpf__clear(void)
>  {
>         struct bpf_perf_object *perf_obj, *tmp;
> @@ -214,6 +218,7 @@ void bpf__clear(void)
>         }
>
>         bpf_program_hash_free();
> +       bpf_map_hash_free();
>  }
>
>  static size_t ptr_hash(const void *__key, void *ctx __maybe_unused)
> @@ -976,7 +981,7 @@ bpf_map_priv__purge(struct bpf_map_priv *priv)
>  }
>
>  static void
> -bpf_map_priv__clear(struct bpf_map *map __maybe_unused,
> +bpf_map_priv__clear(const struct bpf_map *map __maybe_unused,
>                     void *_priv)
>  {
>         struct bpf_map_priv *priv = _priv;
> @@ -985,6 +990,53 @@ bpf_map_priv__clear(struct bpf_map *map __maybe_unused,
>         free(priv);
>  }
>
> +static void *map_priv(const struct bpf_map *map)
> +{
> +       void *priv;
> +
> +       if (IS_ERR_OR_NULL(bpf_map_hash))
> +               return NULL;
> +       if (!hashmap__find(bpf_map_hash, map, &priv))
> +               return NULL;
> +       return priv;
> +}
> +
> +static void bpf_map_hash_free(void)
> +{
> +       struct hashmap_entry *cur;
> +       size_t bkt;
> +
> +       if (IS_ERR_OR_NULL(bpf_map_hash))
> +               return;
> +
> +       hashmap__for_each_entry(bpf_map_hash, cur, bkt)
> +               bpf_map_priv__clear(cur->key, cur->value);
> +
> +       hashmap__free(bpf_map_hash);
> +       bpf_map_hash = NULL;
> +}
> +
> +static int map_set_priv(struct bpf_map *map, void *priv)
> +{
> +       void *old_priv;
> +
> +       if (WARN_ON_ONCE(IS_ERR(bpf_map_hash)))
> +               return PTR_ERR(bpf_program_hash);
> +

you didn't warn for this in the previous patch. I have no preference,
just pointing out assymetry.

> +       if (!bpf_map_hash) {
> +               bpf_map_hash = hashmap__new(ptr_hash, ptr_equal, NULL);
> +               if (IS_ERR(bpf_map_hash))
> +                       return PTR_ERR(bpf_map_hash);
> +       }
> +
> +       old_priv = map_priv(map);
> +       if (old_priv) {
> +               bpf_map_priv__clear(map, old_priv);
> +               return hashmap__set(bpf_map_hash, map, priv, NULL, NULL);
> +       }
> +       return hashmap__add(bpf_map_hash, map, priv);
> +}
> +

[...]

Re: [PATCHv3 0/2] perf/bpf: Replace deprecated code

[Andrii Nakryiko]

On Thu, Feb 24, 2022 at 7:52 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> hi,
> the original patchset [1] removed the whole perf functionality
> with the hope nobody's using that. But it turned out there's
> actually bpf script using prologue functionality, so there
> might be users of this.
>
> v3 changes:
>   - sending priv related changes, because they can be already
>     merged, the rest will need more discussion and work
>
>   - this version gets rid of and adds workaround (and keeps the
>     current functionality) for following deprecated libbpf functions:
>
>       bpf_program__set_priv
>       bpf_program__priv
>       bpf_map__set_priv
>       bpf_map__priv
>
>     Basically it implements workarounds suggested by Andrii in [2].
>

LGTM, for the series:

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> Also available in here:
>   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
>   bpf/depre
>
> thanks,
> jirka
>
>
> [1] https://lore.kernel.org/linux-perf-users/YgoPxhE3OEEmZqla@krava/T/#t
> [2] https://lore.kernel.org/linux-perf-users/YgoPxhE3OEEmZqla@krava/T/#md3ccab9fe70a4583e94603b1a562e369bd67b17d
> ---
> Jiri Olsa (2):
>       perf tools: Remove bpf_program__set_priv/bpf_program__priv usage
>       perf tools: Remove bpf_map__set_priv/bpf_map__priv usage
>
>  tools/perf/util/bpf-loader.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 141 insertions(+), 23 deletions(-)

Re: [PATCH 2/2] perf tools: Remove bpf_map__set_priv/bpf_map__priv usage

[Jiri Olsa]

On Mon, Feb 28, 2022 at 05:49:32PM -0800, Andrii Nakryiko wrote:
> On Thu, Feb 24, 2022 at 7:53 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Both bpf_map__set_priv/bpf_map__priv are deprecated
> > and will be eventually removed.
> >
> > Using hashmap to replace that functionality.
> >
> > Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/perf/util/bpf-loader.c | 66 ++++++++++++++++++++++++++++++++----
> >  1 file changed, 59 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> > index b9d4278895ec..4f6173756a9d 100644
> > --- a/tools/perf/util/bpf-loader.c
> > +++ b/tools/perf/util/bpf-loader.c
> > @@ -27,6 +27,7 @@
> >  #include "llvm-utils.h"
> >  #include "c++/clang-c.h"
> >  #include "hashmap.h"
> > +#include "asm/bug.h"
> >
> >  #include <internal/xyarray.h>
> >
> > @@ -57,6 +58,7 @@ struct bpf_perf_object {
> >
> >  static LIST_HEAD(bpf_objects_list);
> >  static struct hashmap *bpf_program_hash;
> > +static struct hashmap *bpf_map_hash;
> >
> >  static struct bpf_perf_object *
> >  bpf_perf_object__next(struct bpf_perf_object *prev)
> > @@ -204,6 +206,8 @@ static void bpf_program_hash_free(void)
> >         bpf_program_hash = NULL;
> >  }
> >
> > +static void bpf_map_hash_free(void);
> > +
> >  void bpf__clear(void)
> >  {
> >         struct bpf_perf_object *perf_obj, *tmp;
> > @@ -214,6 +218,7 @@ void bpf__clear(void)
> >         }
> >
> >         bpf_program_hash_free();
> > +       bpf_map_hash_free();
> >  }
> >
> >  static size_t ptr_hash(const void *__key, void *ctx __maybe_unused)
> > @@ -976,7 +981,7 @@ bpf_map_priv__purge(struct bpf_map_priv *priv)
> >  }
> >
> >  static void
> > -bpf_map_priv__clear(struct bpf_map *map __maybe_unused,
> > +bpf_map_priv__clear(const struct bpf_map *map __maybe_unused,
> >                     void *_priv)
> >  {
> >         struct bpf_map_priv *priv = _priv;
> > @@ -985,6 +990,53 @@ bpf_map_priv__clear(struct bpf_map *map __maybe_unused,
> >         free(priv);
> >  }
> >
> > +static void *map_priv(const struct bpf_map *map)
> > +{
> > +       void *priv;
> > +
> > +       if (IS_ERR_OR_NULL(bpf_map_hash))
> > +               return NULL;
> > +       if (!hashmap__find(bpf_map_hash, map, &priv))
> > +               return NULL;
> > +       return priv;
> > +}
> > +
> > +static void bpf_map_hash_free(void)
> > +{
> > +       struct hashmap_entry *cur;
> > +       size_t bkt;
> > +
> > +       if (IS_ERR_OR_NULL(bpf_map_hash))
> > +               return;
> > +
> > +       hashmap__for_each_entry(bpf_map_hash, cur, bkt)
> > +               bpf_map_priv__clear(cur->key, cur->value);
> > +
> > +       hashmap__free(bpf_map_hash);
> > +       bpf_map_hash = NULL;
> > +}
> > +
> > +static int map_set_priv(struct bpf_map *map, void *priv)
> > +{
> > +       void *old_priv;
> > +
> > +       if (WARN_ON_ONCE(IS_ERR(bpf_map_hash)))
> > +               return PTR_ERR(bpf_program_hash);
> > +
> 
> you didn't warn for this in the previous patch. I have no preference,
> just pointing out assymetry.

there's already message from the caller when program_set_priv fails,
so I don't think we need another one

jirka

> 
> > +       if (!bpf_map_hash) {
> > +               bpf_map_hash = hashmap__new(ptr_hash, ptr_equal, NULL);
> > +               if (IS_ERR(bpf_map_hash))
> > +                       return PTR_ERR(bpf_map_hash);
> > +       }
> > +
> > +       old_priv = map_priv(map);
> > +       if (old_priv) {
> > +               bpf_map_priv__clear(map, old_priv);
> > +               return hashmap__set(bpf_map_hash, map, priv, NULL, NULL);
> > +       }
> > +       return hashmap__add(bpf_map_hash, map, priv);
> > +}
> > +
> 
> [...]

Re: [PATCHv3 0/2] perf/bpf: Replace deprecated code

[Arnaldo Carvalho de Melo]

Em Mon, Feb 28, 2022 at 05:49:55PM -0800, Andrii Nakryiko escreveu:
> On Thu, Feb 24, 2022 at 7:52 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > hi,
> > the original patchset [1] removed the whole perf functionality
> > with the hope nobody's using that. But it turned out there's
> > actually bpf script using prologue functionality, so there
> > might be users of this.
> >
> > v3 changes:
> >   - sending priv related changes, because they can be already
> >     merged, the rest will need more discussion and work
> >
> >   - this version gets rid of and adds workaround (and keeps the
> >     current functionality) for following deprecated libbpf functions:
> >
> >       bpf_program__set_priv
> >       bpf_program__priv
> >       bpf_map__set_priv
> >       bpf_map__priv
> >
> >     Basically it implements workarounds suggested by Andrii in [2].
> >
> 
> LGTM, for the series:
> 
> Acked-by: Andrii Nakryiko <andrii@kernel.org>

Thanks, applied.

- Arnaldo

 
> > Also available in here:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> >   bpf/depre
> >
> > thanks,
> > jirka
> >
> >
> > [1] https://lore.kernel.org/linux-perf-users/YgoPxhE3OEEmZqla@krava/T/#t
> > [2] https://lore.kernel.org/linux-perf-users/YgoPxhE3OEEmZqla@krava/T/#md3ccab9fe70a4583e94603b1a562e369bd67b17d
> > ---
> > Jiri Olsa (2):
> >       perf tools: Remove bpf_program__set_priv/bpf_program__priv usage
> >       perf tools: Remove bpf_map__set_priv/bpf_map__priv usage
> >
> >  tools/perf/util/bpf-loader.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
> >  1 file changed, 141 insertions(+), 23 deletions(-)

-- 

- Arnaldo