[PATCHv2 0/3] perf tools: Fix prologue generation

[PATCHv2 0/3] perf tools: Fix prologue generation

[Jiri Olsa]

hi,
sending change we discussed some time ago [1] to get rid of
some deprecated functions we use in perf prologue code.

Despite the gloomy discussion I think the final code does
not look that bad ;-)

This patchset removes following libbpf functions from perf:
  bpf_program__set_prep
  bpf_program__nth_fd
  struct bpf_prog_prep_result

v2 changes:
  - use fallback section prog handler, so we don't need to
    use section prefix [Andrii]
  - realloc prog->insns array in bpf_program__set_insns [Andrii]
  - squash patch 1 from previous version with
    bpf_program__set_insns change [Daniel]
  - patch 3 already merged [Arnaldo]
  - added more comments

  meanwhile.. perf/core and bpf-next diverged, so:
    - libbpf bpf_program__set_insns change is based on bpf-next/master
    - perf changes do not apply on bpf-next/master so they are based on
      perf/core ... however they can be merged only after we release
      libbpf 0.8.0 with bpf_program__set_insns change, so we don't break
      the dynamic linking
      I'm sending perf changes now just for review, I'll resend them
      once libbpf 0.8.0 is released

thanks,
jirka


[1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/
---
Jiri Olsa (1):
      libbpf: Add bpf_program__set_insns function

 tools/lib/bpf/libbpf.c   | 22 ++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   | 18 ++++++++++++++++++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 41 insertions(+)

Jiri Olsa (2):
      perf tools: Register fallback libbpf section handler
      perf tools: Rework prologue generation code

 tools/perf/util/bpf-loader.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 157 insertions(+), 18 deletions(-)

[PATCHv2 bpf-next 1/3] libbpf: Add bpf_program__set_insns function

[Jiri Olsa]

Adding bpf_program__set_insns that allows to set new instructions
for program.

This is a very advanced libbpf API and users need to know what
they are doing. This should be used from prog_prepare_load_fn
callback only.

We can have changed instructions after calling prog_prepare_load_fn
callback, reloading them.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/libbpf.c   | 22 ++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   | 18 ++++++++++++++++++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 41 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 15117b9a4d1e..7c17ab9f99ca 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6862,6 +6862,8 @@ static int bpf_object_load_prog_instance(struct bpf_object *obj, struct bpf_prog
 				prog->name, err);
 			return err;
 		}
+		insns = prog->insns;
+		insns_cnt = prog->insns_cnt;
 	}
 
 	if (obj->gen_loader) {
@@ -8790,6 +8792,26 @@ size_t bpf_program__insn_cnt(const struct bpf_program *prog)
 	return prog->insns_cnt;
 }
 
+int bpf_program__set_insns(struct bpf_program *prog,
+			   struct bpf_insn *new_insns, size_t new_insn_cnt)
+{
+	struct bpf_insn *insns;
+
+	if (prog->obj->loaded)
+		return -EBUSY;
+
+	insns = libbpf_reallocarray(prog->insns, new_insn_cnt, sizeof(*insns));
+	if (!insns) {
+		pr_warn("prog '%s': failed to realloc prog code\n", prog->name);
+		return -ENOMEM;
+	}
+	memcpy(insns, new_insns, new_insn_cnt * sizeof(*insns));
+
+	prog->insns = insns;
+	prog->insns_cnt = new_insn_cnt;
+	return 0;
+}
+
 int bpf_program__set_prep(struct bpf_program *prog, int nr_instances,
 			  bpf_program_prep_t prep)
 {
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 114b1f6f73a5..5f94459db751 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -323,6 +323,24 @@ struct bpf_insn;
  * different.
  */
 LIBBPF_API const struct bpf_insn *bpf_program__insns(const struct bpf_program *prog);
+
+/**
+ * @brief **bpf_program__set_insns()** can set BPF program's underlying
+ * BPF instructions.
+ *
+ * WARNING: This is a very advanced libbpf API and users need to know
+ * what they are doing. This should be used from prog_prepare_load_fn
+ * callback only.
+ *
+ * @param prog BPF program for which to return instructions
+ * @param new_insns a pointer to an array of BPF instructions
+ * @param new_insn_cnt number of `struct bpf_insn`'s that form
+ * specified BPF program
+ * @return 0, on success; negative error code, otherwise
+ */
+LIBBPF_API int bpf_program__set_insns(struct bpf_program *prog,
+				      struct bpf_insn *new_insns, size_t new_insn_cnt);
+
 /**
  * @brief **bpf_program__insn_cnt()** returns number of `struct bpf_insn`'s
  * that form specified BPF program.
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index b5bc84039407..9eb14ce152dc 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -448,6 +448,7 @@ LIBBPF_0.8.0 {
 		bpf_object__open_subskeleton;
 		bpf_program__attach_kprobe_multi_opts;
 		bpf_program__attach_usdt;
+		bpf_program__set_insns;
 		libbpf_register_prog_handler;
 		libbpf_unregister_prog_handler;
 } LIBBPF_0.7.0;
-- 
2.35.3

[PATCHv2 perf/core 2/3] perf tools: Register fallback libbpf section handler

[Jiri Olsa]

Perf is using section name to declare special kprobe arguments,
which no longer works with current libbpf, that either requires
certain form of the section name or allows to register custom
handler.

Adding perf support to register 'fallback' section handler to take
care of perf kprobe programs. The fallback means that it handles
any section definition besides the ones that libbpf handles.

The handler serves two purposes:
  - allows perf programs to have special arguments in section name
  - allows perf to use pre-load callback where we can attach init
    code (zeroing all argument registers) to each perf program

The second is essential part of new prologue generation code,
that's coming in following patch.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/bpf-loader.c | 47 ++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index f8ad581ea247..2a2c9512c4e8 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -86,6 +86,7 @@ bpf_perf_object__next(struct bpf_perf_object *prev)
 	     (perf_obj) = (tmp), (tmp) = bpf_perf_object__next(tmp))
 
 static bool libbpf_initialized;
+static int libbpf_sec_handler;
 
 static int bpf_perf_object__add(struct bpf_object *obj)
 {
@@ -99,12 +100,58 @@ static int bpf_perf_object__add(struct bpf_object *obj)
 	return perf_obj ? 0 : -ENOMEM;
 }
 
+static struct bpf_insn prologue_init_insn[] = {
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_1, 0),
+	BPF_MOV64_IMM(BPF_REG_2, 0),
+	BPF_MOV64_IMM(BPF_REG_3, 0),
+	BPF_MOV64_IMM(BPF_REG_4, 0),
+	BPF_MOV64_IMM(BPF_REG_5, 0),
+};
+
+static int libbpf_prog_prepare_load_fn(struct bpf_program *prog,
+				       struct bpf_prog_load_opts *opts __maybe_unused,
+				       long cookie __maybe_unused)
+{
+	size_t init_size_cnt = ARRAY_SIZE(prologue_init_insn);
+	size_t orig_insn_cnt, insn_cnt, init_size, orig_size;
+	const struct bpf_insn *orig_insn;
+	struct bpf_insn *insn;
+
+	/* prepend initialization code to program instructions */
+	orig_insn = bpf_program__insns(prog);
+	orig_insn_cnt = bpf_program__insn_cnt(prog);
+	init_size = init_size_cnt * sizeof(*insn);
+	orig_size = orig_insn_cnt * sizeof(*insn);
+
+	insn_cnt = orig_insn_cnt + init_size_cnt;
+	insn = malloc(insn_cnt * sizeof(*insn));
+	if (!insn)
+		return -ENOMEM;
+
+	memcpy(insn, prologue_init_insn, init_size);
+	memcpy((char *) insn + init_size, orig_insn, orig_size);
+	bpf_program__set_insns(prog, insn, insn_cnt);
+	return 0;
+}
+
 static int libbpf_init(void)
 {
+	LIBBPF_OPTS(libbpf_prog_handler_opts, handler_opts,
+		.prog_prepare_load_fn = libbpf_prog_prepare_load_fn,
+	);
+
 	if (libbpf_initialized)
 		return 0;
 
 	libbpf_set_print(libbpf_perf_print);
+	libbpf_sec_handler = libbpf_register_prog_handler(NULL, BPF_PROG_TYPE_KPROBE,
+							  0, &handler_opts);
+	if (libbpf_sec_handler < 0) {
+		pr_debug("bpf: failed to register libbpf section handler: %d\n",
+			 libbpf_sec_handler);
+		return -BPF_LOADER_ERRNO__INTERNAL;
+	}
 	libbpf_initialized = true;
 	return 0;
 }
-- 
2.35.3

Re: [PATCHv2 perf/core 2/3] perf tools: Register fallback libbpf section handler

[Andrii Nakryiko]

On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Perf is using section name to declare special kprobe arguments,
> which no longer works with current libbpf, that either requires
> certain form of the section name or allows to register custom
> handler.
>
> Adding perf support to register 'fallback' section handler to take
> care of perf kprobe programs. The fallback means that it handles
> any section definition besides the ones that libbpf handles.
>
> The handler serves two purposes:
>   - allows perf programs to have special arguments in section name
>   - allows perf to use pre-load callback where we can attach init
>     code (zeroing all argument registers) to each perf program
>
> The second is essential part of new prologue generation code,
> that's coming in following patch.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/bpf-loader.c | 47 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index f8ad581ea247..2a2c9512c4e8 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -86,6 +86,7 @@ bpf_perf_object__next(struct bpf_perf_object *prev)
>              (perf_obj) = (tmp), (tmp) = bpf_perf_object__next(tmp))
>
>  static bool libbpf_initialized;
> +static int libbpf_sec_handler;
>
>  static int bpf_perf_object__add(struct bpf_object *obj)
>  {
> @@ -99,12 +100,58 @@ static int bpf_perf_object__add(struct bpf_object *obj)
>         return perf_obj ? 0 : -ENOMEM;
>  }
>
> +static struct bpf_insn prologue_init_insn[] = {
> +       BPF_MOV64_IMM(BPF_REG_0, 0),
> +       BPF_MOV64_IMM(BPF_REG_1, 0),

R0 should be initialized before exit anyway. R1 contains context, so
doesn't need initialization, so I think you only need R2-R5?

> +       BPF_MOV64_IMM(BPF_REG_2, 0),
> +       BPF_MOV64_IMM(BPF_REG_3, 0),
> +       BPF_MOV64_IMM(BPF_REG_4, 0),
> +       BPF_MOV64_IMM(BPF_REG_5, 0),
> +};
> +
> +static int libbpf_prog_prepare_load_fn(struct bpf_program *prog,
> +                                      struct bpf_prog_load_opts *opts __maybe_unused,
> +                                      long cookie __maybe_unused)
> +{
> +       size_t init_size_cnt = ARRAY_SIZE(prologue_init_insn);
> +       size_t orig_insn_cnt, insn_cnt, init_size, orig_size;
> +       const struct bpf_insn *orig_insn;
> +       struct bpf_insn *insn;
> +
> +       /* prepend initialization code to program instructions */
> +       orig_insn = bpf_program__insns(prog);
> +       orig_insn_cnt = bpf_program__insn_cnt(prog);
> +       init_size = init_size_cnt * sizeof(*insn);
> +       orig_size = orig_insn_cnt * sizeof(*insn);
> +
> +       insn_cnt = orig_insn_cnt + init_size_cnt;
> +       insn = malloc(insn_cnt * sizeof(*insn));
> +       if (!insn)
> +               return -ENOMEM;
> +
> +       memcpy(insn, prologue_init_insn, init_size);
> +       memcpy((char *) insn + init_size, orig_insn, orig_size);
> +       bpf_program__set_insns(prog, insn, insn_cnt);
> +       return 0;
> +}
> +
>  static int libbpf_init(void)
>  {
> +       LIBBPF_OPTS(libbpf_prog_handler_opts, handler_opts,
> +               .prog_prepare_load_fn = libbpf_prog_prepare_load_fn,
> +       );
> +
>         if (libbpf_initialized)
>                 return 0;
>
>         libbpf_set_print(libbpf_perf_print);
> +       libbpf_sec_handler = libbpf_register_prog_handler(NULL, BPF_PROG_TYPE_KPROBE,
> +                                                         0, &handler_opts);
> +       if (libbpf_sec_handler < 0) {
> +               pr_debug("bpf: failed to register libbpf section handler: %d\n",
> +                        libbpf_sec_handler);
> +               return -BPF_LOADER_ERRNO__INTERNAL;
> +       }
>         libbpf_initialized = true;
>         return 0;
>  }
> --
> 2.35.3
>

Re: [PATCHv2 perf/core 2/3] perf tools: Register fallback libbpf section handler

[Jiri Olsa]

On Tue, May 10, 2022 at 04:45:01PM -0700, Andrii Nakryiko wrote:
> On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Perf is using section name to declare special kprobe arguments,
> > which no longer works with current libbpf, that either requires
> > certain form of the section name or allows to register custom
> > handler.
> >
> > Adding perf support to register 'fallback' section handler to take
> > care of perf kprobe programs. The fallback means that it handles
> > any section definition besides the ones that libbpf handles.
> >
> > The handler serves two purposes:
> >   - allows perf programs to have special arguments in section name
> >   - allows perf to use pre-load callback where we can attach init
> >     code (zeroing all argument registers) to each perf program
> >
> > The second is essential part of new prologue generation code,
> > that's coming in following patch.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/perf/util/bpf-loader.c | 47 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> > index f8ad581ea247..2a2c9512c4e8 100644
> > --- a/tools/perf/util/bpf-loader.c
> > +++ b/tools/perf/util/bpf-loader.c
> > @@ -86,6 +86,7 @@ bpf_perf_object__next(struct bpf_perf_object *prev)
> >              (perf_obj) = (tmp), (tmp) = bpf_perf_object__next(tmp))
> >
> >  static bool libbpf_initialized;
> > +static int libbpf_sec_handler;
> >
> >  static int bpf_perf_object__add(struct bpf_object *obj)
> >  {
> > @@ -99,12 +100,58 @@ static int bpf_perf_object__add(struct bpf_object *obj)
> >         return perf_obj ? 0 : -ENOMEM;
> >  }
> >
> > +static struct bpf_insn prologue_init_insn[] = {
> > +       BPF_MOV64_IMM(BPF_REG_0, 0),
> > +       BPF_MOV64_IMM(BPF_REG_1, 0),
> 
> R0 should be initialized before exit anyway. R1 contains context, so
> doesn't need initialization, so I think you only need R2-R5?

ah right, I'll remove that

thanks,
jirka

> 
> > +       BPF_MOV64_IMM(BPF_REG_2, 0),
> > +       BPF_MOV64_IMM(BPF_REG_3, 0),
> > +       BPF_MOV64_IMM(BPF_REG_4, 0),
> > +       BPF_MOV64_IMM(BPF_REG_5, 0),
> > +};
> > +
> > +static int libbpf_prog_prepare_load_fn(struct bpf_program *prog,
> > +                                      struct bpf_prog_load_opts *opts __maybe_unused,
> > +                                      long cookie __maybe_unused)
> > +{
> > +       size_t init_size_cnt = ARRAY_SIZE(prologue_init_insn);
> > +       size_t orig_insn_cnt, insn_cnt, init_size, orig_size;
> > +       const struct bpf_insn *orig_insn;
> > +       struct bpf_insn *insn;
> > +
> > +       /* prepend initialization code to program instructions */
> > +       orig_insn = bpf_program__insns(prog);
> > +       orig_insn_cnt = bpf_program__insn_cnt(prog);
> > +       init_size = init_size_cnt * sizeof(*insn);
> > +       orig_size = orig_insn_cnt * sizeof(*insn);
> > +
> > +       insn_cnt = orig_insn_cnt + init_size_cnt;
> > +       insn = malloc(insn_cnt * sizeof(*insn));
> > +       if (!insn)
> > +               return -ENOMEM;
> > +
> > +       memcpy(insn, prologue_init_insn, init_size);
> > +       memcpy((char *) insn + init_size, orig_insn, orig_size);
> > +       bpf_program__set_insns(prog, insn, insn_cnt);
> > +       return 0;
> > +}
> > +
> >  static int libbpf_init(void)
> >  {
> > +       LIBBPF_OPTS(libbpf_prog_handler_opts, handler_opts,
> > +               .prog_prepare_load_fn = libbpf_prog_prepare_load_fn,
> > +       );
> > +
> >         if (libbpf_initialized)
> >                 return 0;
> >
> >         libbpf_set_print(libbpf_perf_print);
> > +       libbpf_sec_handler = libbpf_register_prog_handler(NULL, BPF_PROG_TYPE_KPROBE,
> > +                                                         0, &handler_opts);
> > +       if (libbpf_sec_handler < 0) {
> > +               pr_debug("bpf: failed to register libbpf section handler: %d\n",
> > +                        libbpf_sec_handler);
> > +               return -BPF_LOADER_ERRNO__INTERNAL;
> > +       }
> >         libbpf_initialized = true;
> >         return 0;
> >  }
> > --
> > 2.35.3
> >

[PATCHv2 perf/core 3/3] perf tools: Rework prologue generation code

[Jiri Olsa]

Some functions we use for bpf prologue generation are going to be
deprecated. This change reworks current code not to use them.

We need to replace following functions/struct:
   bpf_program__set_prep
   bpf_program__nth_fd
   struct bpf_prog_prep_result

Currently we use bpf_program__set_prep to hook perf callback before
program is loaded and provide new instructions with the prologue.

We replace this function/ality by taking instructions for specific
program, attaching prologue to them and load such new ebpf programs
with prologue using separate bpf_prog_load calls (outside libbpf
load machinery).

Before we can take and use program instructions, we need libbpf to
actually load it. This way we get the final shape of its instructions
with all relocations and verifier adjustments).

There's one glitch though.. perf kprobe program already assumes
generated prologue code with proper values in argument registers,
so loading such program directly will fail in the verifier.

That's where the fallback pre-load handler fits in and prepends
the initialization code to the program. Once such program is loaded
we take its instructions, cut off the initialization code and prepend
the prologue.

I know.. sorry ;-)

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

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 2a2c9512c4e8..2ed977a7b2c4 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -9,6 +9,7 @@
 #include <linux/bpf.h>
 #include <bpf/libbpf.h>
 #include <bpf/bpf.h>
+#include <linux/filter.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/string.h>
@@ -49,6 +50,7 @@ struct bpf_prog_priv {
 	struct bpf_insn *insns_buf;
 	int nr_types;
 	int *type_mapping;
+	int *proglogue_fds;
 };
 
 struct bpf_perf_object {
@@ -56,6 +58,11 @@ struct bpf_perf_object {
 	struct bpf_object *obj;
 };
 
+struct bpf_preproc_result {
+	struct bpf_insn *new_insn_ptr;
+	int new_insn_cnt;
+};
+
 static LIST_HEAD(bpf_objects_list);
 static struct hashmap *bpf_program_hash;
 static struct hashmap *bpf_map_hash;
@@ -235,14 +242,31 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
 	return obj;
 }
 
+static void close_prologue_programs(struct bpf_prog_priv *priv)
+{
+	struct perf_probe_event *pev;
+	int i, fd;
+
+	if (!priv->need_prologue)
+		return;
+	pev = &priv->pev;
+	for (i = 0; i < pev->ntevs; i++) {
+		fd = priv->proglogue_fds[i];
+		if (fd != -1)
+			close(fd);
+	}
+}
+
 static void
 clear_prog_priv(const struct bpf_program *prog __maybe_unused,
 		void *_priv)
 {
 	struct bpf_prog_priv *priv = _priv;
 
+	close_prologue_programs(priv);
 	cleanup_perf_probe_events(&priv->pev, 1);
 	zfree(&priv->insns_buf);
+	zfree(&priv->proglogue_fds);
 	zfree(&priv->type_mapping);
 	zfree(&priv->sys_name);
 	zfree(&priv->evt_name);
@@ -605,8 +629,8 @@ static int bpf__prepare_probe(void)
 
 static int
 preproc_gen_prologue(struct bpf_program *prog, int n,
-		     struct bpf_insn *orig_insns, int orig_insns_cnt,
-		     struct bpf_prog_prep_result *res)
+		     const struct bpf_insn *orig_insns, int orig_insns_cnt,
+		     struct bpf_preproc_result *res)
 {
 	struct bpf_prog_priv *priv = program_priv(prog);
 	struct probe_trace_event *tev;
@@ -654,7 +678,6 @@ preproc_gen_prologue(struct bpf_program *prog, int n,
 
 	res->new_insn_ptr = buf;
 	res->new_insn_cnt = prologue_cnt + orig_insns_cnt;
-	res->pfd = NULL;
 	return 0;
 
 errout:
@@ -762,7 +785,7 @@ static int hook_load_preprocessor(struct bpf_program *prog)
 	struct bpf_prog_priv *priv = program_priv(prog);
 	struct perf_probe_event *pev;
 	bool need_prologue = false;
-	int err, i;
+	int i;
 
 	if (IS_ERR_OR_NULL(priv)) {
 		pr_debug("Internal error when hook preprocessor\n");
@@ -800,6 +823,13 @@ static int hook_load_preprocessor(struct bpf_program *prog)
 		return -ENOMEM;
 	}
 
+	priv->proglogue_fds = malloc(sizeof(int) * pev->ntevs);
+	if (!priv->proglogue_fds) {
+		pr_debug("Not enough memory: alloc prologue fds failed\n");
+		return -ENOMEM;
+	}
+	memset(priv->proglogue_fds, -1, sizeof(int) * pev->ntevs);
+
 	priv->type_mapping = malloc(sizeof(int) * pev->ntevs);
 	if (!priv->type_mapping) {
 		pr_debug("Not enough memory: alloc type_mapping failed\n");
@@ -808,13 +838,7 @@ static int hook_load_preprocessor(struct bpf_program *prog)
 	memset(priv->type_mapping, -1,
 	       sizeof(int) * pev->ntevs);
 
-	err = map_prologue(pev, priv->type_mapping, &priv->nr_types);
-	if (err)
-		return err;
-
-	err = bpf_program__set_prep(prog, priv->nr_types,
-				    preproc_gen_prologue);
-	return err;
+	return map_prologue(pev, priv->type_mapping, &priv->nr_types);
 }
 
 int bpf__probe(struct bpf_object *obj)
@@ -921,6 +945,77 @@ int bpf__unprobe(struct bpf_object *obj)
 	return ret;
 }
 
+static int bpf_object__load_prologue(struct bpf_object *obj)
+{
+	int init_cnt = ARRAY_SIZE(prologue_init_insn);
+	const struct bpf_insn *orig_insns;
+	struct bpf_preproc_result res;
+	struct perf_probe_event *pev;
+	struct bpf_program *prog;
+	int orig_insns_cnt;
+
+	bpf_object__for_each_program(prog, obj) {
+		struct bpf_prog_priv *priv = program_priv(prog);
+		int err, i, fd;
+
+		if (IS_ERR_OR_NULL(priv)) {
+			pr_debug("bpf: failed to get private field\n");
+			return -BPF_LOADER_ERRNO__INTERNAL;
+		}
+
+		if (!priv->need_prologue)
+			continue;
+
+		/*
+		 * For each program that needs prologue we do following:
+		 *
+		 * - take its current instructions and use them
+		 *   to generate the new code with prologue
+		 * - load new instructions with bpf_prog_load
+		 *   and keep the fd in proglogue_fds
+		 * - new fd will be used in bpf__foreach_event
+		 *   to connect this program with perf evsel
+		 */
+		orig_insns = bpf_program__insns(prog);
+		orig_insns_cnt = bpf_program__insn_cnt(prog);
+
+		pev = &priv->pev;
+		for (i = 0; i < pev->ntevs; i++) {
+			/*
+			 * Skipping artificall prologue_init_insn instructions
+			 * (init_cnt), so the prologue can be generated instead
+			 * of them.
+			 */
+			err = preproc_gen_prologue(prog, i,
+						   orig_insns + init_cnt,
+						   orig_insns_cnt - init_cnt,
+						   &res);
+			if (err)
+				return err;
+
+			fd = bpf_prog_load(bpf_program__get_type(prog),
+					   bpf_program__name(prog), "GPL",
+					   res.new_insn_ptr,
+					   res.new_insn_cnt, NULL);
+			if (fd < 0) {
+				char bf[128];
+
+				libbpf_strerror(-errno, bf, sizeof(bf));
+				pr_debug("bpf: load objects with prologue failed: err=%d: (%s)\n",
+					 -errno, bf);
+				return -errno;
+			}
+			priv->proglogue_fds[i] = fd;
+		}
+		/*
+		 * We no longer need the original program,
+		 * we can unload it.
+		 */
+		bpf_program__unload(prog);
+	}
+	return 0;
+}
+
 int bpf__load(struct bpf_object *obj)
 {
 	int err;
@@ -932,7 +1027,7 @@ int bpf__load(struct bpf_object *obj)
 		pr_debug("bpf: load objects failed: err=%d: (%s)\n", err, bf);
 		return err;
 	}
-	return 0;
+	return bpf_object__load_prologue(obj);
 }
 
 int bpf__foreach_event(struct bpf_object *obj,
@@ -967,13 +1062,10 @@ int bpf__foreach_event(struct bpf_object *obj,
 		for (i = 0; i < pev->ntevs; i++) {
 			tev = &pev->tevs[i];
 
-			if (priv->need_prologue) {
-				int type = priv->type_mapping[i];
-
-				fd = bpf_program__nth_fd(prog, type);
-			} else {
+			if (priv->need_prologue)
+				fd = priv->proglogue_fds[i];
+			else
 				fd = bpf_program__fd(prog);
-			}
 
 			if (fd < 0) {
 				pr_debug("bpf: failed to get file descriptor\n");
-- 
2.35.3

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Andrii Nakryiko]

On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> hi,
> sending change we discussed some time ago [1] to get rid of
> some deprecated functions we use in perf prologue code.
>
> Despite the gloomy discussion I think the final code does
> not look that bad ;-)
>
> This patchset removes following libbpf functions from perf:
>   bpf_program__set_prep
>   bpf_program__nth_fd
>   struct bpf_prog_prep_result
>
> v2 changes:
>   - use fallback section prog handler, so we don't need to
>     use section prefix [Andrii]
>   - realloc prog->insns array in bpf_program__set_insns [Andrii]
>   - squash patch 1 from previous version with
>     bpf_program__set_insns change [Daniel]
>   - patch 3 already merged [Arnaldo]
>   - added more comments
>
>   meanwhile.. perf/core and bpf-next diverged, so:
>     - libbpf bpf_program__set_insns change is based on bpf-next/master
>     - perf changes do not apply on bpf-next/master so they are based on
>       perf/core ... however they can be merged only after we release
>       libbpf 0.8.0 with bpf_program__set_insns change, so we don't break
>       the dynamic linking
>       I'm sending perf changes now just for review, I'll resend them
>       once libbpf 0.8.0 is released
>
> thanks,
> jirka
>
>
> [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/
> ---
> Jiri Olsa (1):
>       libbpf: Add bpf_program__set_insns function
>

The first patch looks good to me. The rest I can't really review and
test properly, so I'll leave it up to Arnaldo.

Arnaldo, how do we coordinate these patches? Should they go through
bpf-next (after you Ack them) or you want them in your tree?

I'd like to get the bpf_program__set_insns() patch into bpf-next so
that I can do libbpf v0.8 release, having it in a separate tree is
extremely inconvenient. Please let me know how you think we should
proceed?

>  tools/lib/bpf/libbpf.c   | 22 ++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   | 18 ++++++++++++++++++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 41 insertions(+)
>
> Jiri Olsa (2):
>       perf tools: Register fallback libbpf section handler
>       perf tools: Rework prologue generation code
>
>  tools/perf/util/bpf-loader.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 157 insertions(+), 18 deletions(-)

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Jiri Olsa]

On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote:
> On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > hi,
> > sending change we discussed some time ago [1] to get rid of
> > some deprecated functions we use in perf prologue code.
> >
> > Despite the gloomy discussion I think the final code does
> > not look that bad ;-)
> >
> > This patchset removes following libbpf functions from perf:
> >   bpf_program__set_prep
> >   bpf_program__nth_fd
> >   struct bpf_prog_prep_result
> >
> > v2 changes:
> >   - use fallback section prog handler, so we don't need to
> >     use section prefix [Andrii]
> >   - realloc prog->insns array in bpf_program__set_insns [Andrii]
> >   - squash patch 1 from previous version with
> >     bpf_program__set_insns change [Daniel]
> >   - patch 3 already merged [Arnaldo]
> >   - added more comments
> >
> >   meanwhile.. perf/core and bpf-next diverged, so:
> >     - libbpf bpf_program__set_insns change is based on bpf-next/master
> >     - perf changes do not apply on bpf-next/master so they are based on
> >       perf/core ... however they can be merged only after we release
> >       libbpf 0.8.0 with bpf_program__set_insns change, so we don't break
> >       the dynamic linking
> >       I'm sending perf changes now just for review, I'll resend them
> >       once libbpf 0.8.0 is released
> >
> > thanks,
> > jirka
> >
> >
> > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/
> > ---
> > Jiri Olsa (1):
> >       libbpf: Add bpf_program__set_insns function
> >
> 
> The first patch looks good to me. The rest I can't really review and
> test properly, so I'll leave it up to Arnaldo.
> 
> Arnaldo, how do we coordinate these patches? Should they go through
> bpf-next (after you Ack them) or you want them in your tree?
> 
> I'd like to get the bpf_program__set_insns() patch into bpf-next so
> that I can do libbpf v0.8 release, having it in a separate tree is
> extremely inconvenient. Please let me know how you think we should
> proceed?

we need to wait with perf changes after the libbpf is merged and
libbpf 0.8.0 is released.. so we don't break dynamic linking for
perf

at the moment please just take libbpf change and I'll resend the
perf change later if needed

thanks,
jirka

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Arnaldo Carvalho de Melo]

Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu:
> On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote:
> > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > hi,
> > > sending change we discussed some time ago [1] to get rid of
> > > some deprecated functions we use in perf prologue code.
> > >
> > > Despite the gloomy discussion I think the final code does
> > > not look that bad ;-)
> > >
> > > This patchset removes following libbpf functions from perf:
> > >   bpf_program__set_prep
> > >   bpf_program__nth_fd
> > >   struct bpf_prog_prep_result
> > >
> > > v2 changes:
> > >   - use fallback section prog handler, so we don't need to
> > >     use section prefix [Andrii]
> > >   - realloc prog->insns array in bpf_program__set_insns [Andrii]
> > >   - squash patch 1 from previous version with
> > >     bpf_program__set_insns change [Daniel]
> > >   - patch 3 already merged [Arnaldo]
> > >   - added more comments
> > >
> > >   meanwhile.. perf/core and bpf-next diverged, so:
> > >     - libbpf bpf_program__set_insns change is based on bpf-next/master
> > >     - perf changes do not apply on bpf-next/master so they are based on
> > >       perf/core ... however they can be merged only after we release
> > >       libbpf 0.8.0 with bpf_program__set_insns change, so we don't break
> > >       the dynamic linking
> > >       I'm sending perf changes now just for review, I'll resend them
> > >       once libbpf 0.8.0 is released
> > >
> > > thanks,
> > > jirka
> > >
> > >
> > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/
> > > ---
> > > Jiri Olsa (1):
> > >       libbpf: Add bpf_program__set_insns function
> > >
> > 
> > The first patch looks good to me. The rest I can't really review and
> > test properly, so I'll leave it up to Arnaldo.
> > 
> > Arnaldo, how do we coordinate these patches? Should they go through
> > bpf-next (after you Ack them) or you want them in your tree?
> > 
> > I'd like to get the bpf_program__set_insns() patch into bpf-next so
> > that I can do libbpf v0.8 release, having it in a separate tree is
> > extremely inconvenient. Please let me know how you think we should
> > proceed?
> 
> we need to wait with perf changes after the libbpf is merged and
> libbpf 0.8.0 is released.. so we don't break dynamic linking for
> perf
> 
> at the moment please just take libbpf change and I'll resend the
> perf change later if needed

Ok.

- Arnaldo

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Andrii Nakryiko]

On Wed, May 11, 2022 at 11:22 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu:
> > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote:
> > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > >
> > > > hi,
> > > > sending change we discussed some time ago [1] to get rid of
> > > > some deprecated functions we use in perf prologue code.
> > > >
> > > > Despite the gloomy discussion I think the final code does
> > > > not look that bad ;-)
> > > >
> > > > This patchset removes following libbpf functions from perf:
> > > >   bpf_program__set_prep
> > > >   bpf_program__nth_fd
> > > >   struct bpf_prog_prep_result
> > > >
> > > > v2 changes:
> > > >   - use fallback section prog handler, so we don't need to
> > > >     use section prefix [Andrii]
> > > >   - realloc prog->insns array in bpf_program__set_insns [Andrii]
> > > >   - squash patch 1 from previous version with
> > > >     bpf_program__set_insns change [Daniel]
> > > >   - patch 3 already merged [Arnaldo]
> > > >   - added more comments
> > > >
> > > >   meanwhile.. perf/core and bpf-next diverged, so:
> > > >     - libbpf bpf_program__set_insns change is based on bpf-next/master
> > > >     - perf changes do not apply on bpf-next/master so they are based on
> > > >       perf/core ... however they can be merged only after we release
> > > >       libbpf 0.8.0 with bpf_program__set_insns change, so we don't break
> > > >       the dynamic linking
> > > >       I'm sending perf changes now just for review, I'll resend them
> > > >       once libbpf 0.8.0 is released
> > > >
> > > > thanks,
> > > > jirka
> > > >
> > > >
> > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/
> > > > ---
> > > > Jiri Olsa (1):
> > > >       libbpf: Add bpf_program__set_insns function
> > > >
> > >
> > > The first patch looks good to me. The rest I can't really review and
> > > test properly, so I'll leave it up to Arnaldo.
> > >
> > > Arnaldo, how do we coordinate these patches? Should they go through
> > > bpf-next (after you Ack them) or you want them in your tree?
> > >
> > > I'd like to get the bpf_program__set_insns() patch into bpf-next so
> > > that I can do libbpf v0.8 release, having it in a separate tree is
> > > extremely inconvenient. Please let me know how you think we should
> > > proceed?
> >
> > we need to wait with perf changes after the libbpf is merged and
> > libbpf 0.8.0 is released.. so we don't break dynamic linking for
> > perf
> >
> > at the moment please just take libbpf change and I'll resend the
> > perf change later if needed
>
> Ok.
>

Jiri, libbpf v0.8 is out, can you please re-send your perf patches?


> - Arnaldo

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Ian Rogers]

On Tue, May 17, 2022 at 3:03 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, May 11, 2022 at 11:22 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu:
> > > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote:
> > > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > >
> > > > > hi,
> > > > > sending change we discussed some time ago [1] to get rid of
> > > > > some deprecated functions we use in perf prologue code.
> > > > >
> > > > > Despite the gloomy discussion I think the final code does
> > > > > not look that bad ;-)
> > > > >
> > > > > This patchset removes following libbpf functions from perf:
> > > > >   bpf_program__set_prep
> > > > >   bpf_program__nth_fd
> > > > >   struct bpf_prog_prep_result
> > > > >
> > > > > v2 changes:
> > > > >   - use fallback section prog handler, so we don't need to
> > > > >     use section prefix [Andrii]
> > > > >   - realloc prog->insns array in bpf_program__set_insns [Andrii]
> > > > >   - squash patch 1 from previous version with
> > > > >     bpf_program__set_insns change [Daniel]
> > > > >   - patch 3 already merged [Arnaldo]
> > > > >   - added more comments
> > > > >
> > > > >   meanwhile.. perf/core and bpf-next diverged, so:
> > > > >     - libbpf bpf_program__set_insns change is based on bpf-next/master
> > > > >     - perf changes do not apply on bpf-next/master so they are based on
> > > > >       perf/core ... however they can be merged only after we release
> > > > >       libbpf 0.8.0 with bpf_program__set_insns change, so we don't break
> > > > >       the dynamic linking
> > > > >       I'm sending perf changes now just for review, I'll resend them
> > > > >       once libbpf 0.8.0 is released
> > > > >
> > > > > thanks,
> > > > > jirka
> > > > >
> > > > >
> > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/
> > > > > ---
> > > > > Jiri Olsa (1):
> > > > >       libbpf: Add bpf_program__set_insns function
> > > > >
> > > >
> > > > The first patch looks good to me. The rest I can't really review and
> > > > test properly, so I'll leave it up to Arnaldo.
> > > >
> > > > Arnaldo, how do we coordinate these patches? Should they go through
> > > > bpf-next (after you Ack them) or you want them in your tree?
> > > >
> > > > I'd like to get the bpf_program__set_insns() patch into bpf-next so
> > > > that I can do libbpf v0.8 release, having it in a separate tree is
> > > > extremely inconvenient. Please let me know how you think we should
> > > > proceed?
> > >
> > > we need to wait with perf changes after the libbpf is merged and
> > > libbpf 0.8.0 is released.. so we don't break dynamic linking for
> > > perf
> > >
> > > at the moment please just take libbpf change and I'll resend the
> > > perf change later if needed
> >
> > Ok.
> >
>
> Jiri, libbpf v0.8 is out, can you please re-send your perf patches?

We still haven't done as Andrii suggested in:
https://lore.kernel.org/lkml/CAEf4BzbbOHQZUAe6iWaehKCPQAf3VC=hq657buqe2_yRKxaK-A@mail.gmail.com/
so for LIBBPF_DYNAMIC I believe we're compiling against the header
files in tools/lib rather than the installed libbpf's header files.
This may have some unexpected consequences.

Thanks,
Ian

>
> > - Arnaldo

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Jiri Olsa]

On Tue, May 17, 2022 at 09:45:56PM -0700, Ian Rogers wrote:
> On Tue, May 17, 2022 at 3:03 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, May 11, 2022 at 11:22 AM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> > >
> > > Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu:
> > > > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote:
> > > > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > > >
> > > > > > hi,
> > > > > > sending change we discussed some time ago [1] to get rid of
> > > > > > some deprecated functions we use in perf prologue code.
> > > > > >
> > > > > > Despite the gloomy discussion I think the final code does
> > > > > > not look that bad ;-)
> > > > > >
> > > > > > This patchset removes following libbpf functions from perf:
> > > > > >   bpf_program__set_prep
> > > > > >   bpf_program__nth_fd
> > > > > >   struct bpf_prog_prep_result
> > > > > >
> > > > > > v2 changes:
> > > > > >   - use fallback section prog handler, so we don't need to
> > > > > >     use section prefix [Andrii]
> > > > > >   - realloc prog->insns array in bpf_program__set_insns [Andrii]
> > > > > >   - squash patch 1 from previous version with
> > > > > >     bpf_program__set_insns change [Daniel]
> > > > > >   - patch 3 already merged [Arnaldo]
> > > > > >   - added more comments
> > > > > >
> > > > > >   meanwhile.. perf/core and bpf-next diverged, so:
> > > > > >     - libbpf bpf_program__set_insns change is based on bpf-next/master
> > > > > >     - perf changes do not apply on bpf-next/master so they are based on
> > > > > >       perf/core ... however they can be merged only after we release
> > > > > >       libbpf 0.8.0 with bpf_program__set_insns change, so we don't break
> > > > > >       the dynamic linking
> > > > > >       I'm sending perf changes now just for review, I'll resend them
> > > > > >       once libbpf 0.8.0 is released
> > > > > >
> > > > > > thanks,
> > > > > > jirka
> > > > > >
> > > > > >
> > > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/
> > > > > > ---
> > > > > > Jiri Olsa (1):
> > > > > >       libbpf: Add bpf_program__set_insns function
> > > > > >
> > > > >
> > > > > The first patch looks good to me. The rest I can't really review and
> > > > > test properly, so I'll leave it up to Arnaldo.
> > > > >
> > > > > Arnaldo, how do we coordinate these patches? Should they go through
> > > > > bpf-next (after you Ack them) or you want them in your tree?
> > > > >
> > > > > I'd like to get the bpf_program__set_insns() patch into bpf-next so
> > > > > that I can do libbpf v0.8 release, having it in a separate tree is
> > > > > extremely inconvenient. Please let me know how you think we should
> > > > > proceed?
> > > >
> > > > we need to wait with perf changes after the libbpf is merged and
> > > > libbpf 0.8.0 is released.. so we don't break dynamic linking for
> > > > perf
> > > >
> > > > at the moment please just take libbpf change and I'll resend the
> > > > perf change later if needed
> > >
> > > Ok.
> > >
> >
> > Jiri, libbpf v0.8 is out, can you please re-send your perf patches?
> 
> We still haven't done as Andrii suggested in:
> https://lore.kernel.org/lkml/CAEf4BzbbOHQZUAe6iWaehKCPQAf3VC=hq657buqe2_yRKxaK-A@mail.gmail.com/
> so for LIBBPF_DYNAMIC I believe we're compiling against the header
> files in tools/lib rather than the installed libbpf's header files.
> This may have some unexpected consequences.

like your program is not doing what you expected.. been there ;-)
sounds good, will check

jirka

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Jiri Olsa]

On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote:
> On Wed, May 11, 2022 at 11:22 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu:
> > > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote:
> > > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > >
> > > > > hi,
> > > > > sending change we discussed some time ago [1] to get rid of
> > > > > some deprecated functions we use in perf prologue code.
> > > > >
> > > > > Despite the gloomy discussion I think the final code does
> > > > > not look that bad ;-)
> > > > >
> > > > > This patchset removes following libbpf functions from perf:
> > > > >   bpf_program__set_prep
> > > > >   bpf_program__nth_fd
> > > > >   struct bpf_prog_prep_result
> > > > >
> > > > > v2 changes:
> > > > >   - use fallback section prog handler, so we don't need to
> > > > >     use section prefix [Andrii]
> > > > >   - realloc prog->insns array in bpf_program__set_insns [Andrii]
> > > > >   - squash patch 1 from previous version with
> > > > >     bpf_program__set_insns change [Daniel]
> > > > >   - patch 3 already merged [Arnaldo]
> > > > >   - added more comments
> > > > >
> > > > >   meanwhile.. perf/core and bpf-next diverged, so:
> > > > >     - libbpf bpf_program__set_insns change is based on bpf-next/master
> > > > >     - perf changes do not apply on bpf-next/master so they are based on
> > > > >       perf/core ... however they can be merged only after we release
> > > > >       libbpf 0.8.0 with bpf_program__set_insns change, so we don't break
> > > > >       the dynamic linking
> > > > >       I'm sending perf changes now just for review, I'll resend them
> > > > >       once libbpf 0.8.0 is released
> > > > >
> > > > > thanks,
> > > > > jirka
> > > > >
> > > > >
> > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/
> > > > > ---
> > > > > Jiri Olsa (1):
> > > > >       libbpf: Add bpf_program__set_insns function
> > > > >
> > > >
> > > > The first patch looks good to me. The rest I can't really review and
> > > > test properly, so I'll leave it up to Arnaldo.
> > > >
> > > > Arnaldo, how do we coordinate these patches? Should they go through
> > > > bpf-next (after you Ack them) or you want them in your tree?
> > > >
> > > > I'd like to get the bpf_program__set_insns() patch into bpf-next so
> > > > that I can do libbpf v0.8 release, having it in a separate tree is
> > > > extremely inconvenient. Please let me know how you think we should
> > > > proceed?
> > >
> > > we need to wait with perf changes after the libbpf is merged and
> > > libbpf 0.8.0 is released.. so we don't break dynamic linking for
> > > perf
> > >
> > > at the moment please just take libbpf change and I'll resend the
> > > perf change later if needed
> >
> > Ok.
> >
> 
> Jiri, libbpf v0.8 is out, can you please re-send your perf patches?

yep, just made new fedora package.. will resend the perf changes soon

jirka

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Jiri Olsa]

On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote:
> On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote:
> > On Wed, May 11, 2022 at 11:22 AM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> > >
> > > Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu:
> > > > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote:
> > > > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > > >
> > > > > > hi,
> > > > > > sending change we discussed some time ago [1] to get rid of
> > > > > > some deprecated functions we use in perf prologue code.
> > > > > >
> > > > > > Despite the gloomy discussion I think the final code does
> > > > > > not look that bad ;-)
> > > > > >
> > > > > > This patchset removes following libbpf functions from perf:
> > > > > >   bpf_program__set_prep
> > > > > >   bpf_program__nth_fd
> > > > > >   struct bpf_prog_prep_result
> > > > > >
> > > > > > v2 changes:
> > > > > >   - use fallback section prog handler, so we don't need to
> > > > > >     use section prefix [Andrii]
> > > > > >   - realloc prog->insns array in bpf_program__set_insns [Andrii]
> > > > > >   - squash patch 1 from previous version with
> > > > > >     bpf_program__set_insns change [Daniel]
> > > > > >   - patch 3 already merged [Arnaldo]
> > > > > >   - added more comments
> > > > > >
> > > > > >   meanwhile.. perf/core and bpf-next diverged, so:
> > > > > >     - libbpf bpf_program__set_insns change is based on bpf-next/master
> > > > > >     - perf changes do not apply on bpf-next/master so they are based on
> > > > > >       perf/core ... however they can be merged only after we release
> > > > > >       libbpf 0.8.0 with bpf_program__set_insns change, so we don't break
> > > > > >       the dynamic linking
> > > > > >       I'm sending perf changes now just for review, I'll resend them
> > > > > >       once libbpf 0.8.0 is released
> > > > > >
> > > > > > thanks,
> > > > > > jirka
> > > > > >
> > > > > >
> > > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/
> > > > > > ---
> > > > > > Jiri Olsa (1):
> > > > > >       libbpf: Add bpf_program__set_insns function
> > > > > >
> > > > >
> > > > > The first patch looks good to me. The rest I can't really review and
> > > > > test properly, so I'll leave it up to Arnaldo.
> > > > >
> > > > > Arnaldo, how do we coordinate these patches? Should they go through
> > > > > bpf-next (after you Ack them) or you want them in your tree?
> > > > >
> > > > > I'd like to get the bpf_program__set_insns() patch into bpf-next so
> > > > > that I can do libbpf v0.8 release, having it in a separate tree is
> > > > > extremely inconvenient. Please let me know how you think we should
> > > > > proceed?
> > > >
> > > > we need to wait with perf changes after the libbpf is merged and
> > > > libbpf 0.8.0 is released.. so we don't break dynamic linking for
> > > > perf
> > > >
> > > > at the moment please just take libbpf change and I'll resend the
> > > > perf change later if needed
> > >
> > > Ok.
> > >
> > 
> > Jiri, libbpf v0.8 is out, can you please re-send your perf patches?
> 
> yep, just made new fedora package.. will resend the perf changes soon

fedora package is on the way, but I'll need perf/core to merge
the bpf_program__set_insns change.. Arnaldo, any idea when this
could happen?

thanks,
jirka

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Andrii Nakryiko]

On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote:
> > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote:
> > > On Wed, May 11, 2022 at 11:22 AM Arnaldo Carvalho de Melo
> > > <acme@kernel.org> wrote:
> > > >
> > > > Em Wed, May 11, 2022 at 09:35:34AM +0200, Jiri Olsa escreveu:
> > > > > On Tue, May 10, 2022 at 04:48:55PM -0700, Andrii Nakryiko wrote:
> > > > > > On Tue, May 10, 2022 at 12:47 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > > > >
> > > > > > > hi,
> > > > > > > sending change we discussed some time ago [1] to get rid of
> > > > > > > some deprecated functions we use in perf prologue code.
> > > > > > >
> > > > > > > Despite the gloomy discussion I think the final code does
> > > > > > > not look that bad ;-)
> > > > > > >
> > > > > > > This patchset removes following libbpf functions from perf:
> > > > > > >   bpf_program__set_prep
> > > > > > >   bpf_program__nth_fd
> > > > > > >   struct bpf_prog_prep_result
> > > > > > >
> > > > > > > v2 changes:
> > > > > > >   - use fallback section prog handler, so we don't need to
> > > > > > >     use section prefix [Andrii]
> > > > > > >   - realloc prog->insns array in bpf_program__set_insns [Andrii]
> > > > > > >   - squash patch 1 from previous version with
> > > > > > >     bpf_program__set_insns change [Daniel]
> > > > > > >   - patch 3 already merged [Arnaldo]
> > > > > > >   - added more comments
> > > > > > >
> > > > > > >   meanwhile.. perf/core and bpf-next diverged, so:
> > > > > > >     - libbpf bpf_program__set_insns change is based on bpf-next/master
> > > > > > >     - perf changes do not apply on bpf-next/master so they are based on
> > > > > > >       perf/core ... however they can be merged only after we release
> > > > > > >       libbpf 0.8.0 with bpf_program__set_insns change, so we don't break
> > > > > > >       the dynamic linking
> > > > > > >       I'm sending perf changes now just for review, I'll resend them
> > > > > > >       once libbpf 0.8.0 is released
> > > > > > >
> > > > > > > thanks,
> > > > > > > jirka
> > > > > > >
> > > > > > >
> > > > > > > [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/
> > > > > > > ---
> > > > > > > Jiri Olsa (1):
> > > > > > >       libbpf: Add bpf_program__set_insns function
> > > > > > >
> > > > > >
> > > > > > The first patch looks good to me. The rest I can't really review and
> > > > > > test properly, so I'll leave it up to Arnaldo.
> > > > > >
> > > > > > Arnaldo, how do we coordinate these patches? Should they go through
> > > > > > bpf-next (after you Ack them) or you want them in your tree?
> > > > > >
> > > > > > I'd like to get the bpf_program__set_insns() patch into bpf-next so
> > > > > > that I can do libbpf v0.8 release, having it in a separate tree is
> > > > > > extremely inconvenient. Please let me know how you think we should
> > > > > > proceed?
> > > > >
> > > > > we need to wait with perf changes after the libbpf is merged and
> > > > > libbpf 0.8.0 is released.. so we don't break dynamic linking for
> > > > > perf
> > > > >
> > > > > at the moment please just take libbpf change and I'll resend the
> > > > > perf change later if needed
> > > >
> > > > Ok.
> > > >
> > >
> > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches?
> >
> > yep, just made new fedora package.. will resend the perf changes soon
>
> fedora package is on the way, but I'll need perf/core to merge
> the bpf_program__set_insns change.. Arnaldo, any idea when this
> could happen?
>

Jiri, Arnaldo,

Can we land these patches through bpf-next to avoid such complicated
cross-tree dependencies? As I started removing libbpf APIs I also
noticed that perf is still using few other deprecated APIs:
  - bpf_map__next;
  - bpf_program__next;
  - bpf_load_program;
  - btf__get_from_id;

It's trivial to fix up, but doing it across few trees will delay
libbpf work as well.

So let's land this through bpf-next, if Arnaldo doesn't mind?

> thanks,
> jirka

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Arnaldo Carvalho de Melo]

Em Fri, May 20, 2022 at 02:46:49PM -0700, Andrii Nakryiko escreveu:
> On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote:
> > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote:
> > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches?

> > > yep, just made new fedora package.. will resend the perf changes soon

> > fedora package is on the way, but I'll need perf/core to merge
> > the bpf_program__set_insns change.. Arnaldo, any idea when this
> > could happen?

> Can we land these patches through bpf-next to avoid such complicated
> cross-tree dependencies? As I started removing libbpf APIs I also
> noticed that perf is still using few other deprecated APIs:
>   - bpf_map__next;
>   - bpf_program__next;
>   - bpf_load_program;
>   - btf__get_from_id;
 
> It's trivial to fix up, but doing it across few trees will delay
> libbpf work as well.
 
> So let's land this through bpf-next, if Arnaldo doesn't mind?

Yeah, that should be ok, the only consideration is that I'm submitting
this today to Linus:

https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=0ae065a5d265bc5ada13e350015458e0c5e5c351

To address this:

https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com/

- Arnaldo

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Jiri Olsa]

On Sat, May 21, 2022 at 02:44:05PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, May 20, 2022 at 02:46:49PM -0700, Andrii Nakryiko escreveu:
> > On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote:
> > > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote:
> > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches?
> 
> > > > yep, just made new fedora package.. will resend the perf changes soon
> 
> > > fedora package is on the way, but I'll need perf/core to merge
> > > the bpf_program__set_insns change.. Arnaldo, any idea when this
> > > could happen?
> 
> > Can we land these patches through bpf-next to avoid such complicated
> > cross-tree dependencies? As I started removing libbpf APIs I also
> > noticed that perf is still using few other deprecated APIs:
> >   - bpf_map__next;
> >   - bpf_program__next;
> >   - bpf_load_program;
> >   - btf__get_from_id;

these were added just to bypass the time window when they were not
available in the package, so can be removed now (in the patch below)

>  
> > It's trivial to fix up, but doing it across few trees will delay
> > libbpf work as well.
>  
> > So let's land this through bpf-next, if Arnaldo doesn't mind?
> 
> Yeah, that should be ok, the only consideration is that I'm submitting
> this today to Linus:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=0ae065a5d265bc5ada13e350015458e0c5e5c351
> 
> To address this:
> 
> https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com/

ok, we can do that via bpf-next, but of course there's a problem ;-)

perf/core already has dependency commit [1]

so either we wait for perf/core and bpf-next/master to sync or:

  - perf/core reverts [1] and
  - bpf-next/master takes [1] and the rest

I have the changes ready if you guys are ok with that

thanks,
jirka
 

[1] https://lore.kernel.org/bpf/20220422100025.1469207-4-jolsa@kernel.org/

---
Subject: [PATCH bpf-next] perf tools: Remove the weak libbpf functions

We added weak functions for some new libbpf functions because
they were not packaged at that time [1].

These functions are now available in package, so we can remove
their weak perf variants.

[1] https://lore.kernel.org/linux-perf-users/20211109140707.1689940-2-jolsa@kernel.org/

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/bpf-event.c | 51 -------------------------------------
 1 file changed, 51 deletions(-)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index 94624733af7e..025f331b3867 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -22,57 +22,6 @@
 #include "record.h"
 #include "util/synthetic-events.h"
 
-struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
-{
-       struct btf *btf;
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-       int err = btf__get_from_id(id, &btf);
-#pragma GCC diagnostic pop
-
-       return err ? ERR_PTR(err) : btf;
-}
-
-int __weak bpf_prog_load(enum bpf_prog_type prog_type,
-			 const char *prog_name __maybe_unused,
-			 const char *license,
-			 const struct bpf_insn *insns, size_t insn_cnt,
-			 const struct bpf_prog_load_opts *opts)
-{
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-	return bpf_load_program(prog_type, insns, insn_cnt, license,
-				opts->kern_version, opts->log_buf, opts->log_size);
-#pragma GCC diagnostic pop
-}
-
-struct bpf_program * __weak
-bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
-{
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-	return bpf_program__next(prev, obj);
-#pragma GCC diagnostic pop
-}
-
-struct bpf_map * __weak
-bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev)
-{
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-	return bpf_map__next(prev, obj);
-#pragma GCC diagnostic pop
-}
-
-const void * __weak
-btf__raw_data(const struct btf *btf_ro, __u32 *size)
-{
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-	return btf__get_raw_data(btf_ro, size);
-#pragma GCC diagnostic pop
-}
-
 static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len)
 {
 	int ret = 0;
-- 
2.35.3

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Andrii Nakryiko]

On Mon, May 23, 2022 at 12:49 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Sat, May 21, 2022 at 02:44:05PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, May 20, 2022 at 02:46:49PM -0700, Andrii Nakryiko escreveu:
> > > On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > > > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote:
> > > > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote:
> > > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches?
> >
> > > > > yep, just made new fedora package.. will resend the perf changes soon
> >
> > > > fedora package is on the way, but I'll need perf/core to merge
> > > > the bpf_program__set_insns change.. Arnaldo, any idea when this
> > > > could happen?
> >
> > > Can we land these patches through bpf-next to avoid such complicated
> > > cross-tree dependencies? As I started removing libbpf APIs I also
> > > noticed that perf is still using few other deprecated APIs:
> > >   - bpf_map__next;
> > >   - bpf_program__next;
> > >   - bpf_load_program;
> > >   - btf__get_from_id;
>
> these were added just to bypass the time window when they were not
> available in the package, so can be removed now (in the patch below)
>
> >
> > > It's trivial to fix up, but doing it across few trees will delay
> > > libbpf work as well.
> >
> > > So let's land this through bpf-next, if Arnaldo doesn't mind?
> >
> > Yeah, that should be ok, the only consideration is that I'm submitting
> > this today to Linus:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=0ae065a5d265bc5ada13e350015458e0c5e5c351
> >
> > To address this:
> >
> > https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com/
>
> ok, we can do that via bpf-next, but of course there's a problem ;-)
>
> perf/core already has dependency commit [1]
>
> so either we wait for perf/core and bpf-next/master to sync or:
>
>   - perf/core reverts [1] and
>   - bpf-next/master takes [1] and the rest
>
> I have the changes ready if you guys are ok with that

So, if I understand correctly, with merge window open bpf-next/master
will get code from perf/core soon when we merge tip back in. So we can
wait for that to happen and not revert anything.

So please add the below patch to your series and resend once tip is
merged into bpf-next? Thanks!

>
> thanks,
> jirka
>
>
> [1] https://lore.kernel.org/bpf/20220422100025.1469207-4-jolsa@kernel.org/
>
> ---
> Subject: [PATCH bpf-next] perf tools: Remove the weak libbpf functions
>
> We added weak functions for some new libbpf functions because
> they were not packaged at that time [1].
>
> These functions are now available in package, so we can remove
> their weak perf variants.
>
> [1] https://lore.kernel.org/linux-perf-users/20211109140707.1689940-2-jolsa@kernel.org/
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/bpf-event.c | 51 -------------------------------------
>  1 file changed, 51 deletions(-)
>
> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index 94624733af7e..025f331b3867 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
> @@ -22,57 +22,6 @@
>  #include "record.h"
>  #include "util/synthetic-events.h"
>
> -struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
> -{
> -       struct btf *btf;
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> -       int err = btf__get_from_id(id, &btf);
> -#pragma GCC diagnostic pop
> -
> -       return err ? ERR_PTR(err) : btf;
> -}
> -
> -int __weak bpf_prog_load(enum bpf_prog_type prog_type,
> -                        const char *prog_name __maybe_unused,
> -                        const char *license,
> -                        const struct bpf_insn *insns, size_t insn_cnt,
> -                        const struct bpf_prog_load_opts *opts)
> -{
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> -       return bpf_load_program(prog_type, insns, insn_cnt, license,
> -                               opts->kern_version, opts->log_buf, opts->log_size);
> -#pragma GCC diagnostic pop
> -}
> -
> -struct bpf_program * __weak
> -bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
> -{
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> -       return bpf_program__next(prev, obj);
> -#pragma GCC diagnostic pop
> -}
> -
> -struct bpf_map * __weak
> -bpf_object__next_map(const struct bpf_object *obj, const struct bpf_map *prev)
> -{
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> -       return bpf_map__next(prev, obj);
> -#pragma GCC diagnostic pop
> -}
> -
> -const void * __weak
> -btf__raw_data(const struct btf *btf_ro, __u32 *size)
> -{
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> -       return btf__get_raw_data(btf_ro, size);
> -#pragma GCC diagnostic pop
> -}
> -
>  static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len)
>  {
>         int ret = 0;
> --
> 2.35.3
>

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Jiri Olsa]

On Mon, May 23, 2022 at 03:43:10PM -0700, Andrii Nakryiko wrote:
> On Mon, May 23, 2022 at 12:49 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Sat, May 21, 2022 at 02:44:05PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Fri, May 20, 2022 at 02:46:49PM -0700, Andrii Nakryiko escreveu:
> > > > On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > > > > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote:
> > > > > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote:
> > > > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches?
> > >
> > > > > > yep, just made new fedora package.. will resend the perf changes soon
> > >
> > > > > fedora package is on the way, but I'll need perf/core to merge
> > > > > the bpf_program__set_insns change.. Arnaldo, any idea when this
> > > > > could happen?
> > >
> > > > Can we land these patches through bpf-next to avoid such complicated
> > > > cross-tree dependencies? As I started removing libbpf APIs I also
> > > > noticed that perf is still using few other deprecated APIs:
> > > >   - bpf_map__next;
> > > >   - bpf_program__next;
> > > >   - bpf_load_program;
> > > >   - btf__get_from_id;
> >
> > these were added just to bypass the time window when they were not
> > available in the package, so can be removed now (in the patch below)
> >
> > >
> > > > It's trivial to fix up, but doing it across few trees will delay
> > > > libbpf work as well.
> > >
> > > > So let's land this through bpf-next, if Arnaldo doesn't mind?
> > >
> > > Yeah, that should be ok, the only consideration is that I'm submitting
> > > this today to Linus:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=0ae065a5d265bc5ada13e350015458e0c5e5c351
> > >
> > > To address this:
> > >
> > > https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com/
> >
> > ok, we can do that via bpf-next, but of course there's a problem ;-)
> >
> > perf/core already has dependency commit [1]
> >
> > so either we wait for perf/core and bpf-next/master to sync or:
> >
> >   - perf/core reverts [1] and
> >   - bpf-next/master takes [1] and the rest
> >
> > I have the changes ready if you guys are ok with that
> 
> So, if I understand correctly, with merge window open bpf-next/master
> will get code from perf/core soon when we merge tip back in. So we can
> wait for that to happen and not revert anything.
> 
> So please add the below patch to your series and resend once tip is
> merged into bpf-next? Thanks!

ok

jirka

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Andrii Nakryiko]

On Tue, May 24, 2022 at 1:28 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, May 23, 2022 at 03:43:10PM -0700, Andrii Nakryiko wrote:
> > On Mon, May 23, 2022 at 12:49 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Sat, May 21, 2022 at 02:44:05PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Fri, May 20, 2022 at 02:46:49PM -0700, Andrii Nakryiko escreveu:
> > > > > On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > > > > > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote:
> > > > > > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote:
> > > > > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches?
> > > >
> > > > > > > yep, just made new fedora package.. will resend the perf changes soon
> > > >
> > > > > > fedora package is on the way, but I'll need perf/core to merge
> > > > > > the bpf_program__set_insns change.. Arnaldo, any idea when this
> > > > > > could happen?
> > > >
> > > > > Can we land these patches through bpf-next to avoid such complicated
> > > > > cross-tree dependencies? As I started removing libbpf APIs I also
> > > > > noticed that perf is still using few other deprecated APIs:
> > > > >   - bpf_map__next;
> > > > >   - bpf_program__next;
> > > > >   - bpf_load_program;
> > > > >   - btf__get_from_id;
> > >
> > > these were added just to bypass the time window when they were not
> > > available in the package, so can be removed now (in the patch below)
> > >
> > > >
> > > > > It's trivial to fix up, but doing it across few trees will delay
> > > > > libbpf work as well.
> > > >
> > > > > So let's land this through bpf-next, if Arnaldo doesn't mind?
> > > >
> > > > Yeah, that should be ok, the only consideration is that I'm submitting
> > > > this today to Linus:
> > > >
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=0ae065a5d265bc5ada13e350015458e0c5e5c351
> > > >
> > > > To address this:
> > > >
> > > > https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com/
> > >
> > > ok, we can do that via bpf-next, but of course there's a problem ;-)
> > >
> > > perf/core already has dependency commit [1]
> > >
> > > so either we wait for perf/core and bpf-next/master to sync or:
> > >
> > >   - perf/core reverts [1] and
> > >   - bpf-next/master takes [1] and the rest
> > >
> > > I have the changes ready if you guys are ok with that
> >
> > So, if I understand correctly, with merge window open bpf-next/master
> > will get code from perf/core soon when we merge tip back in. So we can
> > wait for that to happen and not revert anything.
> >
> > So please add the below patch to your series and resend once tip is
> > merged into bpf-next? Thanks!
>
> ok

Hm.. Ok, so I don't see your patches in tip yet. I see them in
perf/core only. Which means things won't happen naturally soon. How
should we proceed? I'm sitting on a pile of patches removing a lot of
code from libbpf and I'd rather get it out soon, but I can't because
of them breaking perf in bpf-next without Jiri's changes.

Arnaldo, what's your suggestion? Can we land remaining Jiri's patches
into perf/core and then you can create a tag for us to merge into
bpf-next, so that we avoid any conflicts later? Would that work? I
think we did something like that with other trees (e.g., RCU), when we
had dependencies like this before.

Thoughts?

>
> jirka

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Jiri Olsa]

On Wed, Jun 01, 2022 at 10:39:08AM -0700, Andrii Nakryiko wrote:
> On Tue, May 24, 2022 at 1:28 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Mon, May 23, 2022 at 03:43:10PM -0700, Andrii Nakryiko wrote:
> > > On Mon, May 23, 2022 at 12:49 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > > >
> > > > On Sat, May 21, 2022 at 02:44:05PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > Em Fri, May 20, 2022 at 02:46:49PM -0700, Andrii Nakryiko escreveu:
> > > > > > On Thu, May 19, 2022 at 4:03 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > > > > > > On Wed, May 18, 2022 at 11:46:44AM +0200, Jiri Olsa wrote:
> > > > > > > > On Tue, May 17, 2022 at 03:02:53PM -0700, Andrii Nakryiko wrote:
> > > > > > > > > Jiri, libbpf v0.8 is out, can you please re-send your perf patches?
> > > > >
> > > > > > > > yep, just made new fedora package.. will resend the perf changes soon
> > > > >
> > > > > > > fedora package is on the way, but I'll need perf/core to merge
> > > > > > > the bpf_program__set_insns change.. Arnaldo, any idea when this
> > > > > > > could happen?
> > > > >
> > > > > > Can we land these patches through bpf-next to avoid such complicated
> > > > > > cross-tree dependencies? As I started removing libbpf APIs I also
> > > > > > noticed that perf is still using few other deprecated APIs:
> > > > > >   - bpf_map__next;
> > > > > >   - bpf_program__next;
> > > > > >   - bpf_load_program;
> > > > > >   - btf__get_from_id;
> > > >
> > > > these were added just to bypass the time window when they were not
> > > > available in the package, so can be removed now (in the patch below)
> > > >
> > > > >
> > > > > > It's trivial to fix up, but doing it across few trees will delay
> > > > > > libbpf work as well.
> > > > >
> > > > > > So let's land this through bpf-next, if Arnaldo doesn't mind?
> > > > >
> > > > > Yeah, that should be ok, the only consideration is that I'm submitting
> > > > > this today to Linus:
> > > > >
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/urgent&id=0ae065a5d265bc5ada13e350015458e0c5e5c351
> > > > >
> > > > > To address this:
> > > > >
> > > > > https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com/
> > > >
> > > > ok, we can do that via bpf-next, but of course there's a problem ;-)
> > > >
> > > > perf/core already has dependency commit [1]
> > > >
> > > > so either we wait for perf/core and bpf-next/master to sync or:
> > > >
> > > >   - perf/core reverts [1] and
> > > >   - bpf-next/master takes [1] and the rest
> > > >
> > > > I have the changes ready if you guys are ok with that
> > >
> > > So, if I understand correctly, with merge window open bpf-next/master
> > > will get code from perf/core soon when we merge tip back in. So we can
> > > wait for that to happen and not revert anything.
> > >
> > > So please add the below patch to your series and resend once tip is
> > > merged into bpf-next? Thanks!
> >
> > ok
> 
> Hm.. Ok, so I don't see your patches in tip yet. I see them in
> perf/core only. Which means things won't happen naturally soon. How
> should we proceed? I'm sitting on a pile of patches removing a lot of
> code from libbpf and I'd rather get it out soon, but I can't because
> of them breaking perf in bpf-next without Jiri's changes.

sorry it's merged in linus master, Arnaldo no longer goes through tip tree

> 
> Arnaldo, what's your suggestion? Can we land remaining Jiri's patches
> into perf/core and then you can create a tag for us to merge into
> bpf-next, so that we avoid any conflicts later? Would that work? I
> think we did something like that with other trees (e.g., RCU), when we
> had dependencies like this before.

either way is fine for me.. I just rebased those changes on top of perf/core

jirka

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Tue, 10 May 2022 09:46:56 +0200 you wrote:
> hi,
> sending change we discussed some time ago [1] to get rid of
> some deprecated functions we use in perf prologue code.
> 
> Despite the gloomy discussion I think the final code does
> not look that bad ;-)
> 
> [...]

Here is the summary with links:
  - [PATCHv2,bpf-next,1/3] libbpf: Add bpf_program__set_insns function
    https://git.kernel.org/bpf/bpf-next/c/b63b3c490eee
  - [PATCHv2,perf/core,2/3] perf tools: Register fallback libbpf section handler
    (no matching commit)
  - [PATCHv2,perf/core,3/3] perf tools: Rework prologue generation code
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Arnaldo Carvalho de Melo]

Em Tue, May 10, 2022 at 09:46:56AM +0200, Jiri Olsa escreveu:
> hi,
> sending change we discussed some time ago [1] to get rid of
> some deprecated functions we use in perf prologue code.
> 
> Despite the gloomy discussion I think the final code does
> not look that bad ;-)
> 
> This patchset removes following libbpf functions from perf:
>   bpf_program__set_prep
>   bpf_program__nth_fd
>   struct bpf_prog_prep_result

So, the first patch is already in torvalds/master, I tried applying the
other two patches to my local perf/core, that already is merged with
torvalds/master and:

[root@quaco ~]# perf test 42
 42: BPF filter                                                      :
 42.1: Basic BPF filtering                                           : FAILED!
 42.2: BPF pinning                                                   : FAILED!
 42.3: BPF prologue generation                                       : FAILED!
[root@quaco ~]#

I'll push my local perf/core to tmp.perf/core and continue tomorrow.

Its failing around here:

Open Debuginfo file: /root/.cache/debuginfod_client/e1c3de4b4c5db158f2098e80f2bf9140e8cfbdb6/debuginfo
Try to find probe point from debuginfo.
Matched function: do_epoll_wait [3806bb5]
Probe point found: do_epoll_wait+0
Found 1 probe_trace_events.
Looking at the vmlinux_path (8 entries long)
symsrc__init: build id mismatch for vmlinux.
symsrc__init: cannot get elf header.
Using /proc/kcore for kernel data
Using /proc/kallsyms for symbols
do_epoll_wait is out of .text, skip it.
Post processing failed or all events are skipped. (1)
Probe point 'do_epoll_wait' not found.
bpf_probe: failed to convert perf probe events
Failed to add events selected by BPF
test child finished with -1
---- end ----
BPF filter subtest 1: FAILED

But:

[root@quaco ~]# grep do_epoll_wait /proc/kallsyms
ffffffff973c2a30 t do_epoll_wait
[root@quaco ~]#

- Arnaldo
 
> v2 changes:
>   - use fallback section prog handler, so we don't need to
>     use section prefix [Andrii]
>   - realloc prog->insns array in bpf_program__set_insns [Andrii]
>   - squash patch 1 from previous version with
>     bpf_program__set_insns change [Daniel]
>   - patch 3 already merged [Arnaldo]
>   - added more comments
> 
>   meanwhile.. perf/core and bpf-next diverged, so:
>     - libbpf bpf_program__set_insns change is based on bpf-next/master
>     - perf changes do not apply on bpf-next/master so they are based on
>       perf/core ... however they can be merged only after we release
>       libbpf 0.8.0 with bpf_program__set_insns change, so we don't break
>       the dynamic linking
>       I'm sending perf changes now just for review, I'll resend them
>       once libbpf 0.8.0 is released
> 
> thanks,
> jirka
> 
> 
> [1] https://lore.kernel.org/bpf/CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com/
> ---
> Jiri Olsa (1):
>       libbpf: Add bpf_program__set_insns function
> 
>  tools/lib/bpf/libbpf.c   | 22 ++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   | 18 ++++++++++++++++++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 41 insertions(+)
> 
> Jiri Olsa (2):
>       perf tools: Register fallback libbpf section handler
>       perf tools: Rework prologue generation code
> 
>  tools/perf/util/bpf-loader.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 157 insertions(+), 18 deletions(-)

-- 

- Arnaldo

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Jiri Olsa]

On Thu, May 26, 2022 at 10:16:11PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, May 10, 2022 at 09:46:56AM +0200, Jiri Olsa escreveu:
> > hi,
> > sending change we discussed some time ago [1] to get rid of
> > some deprecated functions we use in perf prologue code.
> > 
> > Despite the gloomy discussion I think the final code does
> > not look that bad ;-)
> > 
> > This patchset removes following libbpf functions from perf:
> >   bpf_program__set_prep
> >   bpf_program__nth_fd
> >   struct bpf_prog_prep_result
> 
> So, the first patch is already in torvalds/master, I tried applying the
> other two patches to my local perf/core, that already is merged with
> torvalds/master and:
> 
> [root@quaco ~]# perf test 42
>  42: BPF filter                                                      :
>  42.1: Basic BPF filtering                                           : FAILED!
>  42.2: BPF pinning                                                   : FAILED!
>  42.3: BPF prologue generation                                       : FAILED!
> [root@quaco ~]#
> 
> I'll push my local perf/core to tmp.perf/core and continue tomorrow.

hi,
I just rebased my changes on top of your perf/core and it seems to work:

	[root@krava perf]# ./perf test bpf
	 40: LLVM search and compile                                         :
	 40.1: Basic BPF llvm compile                                        : Ok
	 40.3: Compile source for BPF prologue generation                    : Ok
	 40.4: Compile source for BPF relocation                             : Ok
	 42: BPF filter                                                      :
	 42.1: Basic BPF filtering                                           : Ok
	 42.2: BPF pinning                                                   : Ok
	 42.3: BPF prologue generation                                       : Ok

is it still a problem?

jirka

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Andrii Nakryiko]

On Wed, Jun 1, 2022 at 11:11 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, May 26, 2022 at 10:16:11PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, May 10, 2022 at 09:46:56AM +0200, Jiri Olsa escreveu:
> > > hi,
> > > sending change we discussed some time ago [1] to get rid of
> > > some deprecated functions we use in perf prologue code.
> > >
> > > Despite the gloomy discussion I think the final code does
> > > not look that bad ;-)
> > >
> > > This patchset removes following libbpf functions from perf:
> > >   bpf_program__set_prep
> > >   bpf_program__nth_fd
> > >   struct bpf_prog_prep_result
> >
> > So, the first patch is already in torvalds/master, I tried applying the
> > other two patches to my local perf/core, that already is merged with
> > torvalds/master and:
> >
> > [root@quaco ~]# perf test 42
> >  42: BPF filter                                                      :
> >  42.1: Basic BPF filtering                                           : FAILED!
> >  42.2: BPF pinning                                                   : FAILED!
> >  42.3: BPF prologue generation                                       : FAILED!
> > [root@quaco ~]#
> >
> > I'll push my local perf/core to tmp.perf/core and continue tomorrow.
>
> hi,
> I just rebased my changes on top of your perf/core and it seems to work:
>
>         [root@krava perf]# ./perf test bpf
>          40: LLVM search and compile                                         :
>          40.1: Basic BPF llvm compile                                        : Ok
>          40.3: Compile source for BPF prologue generation                    : Ok
>          40.4: Compile source for BPF relocation                             : Ok
>          42: BPF filter                                                      :
>          42.1: Basic BPF filtering                                           : Ok
>          42.2: BPF pinning                                                   : Ok
>          42.3: BPF prologue generation                                       : Ok
>
> is it still a problem?
>

Ok, so I checked with Jakub, net-next will be forwarded to
linus/master tomorrow or so, so after that bpf-next will get forwarded
as well and we'll have all those patches of yours. So let's go back to
plan A: send your perf changes based on bpf-next. Thanks and sorry for
the extra noise with all the back and forth.


> jirka

Re: [PATCHv2 0/3] perf tools: Fix prologue generation

[Jiri Olsa]

On Wed, Jun 01, 2022 at 03:21:06PM -0700, Andrii Nakryiko wrote:
> On Wed, Jun 1, 2022 at 11:11 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Thu, May 26, 2022 at 10:16:11PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, May 10, 2022 at 09:46:56AM +0200, Jiri Olsa escreveu:
> > > > hi,
> > > > sending change we discussed some time ago [1] to get rid of
> > > > some deprecated functions we use in perf prologue code.
> > > >
> > > > Despite the gloomy discussion I think the final code does
> > > > not look that bad ;-)
> > > >
> > > > This patchset removes following libbpf functions from perf:
> > > >   bpf_program__set_prep
> > > >   bpf_program__nth_fd
> > > >   struct bpf_prog_prep_result
> > >
> > > So, the first patch is already in torvalds/master, I tried applying the
> > > other two patches to my local perf/core, that already is merged with
> > > torvalds/master and:
> > >
> > > [root@quaco ~]# perf test 42
> > >  42: BPF filter                                                      :
> > >  42.1: Basic BPF filtering                                           : FAILED!
> > >  42.2: BPF pinning                                                   : FAILED!
> > >  42.3: BPF prologue generation                                       : FAILED!
> > > [root@quaco ~]#
> > >
> > > I'll push my local perf/core to tmp.perf/core and continue tomorrow.
> >
> > hi,
> > I just rebased my changes on top of your perf/core and it seems to work:
> >
> >         [root@krava perf]# ./perf test bpf
> >          40: LLVM search and compile                                         :
> >          40.1: Basic BPF llvm compile                                        : Ok
> >          40.3: Compile source for BPF prologue generation                    : Ok
> >          40.4: Compile source for BPF relocation                             : Ok
> >          42: BPF filter                                                      :
> >          42.1: Basic BPF filtering                                           : Ok
> >          42.2: BPF pinning                                                   : Ok
> >          42.3: BPF prologue generation                                       : Ok
> >
> > is it still a problem?
> >
> 
> Ok, so I checked with Jakub, net-next will be forwarded to
> linus/master tomorrow or so, so after that bpf-next will get forwarded
> as well and we'll have all those patches of yours. So let's go back to
> plan A: send your perf changes based on bpf-next. Thanks and sorry for
> the extra noise with all the back and forth.

ok, np

jirka