[PATCH bpf-next v4 0/2] perf: stop using deprecated bpf APIs

[PATCH bpf-next v4 0/2] perf: stop using deprecated bpf APIs

[Christy Lee]

libbpf's bpf_load_program() and bpf__object_next() APIs are deprecated.
remove perf's usage of these deprecated functions. After this patch
set, the only remaining libbpf deprecated APIs in perf would be
bpf_program__set_prep() and bpf_program__nth_fd().

Changelog:
----------
v3 -> v4:
* Fixed commit title
* Added weak definition for deprecated function

v2 -> v3:
https://lore.kernel.org/all/20220106200032.3067127-1-christylee@fb.com/

Patch 2/2:
Fixed commit message to use upstream perf

v1 -> v2:
https://lore.kernel.org/all/20211216222108.110518-1-christylee@fb.com/

Patch 1/2:
Added missing commit message

Patch 2/2:
Added more details to commit message and added steps to reproduce
original test case.

Christy Lee (2):
  perf: stop using deprecated bpf_load_program() API
  perf: stop using deprecated bpf_object__next() API

 tools/perf/tests/bpf.c       | 14 ++-----
 tools/perf/util/bpf-event.c  | 16 ++++++++
 tools/perf/util/bpf-loader.c | 72 +++++++++++++++++++++++++++---------
 tools/perf/util/bpf-loader.h |  1 +
 4 files changed, 75 insertions(+), 28 deletions(-)

--
2.30.2

[PATCH bpf-next v4 1/2] perf: stop using deprecated bpf_load_program() API

[Christy Lee]

bpf_load_program() API is deprecated, remove perf's usage of the
deprecated function. Add a __weak function declaration for libbpf
version compatibility.

Signed-off-by: Christy Lee <christylee@fb.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/perf/tests/bpf.c      | 14 ++++----------
 tools/perf/util/bpf-event.c | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index 573490530194..57b9591f7cbb 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -281,8 +281,8 @@ static int __test__bpf(int idx)
 
 static int check_env(void)
 {
+	LIBBPF_OPTS(bpf_prog_load_opts, opts);
 	int err;
-	unsigned int kver_int;
 	char license[] = "GPL";
 
 	struct bpf_insn insns[] = {
@@ -290,19 +290,13 @@ static int check_env(void)
 		BPF_EXIT_INSN(),
 	};
 
-	err = fetch_kernel_version(&kver_int, NULL, 0);
+	err = fetch_kernel_version(&opts.kern_version, NULL, 0);
 	if (err) {
 		pr_debug("Unable to get kernel version\n");
 		return err;
 	}
-
-/* temporarily disable libbpf deprecation warnings */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-	err = bpf_load_program(BPF_PROG_TYPE_KPROBE, insns,
-			       ARRAY_SIZE(insns),
-			       license, kver_int, NULL, 0);
-#pragma GCC diagnostic pop
+	err = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, license, insns,
+			    ARRAY_SIZE(insns), &opts);
 	if (err < 0) {
 		pr_err("Missing basic BPF support, skip this test: %s\n",
 		       strerror(errno));
diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index a517eaa51eb3..48872276c0b7 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -33,6 +33,22 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
        return err ? ERR_PTR(err) : btf;
 }
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-parameter"
+int __weak
+bpf_prog_load(enum bpf_prog_type prog_type,
+		const char *prog_name, 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
+}
+#pragma GCC diagnostic pop
+
 struct bpf_program * __weak
 bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
 {
-- 
2.30.2

[PATCH bpf-next v4 2/2] perf: stop using deprecated bpf_object__next() API

[Christy Lee]

Libbpf has deprecated the ability to keep track of object list inside
libbpf, it now requires applications to track usage multiple bpf
objects directly. Remove usage of bpf_object__next() API  and hoist the
tracking logic to perf.

Committer note:

This is tested by following the committer's note in the original commit
"aa3abf30bb28addcf593578d37447d42e3f65fc3".

I ran 'perf test -v LLVM' and used it's output to generate a script for
compiling the perf test object:

--------------------------------------------------
$ cat ~/bin/hello-ebpf
INPUT_FILE=/tmp/test.c
OUTPUT_FILE=/tmp/test.o

export KBUILD_DIR=/lib/modules/5.12.0-0_fbk2_3390_g7ecb4ac46d7f/build
export NR_CPUS=56
export LINUX_VERSION_CODE=0x50c00
export CLANG_EXEC=/data/users/christylee/devtools/llvm/latest/bin/clang
export CLANG_OPTIONS=-xc
export KERNEL_INC_OPTIONS="KERNEL_INC_OPTIONS= -nostdinc \
-isystem /usr/lib/gcc/x86_64-redhat-linux/8/include -I./arch/x86/include \
-I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi \
-I./arch/x86/include/generated/uapi -I./include/uapi \
-I./include/generated/uapi -include ./include/linux/compiler-version.h \
-include ./include/linux/kconfig.h"
export PERF_BPF_INC_OPTIONS=-I/usr/lib/perf/include/bpf
export WORKING_DIR=/lib/modules/5.12.0-0_fbk2_3390_g7ecb4ac46d7f/build
export CLANG_SOURCE=-

rm -f $OUTPUT_FILE
cat $INPUT_FILE | /data/users/christylee/devtools/llvm/latest/bin/clang \
-D__KERNEL__ -D__NR_CPUS__=56 -DLINUX_VERSION_CODE=0x50c00 -xc  \
-I/usr/lib/perf/include/bpf -nostdinc \
-isystem /usr/lib/gcc/x86_64-redhat-linux/8/include -I./arch/x86/include \
-I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi \
-I./arch/x86/include/generated/uapi -I./include/uapi \
-I./include/generated/uapi -include ./include/linux/compiler-version.h \
-include ./include/linux/kconfig.h -Wno-unused-value -Wno-pointer-sign \
-working-directory /lib/modules/5.12.0-0_fbk2_3390_g7ecb4ac46d7f/build \
-c - -target bpf -O2 -o $OUTPUT_FILE
--------------------------------------------------

I then wrote and compiled a script that ask to get asks to put a probe
at a function that
does not exists in the kernel, it errors out as expected:

$ cat /tmp/test.c
__attribute__((section("probe_point=not_exist"), used))
int probe_point(void *ctx) {
    return 0;
}
char _license[] __attribute__((section("license"), used)) = "GPL";
int _version __attribute__((section("version"), used)) = 0x40100;

$ cd ~/bin && ./hello-ebpf
$ ./perf record --event /tmp/test.o sleep 1

Probe point 'not_exist' not found.
event syntax error: '/tmp/test.o'
                     \___ You need to check probing points in BPF file

(add -v to see detail)
Run 'perf list' for a list of valid events

 Usage: perf record [<options>] [<command>]
    or: perf record [<options>] -- <command> [<options>]

    -e, --event <event>   event selector. use 'perf list' to list
available events

---------------------------------------------------

Next I changed the attribute to something that exists in the kernel.
As expected, it errors out
with permission problem:
$ cat /tmp/test.c
__attribute__((section("probe_point=kernel_execve"), used))
int probe_point(void *ctx) {
    return 0;
}
char _license[] __attribute__((section("license"), used)) = "GPL";
int _version __attribute__((section("version"), used)) = 0x40100;

$ grep kernel_execve /proc/kallsyms
ffffffff812dc210 T kernel_execve

$ cd ~/bin && ./hello-ebpf
$ ./perf record --event /tmp/test.o sleep 1

Failed to open kprobe_events: Permission denied
event syntax error: '/tmp/test.o'
                     \___ You need to be root

(add -v to see detail)
Run 'perf list' for a list of valid events

 Usage: perf record [<options>] [<command>]
    or: perf record [<options>] -- <command> [<options>]

    -e, --event <event>   event selector. use 'perf list' to list
available events

---------------------------------------------------

Reran as root, see that the probe point worked as intended.

$ sudo -i
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.018 MB perf.data ]
perf_bpf_probe:probe_point
perf_bpf_probe:probe_point: type: 2, size: 128, config: 0x8e9, \
{ sample_period, sample_freq }: 1, \
sample_type: IP|TID|TIME|CPU|PERIOD|RAW, read_format: ID, disabled: 1, \
inherit: 1, mmap: 1, comm: 1, enable_on_exec: 1, task: 1, \
sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, \
bpf_event: 1

---------------------------------------------------

Signed-off-by: Christy Lee <christylee@fb.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/perf/util/bpf-loader.c | 72 +++++++++++++++++++++++++++---------
 tools/perf/util/bpf-loader.h |  1 +
 2 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 4631cac3957f..b1822f8af2bb 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -29,9 +29,6 @@
 
 #include <internal/xyarray.h>
 
-/* temporarily disable libbpf deprecation warnings */
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-
 static int libbpf_perf_print(enum libbpf_print_level level __attribute__((unused)),
 			      const char *fmt, va_list args)
 {
@@ -49,6 +46,36 @@ struct bpf_prog_priv {
 	int *type_mapping;
 };
 
+struct bpf_perf_object {
+	struct bpf_object *obj;
+	struct list_head list;
+};
+
+static LIST_HEAD(bpf_objects_list);
+
+struct bpf_perf_object *bpf_perf_object__next(struct bpf_perf_object *prev)
+{
+	struct bpf_perf_object *next;
+
+	if (!prev)
+		next = list_first_entry(&bpf_objects_list,
+					struct bpf_perf_object, list);
+	else
+		next = list_next_entry(prev, list);
+
+	/* Empty list is noticed here so don't need checking on entry. */
+	if (&next->list == &bpf_objects_list)
+		return NULL;
+
+	return next;
+}
+
+#define bpf_perf_object__for_each(perf_obj, tmp, obj)                          \
+	for ((perf_obj) = bpf_perf_object__next(NULL),                         \
+	    (tmp) = bpf_perf_object__next(perf_obj), (obj) = NULL;             \
+	     (perf_obj) != NULL; (perf_obj) = (tmp),                           \
+	    (tmp) = bpf_perf_object__next(tmp), (obj) = (perf_obj)->obj)
+
 static bool libbpf_initialized;
 
 struct bpf_object *
@@ -113,9 +140,10 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
 
 void bpf__clear(void)
 {
-	struct bpf_object *obj, *tmp;
+	struct bpf_perf_object *perf_obj, *tmp;
+	struct bpf_object *obj;
 
-	bpf_object__for_each_safe(obj, tmp) {
+	bpf_perf_object__for_each(perf_obj, tmp, obj) {
 		bpf__unprobe(obj);
 		bpf_object__close(obj);
 	}
@@ -621,8 +649,12 @@ static int hook_load_preprocessor(struct bpf_program *prog)
 	if (err)
 		return err;
 
+/* temporarily disable libbpf deprecation warnings */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 	err = bpf_program__set_prep(prog, priv->nr_types,
 				    preproc_gen_prologue);
+#pragma GCC diagnostic pop
 	return err;
 }
 
@@ -776,7 +808,11 @@ int bpf__foreach_event(struct bpf_object *obj,
 			if (priv->need_prologue) {
 				int type = priv->type_mapping[i];
 
+/* temporarily disable libbpf deprecation warnings */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 				fd = bpf_program__nth_fd(prog, type);
+#pragma GCC diagnostic pop
 			} else {
 				fd = bpf_program__fd(prog);
 			}
@@ -1488,10 +1524,11 @@ apply_obj_config_object(struct bpf_object *obj)
 
 int bpf__apply_obj_config(void)
 {
-	struct bpf_object *obj, *tmp;
+	struct bpf_perf_object *perf_obj, *tmp;
+	struct bpf_object *obj;
 	int err;
 
-	bpf_object__for_each_safe(obj, tmp) {
+	bpf_perf_object__for_each(perf_obj, tmp, obj) {
 		err = apply_obj_config_object(obj);
 		if (err)
 			return err;
@@ -1500,26 +1537,25 @@ int bpf__apply_obj_config(void)
 	return 0;
 }
 
-#define bpf__for_each_map(pos, obj, objtmp)	\
-	bpf_object__for_each_safe(obj, objtmp)	\
-		bpf_object__for_each_map(pos, obj)
+#define bpf__perf_for_each_map(perf_obj, tmp, obj, map)                        \
+	bpf_perf_object__for_each(perf_obj, tmp, obj)                          \
+		bpf_object__for_each_map(map, obj)
 
-#define bpf__for_each_map_named(pos, obj, objtmp, name)	\
-	bpf__for_each_map(pos, obj, objtmp) 		\
-		if (bpf_map__name(pos) && 		\
-			(strcmp(name, 			\
-				bpf_map__name(pos)) == 0))
+#define bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name)            \
+	bpf__perf_for_each_map(perf_obj, tmp, obj, map)                        \
+		if (bpf_map__name(map) && (strcmp(name, bpf_map__name(map)) == 0))
 
 struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
 {
 	struct bpf_map_priv *tmpl_priv = NULL;
-	struct bpf_object *obj, *tmp;
+	struct bpf_perf_object *perf_obj, *tmp;
+	struct bpf_object *obj;
 	struct evsel *evsel = NULL;
 	struct bpf_map *map;
 	int err;
 	bool need_init = false;
 
-	bpf__for_each_map_named(map, obj, tmp, name) {
+	bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name) {
 		struct bpf_map_priv *priv = bpf_map__priv(map);
 
 		if (IS_ERR(priv))
@@ -1555,7 +1591,7 @@ struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
 		evsel = evlist__last(evlist);
 	}
 
-	bpf__for_each_map_named(map, obj, tmp, name) {
+	bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name) {
 		struct bpf_map_priv *priv = bpf_map__priv(map);
 
 		if (IS_ERR(priv))
diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
index 5d1c725cea29..95262b7e936f 100644
--- a/tools/perf/util/bpf-loader.h
+++ b/tools/perf/util/bpf-loader.h
@@ -53,6 +53,7 @@ typedef int (*bpf_prog_iter_callback_t)(const char *group, const char *event,
 
 #ifdef HAVE_LIBBPF_SUPPORT
 struct bpf_object *bpf__prepare_load(const char *filename, bool source);
+struct bpf_perf_object *bpf_perf_object__next(struct bpf_perf_object *prev);
 int bpf__strerror_prepare_load(const char *filename, bool source,
 			       int err, char *buf, size_t size);
 
-- 
2.30.2

Re: [PATCH bpf-next v4 2/2] perf: stop using deprecated bpf_object__next() API

[Song Liu]

> On Jan 19, 2022, at 3:06 PM, Christy Lee <christylee@fb.com> wrote:
> 

[...]

> $ sudo -i
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.018 MB perf.data ]
> perf_bpf_probe:probe_point
> perf_bpf_probe:probe_point: type: 2, size: 128, config: 0x8e9, \
> { sample_period, sample_freq }: 1, \
> sample_type: IP|TID|TIME|CPU|PERIOD|RAW, read_format: ID, disabled: 1, \
> inherit: 1, mmap: 1, comm: 1, enable_on_exec: 1, task: 1, \
> sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, \
> bpf_event: 1
> 
> ---------------------------------------------------
> 
> Signed-off-by: Christy Lee <christylee@fb.com>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Song Liu <songliubraving@fb.com>

[...]

Re: [PATCH bpf-next v4 1/2] perf: stop using deprecated bpf_load_program() API

[Andrii Nakryiko]

On Wed, Jan 19, 2022 at 3:09 PM Christy Lee <christylee@fb.com> wrote:
>
> bpf_load_program() API is deprecated, remove perf's usage of the
> deprecated function. Add a __weak function declaration for libbpf
> version compatibility.
>
> Signed-off-by: Christy Lee <christylee@fb.com>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  tools/perf/tests/bpf.c      | 14 ++++----------
>  tools/perf/util/bpf-event.c | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> index 573490530194..57b9591f7cbb 100644
> --- a/tools/perf/tests/bpf.c
> +++ b/tools/perf/tests/bpf.c
> @@ -281,8 +281,8 @@ static int __test__bpf(int idx)
>
>  static int check_env(void)
>  {
> +       LIBBPF_OPTS(bpf_prog_load_opts, opts);
>         int err;
> -       unsigned int kver_int;
>         char license[] = "GPL";
>
>         struct bpf_insn insns[] = {
> @@ -290,19 +290,13 @@ static int check_env(void)
>                 BPF_EXIT_INSN(),
>         };
>
> -       err = fetch_kernel_version(&kver_int, NULL, 0);
> +       err = fetch_kernel_version(&opts.kern_version, NULL, 0);
>         if (err) {
>                 pr_debug("Unable to get kernel version\n");
>                 return err;
>         }
> -
> -/* temporarily disable libbpf deprecation warnings */
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> -       err = bpf_load_program(BPF_PROG_TYPE_KPROBE, insns,
> -                              ARRAY_SIZE(insns),
> -                              license, kver_int, NULL, 0);
> -#pragma GCC diagnostic pop
> +       err = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, license, insns,
> +                           ARRAY_SIZE(insns), &opts);
>         if (err < 0) {
>                 pr_err("Missing basic BPF support, skip this test: %s\n",
>                        strerror(errno));
> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index a517eaa51eb3..48872276c0b7 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
> @@ -33,6 +33,22 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
>         return err ? ERR_PTR(err) : btf;
>  }
>
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wunused-parameter"

perf is using __maybe_unused to mark unused args, it's cleaner than pragma

> +int __weak
> +bpf_prog_load(enum bpf_prog_type prog_type,
> +               const char *prog_name, 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
> +}
> +#pragma GCC diagnostic pop
> +
>  struct bpf_program * __weak
>  bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
>  {
> --
> 2.30.2
>

Re: [PATCH bpf-next v4 1/2] perf: stop using deprecated bpf_load_program() API

[Andrii Nakryiko]

On Wed, Jan 19, 2022 at 3:20 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Jan 19, 2022, at 3:06 PM, Christy Lee <christylee@fb.com> wrote:
> >
> > bpf_load_program() API is deprecated, remove perf's usage of the
> > deprecated function. Add a __weak function declaration for libbpf
> > version compatibility.
> >
> > Signed-off-by: Christy Lee <christylee@fb.com>
> > Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> Acked-by: Song Liu <songliubraving@fb.com>
>
> With one minor comment below.
>
> > ---
> > tools/perf/tests/bpf.c      | 14 ++++----------
> > tools/perf/util/bpf-event.c | 16 ++++++++++++++++
> > 2 files changed, 20 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> > index 573490530194..57b9591f7cbb 100644
> > --- a/tools/perf/tests/bpf.c
> > +++ b/tools/perf/tests/bpf.c
> > @@ -281,8 +281,8 @@ static int __test__bpf(int idx)
> >
> > static int check_env(void)
> > {
> > +     LIBBPF_OPTS(bpf_prog_load_opts, opts);
>
> This changes seems unnecessary.
>
> >       int err;
> > -     unsigned int kver_int;
> >       char license[] = "GPL";
> >
> >       struct bpf_insn insns[] = {
> > @@ -290,19 +290,13 @@ static int check_env(void)
> >               BPF_EXIT_INSN(),
> >       };
> >
> > -     err = fetch_kernel_version(&kver_int, NULL, 0);
> > +     err = fetch_kernel_version(&opts.kern_version, NULL, 0);

Opts are used here. Libbpf is now performing the same kern_version
extraction logic, so we might not need fetch_kernel_version() now, but
I'd still keep it to avoid unnecessary regressions.


> >       if (err) {
> >               pr_debug("Unable to get kernel version\n");
> >               return err;
> >       }
> > -
> > -/* temporarily disable libbpf deprecation warnings */
> > -#pragma GCC diagnostic push
> > -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > -     err = bpf_load_program(BPF_PROG_TYPE_KPROBE, insns,
> > -                            ARRAY_SIZE(insns),
> > -                            license, kver_int, NULL, 0);
> > -#pragma GCC diagnostic pop
> > +     err = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, license, insns,
> > +                         ARRAY_SIZE(insns), &opts);
> >       if (err < 0) {
> >               pr_err("Missing basic BPF support, skip this test: %s\n",
> >                      strerror(errno));
> > diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> > index a517eaa51eb3..48872276c0b7 100644
> > --- a/tools/perf/util/bpf-event.c
> > +++ b/tools/perf/util/bpf-event.c
> > @@ -33,6 +33,22 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
> >        return err ? ERR_PTR(err) : btf;
> > }
> >
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wunused-parameter"
> > +int __weak
> > +bpf_prog_load(enum bpf_prog_type prog_type,
> > +             const char *prog_name, 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
> > +}
> > +#pragma GCC diagnostic pop
> > +
> > struct bpf_program * __weak
> > bpf_object__next_program(const struct bpf_object *obj, struct bpf_program *prev)
> > {
> > --
> > 2.30.2
> >
>

Re: [PATCH bpf-next v4 1/2] perf: stop using deprecated bpf_load_program() API

[Andrii Nakryiko]

On Wed, Jan 19, 2022 at 3:35 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Jan 19, 2022, at 3:26 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Jan 19, 2022 at 3:20 PM Song Liu <songliubraving@fb.com> wrote:
> >>
> >>
> >>
> >>> On Jan 19, 2022, at 3:06 PM, Christy Lee <christylee@fb.com> wrote:
> >>>
> >>> bpf_load_program() API is deprecated, remove perf's usage of the
> >>> deprecated function. Add a __weak function declaration for libbpf
> >>> version compatibility.
> >>>
> >>> Signed-off-by: Christy Lee <christylee@fb.com>
> >>> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> >>
> >> Acked-by: Song Liu <songliubraving@fb.com>
> >>
> >> With one minor comment below.
> >>
> >>> ---
> >>> tools/perf/tests/bpf.c      | 14 ++++----------
> >>> tools/perf/util/bpf-event.c | 16 ++++++++++++++++
> >>> 2 files changed, 20 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> >>> index 573490530194..57b9591f7cbb 100644
> >>> --- a/tools/perf/tests/bpf.c
> >>> +++ b/tools/perf/tests/bpf.c
> >>> @@ -281,8 +281,8 @@ static int __test__bpf(int idx)
> >>>
> >>> static int check_env(void)
> >>> {
> >>> +     LIBBPF_OPTS(bpf_prog_load_opts, opts);
> >>
> >> This changes seems unnecessary.
> >>
> >>>      int err;
> >>> -     unsigned int kver_int;
> >>>      char license[] = "GPL";
> >>>
> >>>      struct bpf_insn insns[] = {
> >>> @@ -290,19 +290,13 @@ static int check_env(void)
> >>>              BPF_EXIT_INSN(),
> >>>      };
> >>>
> >>> -     err = fetch_kernel_version(&kver_int, NULL, 0);
> >>> +     err = fetch_kernel_version(&opts.kern_version, NULL, 0);
> >
> > Opts are used here. Libbpf is now performing the same kern_version
> > extraction logic, so we might not need fetch_kernel_version() now, but
> > I'd still keep it to avoid unnecessary regressions.
>
> Yeah, I noticed opts is used. But we only use opts.kern_version
> here, which is an u32. Will this be a problem if we compile perf
> against an older libbpf? (if we ever do that).

Oh, because struct bpf_prog_load_opts might not be defined? Yes, but
there will also be a problem if linking against libbpf older than v0.6
anyways (due to the bpf_prog_load() macro overload shenanigans), so I
think libbpf 0.6 is the minimum version for perf, essentially.

>
> Thanks,
> Song
>
>
>
>

Re: [PATCH bpf-next v4 0/2] perf: stop using deprecated bpf APIs

[Andrii Nakryiko]

On Wed, Jan 19, 2022 at 3:09 PM Christy Lee <christylee@fb.com> wrote:
>
> libbpf's bpf_load_program() and bpf__object_next() APIs are deprecated.
> remove perf's usage of these deprecated functions. After this patch
> set, the only remaining libbpf deprecated APIs in perf would be
> bpf_program__set_prep() and bpf_program__nth_fd().
>

Arnaldo, do you want to take this through perf tree or should we apply
this to bpf-next? If the latter, can you give your ack as well?
Thanks!

> Changelog:
> ----------
> v3 -> v4:
> * Fixed commit title
> * Added weak definition for deprecated function
>
> v2 -> v3:
> https://lore.kernel.org/all/20220106200032.3067127-1-christylee@fb.com/
>
> Patch 2/2:
> Fixed commit message to use upstream perf
>
> v1 -> v2:
> https://lore.kernel.org/all/20211216222108.110518-1-christylee@fb.com/
>
> Patch 1/2:
> Added missing commit message
>
> Patch 2/2:
> Added more details to commit message and added steps to reproduce
> original test case.
>
> Christy Lee (2):
>   perf: stop using deprecated bpf_load_program() API
>   perf: stop using deprecated bpf_object__next() API
>
>  tools/perf/tests/bpf.c       | 14 ++-----
>  tools/perf/util/bpf-event.c  | 16 ++++++++
>  tools/perf/util/bpf-loader.c | 72 +++++++++++++++++++++++++++---------
>  tools/perf/util/bpf-loader.h |  1 +
>  4 files changed, 75 insertions(+), 28 deletions(-)
>
> --
> 2.30.2

Re: [PATCH bpf-next v4 0/2] perf: stop using deprecated bpf APIs

[Arnaldo Carvalho de Melo]

Em Thu, Jan 20, 2022 at 02:58:29PM -0800, Andrii Nakryiko escreveu:
> On Wed, Jan 19, 2022 at 3:09 PM Christy Lee <christylee@fb.com> wrote:
> >
> > libbpf's bpf_load_program() and bpf__object_next() APIs are deprecated.
> > remove perf's usage of these deprecated functions. After this patch
> > set, the only remaining libbpf deprecated APIs in perf would be
> > bpf_program__set_prep() and bpf_program__nth_fd().
> >
> 
> Arnaldo, do you want to take this through perf tree or should we apply
> this to bpf-next? If the latter, can you give your ack as well?
> Thanks!

I'd love to be able to test it with the containers, after I push the
current batch to Linus.

- Arnaldo
 
> > Changelog:
> > ----------
> > v3 -> v4:
> > * Fixed commit title
> > * Added weak definition for deprecated function
> >
> > v2 -> v3:
> > https://lore.kernel.org/all/20220106200032.3067127-1-christylee@fb.com/
> >
> > Patch 2/2:
> > Fixed commit message to use upstream perf
> >
> > v1 -> v2:
> > https://lore.kernel.org/all/20211216222108.110518-1-christylee@fb.com/
> >
> > Patch 1/2:
> > Added missing commit message
> >
> > Patch 2/2:
> > Added more details to commit message and added steps to reproduce
> > original test case.
> >
> > Christy Lee (2):
> >   perf: stop using deprecated bpf_load_program() API
> >   perf: stop using deprecated bpf_object__next() API
> >
> >  tools/perf/tests/bpf.c       | 14 ++-----
> >  tools/perf/util/bpf-event.c  | 16 ++++++++
> >  tools/perf/util/bpf-loader.c | 72 +++++++++++++++++++++++++++---------
> >  tools/perf/util/bpf-loader.h |  1 +
> >  4 files changed, 75 insertions(+), 28 deletions(-)
> >
> > --
> > 2.30.2

-- 

- Arnaldo

Re: [PATCH bpf-next v4 0/2] perf: stop using deprecated bpf APIs

[Arnaldo Carvalho de Melo]

Em Sat, Jan 22, 2022 at 05:01:50PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jan 20, 2022 at 02:58:29PM -0800, Andrii Nakryiko escreveu:
> > On Wed, Jan 19, 2022 at 3:09 PM Christy Lee <christylee@fb.com> wrote:
> > >
> > > libbpf's bpf_load_program() and bpf__object_next() APIs are deprecated.
> > > remove perf's usage of these deprecated functions. After this patch
> > > set, the only remaining libbpf deprecated APIs in perf would be
> > > bpf_program__set_prep() and bpf_program__nth_fd().
> > >
> > 
> > Arnaldo, do you want to take this through perf tree or should we apply
> > this to bpf-next? If the latter, can you give your ack as well?
> > Thanks!
> 
> I'd love to be able to test it with the containers, after I push the
> current batch to Linus.

I just looked at it, simple enough, applied.

- Arnaldo
 
> - Arnaldo
>  
> > > Changelog:
> > > ----------
> > > v3 -> v4:
> > > * Fixed commit title
> > > * Added weak definition for deprecated function
> > >
> > > v2 -> v3:
> > > https://lore.kernel.org/all/20220106200032.3067127-1-christylee@fb.com/
> > >
> > > Patch 2/2:
> > > Fixed commit message to use upstream perf
> > >
> > > v1 -> v2:
> > > https://lore.kernel.org/all/20211216222108.110518-1-christylee@fb.com/
> > >
> > > Patch 1/2:
> > > Added missing commit message
> > >
> > > Patch 2/2:
> > > Added more details to commit message and added steps to reproduce
> > > original test case.
> > >
> > > Christy Lee (2):
> > >   perf: stop using deprecated bpf_load_program() API
> > >   perf: stop using deprecated bpf_object__next() API
> > >
> > >  tools/perf/tests/bpf.c       | 14 ++-----
> > >  tools/perf/util/bpf-event.c  | 16 ++++++++
> > >  tools/perf/util/bpf-loader.c | 72 +++++++++++++++++++++++++++---------
> > >  tools/perf/util/bpf-loader.h |  1 +
> > >  4 files changed, 75 insertions(+), 28 deletions(-)
> > >
> > > --
> > > 2.30.2
> 
> -- 
> 
> - Arnaldo

-- 

- Arnaldo

Re: [PATCH bpf-next v4 0/2] perf: stop using deprecated bpf APIs

[Arnaldo Carvalho de Melo]

Em Sat, Jan 22, 2022 at 05:38:56PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Sat, Jan 22, 2022 at 05:01:50PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jan 20, 2022 at 02:58:29PM -0800, Andrii Nakryiko escreveu:
> > > On Wed, Jan 19, 2022 at 3:09 PM Christy Lee <christylee@fb.com> wrote:
> > > >
> > > > libbpf's bpf_load_program() and bpf__object_next() APIs are deprecated.
> > > > remove perf's usage of these deprecated functions. After this patch
> > > > set, the only remaining libbpf deprecated APIs in perf would be
> > > > bpf_program__set_prep() and bpf_program__nth_fd().
> > > >
> > > 
> > > Arnaldo, do you want to take this through perf tree or should we apply
> > > this to bpf-next? If the latter, can you give your ack as well?
> > > Thanks!
> > 
> > I'd love to be able to test it with the containers, after I push the
> > current batch to Linus.
> 
> I just looked at it, simple enough, applied.

I take that back, it breaks a BPF test case in 'perf test'

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

And it leaves the test probe in place:

[root@quaco ~]# perf probe -l
  perf_bpf_probe:func  (on do_epoll_wait@fs/eventpoll.c)
[root@quaco ~]#

So I have to remove it manually:

[root@quaco ~]# perf probe -d perf_bpf_probe:func
Removed event: perf_bpf_probe:func
[root@quaco ~]#

Then revert this patch and try again, back working:

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


With the patch, you can get a verbose output, I'll continue this later
if noone fixes it before:

[root@quaco ~]# perf test -v 42
 42: BPF filter                                                      :
 42.1: Basic BPF filtering                                           :
--- start ---
test child forked, pid 1344304
Kernel build dir is set to /lib/modules/5.15.7-200.fc35.x86_64/build
set env: KBUILD_DIR=/lib/modules/5.15.7-200.fc35.x86_64/build
unset env: KBUILD_OPTS
include option is set to -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/11/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h 
set env: NR_CPUS=8
set env: LINUX_VERSION_CODE=0x50f07
set env: CLANG_EXEC=/usr/lib64/ccache/clang
set env: CLANG_OPTIONS=-xc -g
set env: KERNEL_INC_OPTIONS=-nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/11/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h 
set env: PERF_BPF_INC_OPTIONS=-I/home/acme/lib/perf/include/bpf
set env: WORKING_DIR=/lib/modules/5.15.7-200.fc35.x86_64/build
set env: CLANG_SOURCE=-
llvm compiling command template: echo '// SPDX-License-Identifier: GPL-2.0
/*
 * bpf-script-example.c
 * Test basic LLVM building
 */
#ifndef LINUX_VERSION_CODE
# error Need LINUX_VERSION_CODE
# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
#endif
#define BPF_ANY 0
#define BPF_MAP_TYPE_ARRAY 2
#define BPF_FUNC_map_lookup_elem 1
#define BPF_FUNC_map_update_elem 2

static void *(*bpf_map_lookup_elem)(void *map, void *key) =
	(void *) BPF_FUNC_map_lookup_elem;
static void *(*bpf_map_update_elem)(void *map, void *key, void *value, int flags) =
	(void *) BPF_FUNC_map_update_elem;

struct bpf_map_def {
	unsigned int type;
	unsigned int key_size;
	unsigned int value_size;
	unsigned int max_entries;
};

#define SEC(NAME) __attribute__((section(NAME), used))
struct bpf_map_def SEC("maps") flip_table = {
	.type = BPF_MAP_TYPE_ARRAY,
	.key_size = sizeof(int),
	.value_size = sizeof(int),
	.max_entries = 1,
};

SEC("func=do_epoll_wait")
int bpf_func__SyS_epoll_pwait(void *ctx)
{
	int ind =0;
	int *flag = bpf_map_lookup_elem(&flip_table, &ind);
	int new_flag;
	if (!flag)
		return 0;
	/* flip flag and store back */
	new_flag = !*flag;
	bpf_map_update_elem(&flip_table, &ind, &new_flag, BPF_ANY);
	return new_flag;
}
char _license[] SEC("license") = "GPL";
int _version SEC("version") = LINUX_VERSION_CODE;
' | $CLANG_EXEC -D__KERNEL__ -D__NR_CPUS__=$NR_CPUS -DLINUX_VERSION_CODE=$LINUX_VERSION_CODE $CLANG_OPTIONS $PERF_BPF_INC_OPTIONS $KERNEL_INC_OPTIONS -Wno-unused-value -Wno-pointer-sign -working-directory $WORKING_DIR -c "$CLANG_SOURCE" -target bpf $CLANG_EMIT_LLVM -O2 -o - $LLVM_OPTIONS_PIPE
llvm compiling command : echo '// SPDX-License-Identifier: GPL-2.0
/*
 * bpf-script-example.c
 * Test basic LLVM building
 */
#ifndef LINUX_VERSION_CODE
# error Need LINUX_VERSION_CODE
# error Example: for 4.2 kernel, put 'clang-opt=-DLINUX_VERSION_CODE=0x40200 into llvm section of ~/.perfconfig'
#endif
#define BPF_ANY 0
#define BPF_MAP_TYPE_ARRAY 2
#define BPF_FUNC_map_lookup_elem 1
#define BPF_FUNC_map_update_elem 2

static void *(*bpf_map_lookup_elem)(void *map, void *key) =
	(void *) BPF_FUNC_map_lookup_elem;
static void *(*bpf_map_update_elem)(void *map, void *key, void *value, int flags) =
	(void *) BPF_FUNC_map_update_elem;

struct bpf_map_def {
	unsigned int type;
	unsigned int key_size;
	unsigned int value_size;
	unsigned int max_entries;
};

#define SEC(NAME) __attribute__((section(NAME), used))
struct bpf_map_def SEC(maps) flip_table = {
	.type = BPF_MAP_TYPE_ARRAY,
	.key_size = sizeof(int),
	.value_size = sizeof(int),
	.max_entries = 1,
};

SEC(func=do_epoll_wait)
int bpf_func__SyS_epoll_pwait(void *ctx)
{
	int ind =0;
	int *flag = bpf_map_lookup_elem(&flip_table, &ind);
	int new_flag;
	if (!flag)
		return 0;
	/* flip flag and store back */
	new_flag = !*flag;
	bpf_map_update_elem(&flip_table, &ind, &new_flag, BPF_ANY);
	return new_flag;
}
char _license[] SEC(license) = GPL;
int _version SEC(version) = LINUX_VERSION_CODE;
' | /usr/lib64/ccache/clang -D__KERNEL__ -D__NR_CPUS__=8 -DLINUX_VERSION_CODE=0x50f07 -xc -g -I/home/acme/lib/perf/include/bpf -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/11/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h  -Wno-unused-value -Wno-pointer-sign -working-directory /lib/modules/5.15.7-200.fc35.x86_64/build -c - -target bpf  -O2 -o - 
libbpf: loading object '[basic_bpf_test]' from buffer
libbpf: elf: section(3) func=do_epoll_wait, size 192, link 0, flags 6, type=1
libbpf: sec 'func=do_epoll_wait': found program 'bpf_func__SyS_epoll_pwait' at insn offset 0 (0 bytes), code size 24 insns (192 bytes)
libbpf: elf: section(4) .relfunc=do_epoll_wait, size 32, link 22, flags 0, type=9
libbpf: elf: section(5) maps, size 16, link 0, flags 3, type=1
libbpf: elf: section(6) license, size 4, link 0, flags 3, type=1
libbpf: license of [basic_bpf_test] is GPL
libbpf: elf: section(7) version, size 4, link 0, flags 3, type=1
libbpf: kernel version of [basic_bpf_test] is 50f07
libbpf: elf: section(13) .BTF, size 575, link 0, flags 0, type=1
libbpf: elf: section(15) .BTF.ext, size 256, link 0, flags 0, type=1
libbpf: elf: section(22) .symtab, size 336, link 1, flags 0, type=2
libbpf: looking for externs among 14 symbols...
libbpf: collected 0 externs total
libbpf: elf: found 1 legacy map definitions (16 bytes) in [basic_bpf_test]
libbpf: map 'flip_table' (legacy): at sec_idx 5, offset 0.
libbpf: map 11 is "flip_table"
libbpf: prog 'bpf_func__SyS_epoll_pwait': unrecognized ELF section name 'func=do_epoll_wait'
libbpf: sec '.relfunc=do_epoll_wait': collecting relocation for section(3) 'func=do_epoll_wait'
libbpf: sec '.relfunc=do_epoll_wait': relo #0: insn #4 against 'flip_table'
libbpf: prog 'bpf_func__SyS_epoll_pwait': found map 0 (flip_table, sec 5, off 0) for insn #4
libbpf: sec '.relfunc=do_epoll_wait': relo #1: insn #17 against 'flip_table'
libbpf: prog 'bpf_func__SyS_epoll_pwait': found map 0 (flip_table, sec 5, off 0) for insn #17
bpf: config program 'func=do_epoll_wait'
symbol:do_epoll_wait file:(null) line:0 offset:0 return:0 lazy:(null)
bpf: config 'func=do_epoll_wait' is ok
Looking at the vmlinux_path (8 entries long)
symsrc__init: build id mismatch for vmlinux.
Using /usr/lib/debug/lib/modules/5.15.7-200.fc35.x86_64/vmlinux for symbols
Open Debuginfo file: /usr/lib/debug/.build-id/cb/36dde59d9c2c72bf9dbb845549e6073181579d.debug
Try to find probe point from debuginfo.
Matched function: do_epoll_wait [3917119]
Probe point found: do_epoll_wait+0
Found 1 probe_trace_events.
Opening /sys/kernel/tracing//kprobe_events write=1
Opening /sys/kernel/tracing//README write=0
Writing event: p:perf_bpf_probe/func _text+3829568
libbpf: map:flip_table container_name:____btf_map_flip_table cannot be found in BTF. Missing BPF_ANNOTATE_KV_PAIR?
libbpf: map 'flip_table': created successfully, fd=4
add bpf event perf_bpf_probe:func and attach bpf program 5
adding perf_bpf_probe:func
adding perf_bpf_probe:func to 0x2f5f630
Using CPUID GenuineIntel-6-8E-A
mmap size 1052672B
test child finished with 0
---- end ----
BPF filter subtest 1: Ok
 42.2: BPF pinning                                                   :
--- start ---
test child forked, pid 1344478
Kernel build dir is set to /lib/modules/5.15.7-200.fc35.x86_64/build
set env: KBUILD_DIR=/lib/modules/5.15.7-200.fc35.x86_64/build
unset env: KBUILD_OPTS
include option is set to -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/11/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h 
set env: NR_CPUS=8
set env: LINUX_VERSION_CODE=0x50f07
set env: CLANG_EXEC=/usr/lib64/ccache/clang
set env: CLANG_OPTIONS=-xc -g
set env: KERNEL_INC_OPTIONS=-nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/11/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h 
set env: PERF_BPF_INC_OPTIONS=-I/home/acme/lib/perf/include/bpf
set env: WORKING_DIR=/lib/modules/5.15.7-200.fc35.x86_64/build
set env: CLANG_SOURCE=-
llvm compiling command template: echo '// SPDX-License-Identifier: GPL-2.0
/*
 * bpf-script-example.c
 * Test basic LLVM building
 */
#ifndef LINUX_VERSION_CODE
# error Need LINUX_VERSION_CODE
# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
#endif
#define BPF_ANY 0
#define BPF_MAP_TYPE_ARRAY 2
#define BPF_FUNC_map_lookup_elem 1
#define BPF_FUNC_map_update_elem 2

static void *(*bpf_map_lookup_elem)(void *map, void *key) =
	(void *) BPF_FUNC_map_lookup_elem;
static void *(*bpf_map_update_elem)(void *map, void *key, void *value, int flags) =
	(void *) BPF_FUNC_map_update_elem;

struct bpf_map_def {
	unsigned int type;
	unsigned int key_size;
	unsigned int value_size;
	unsigned int max_entries;
};

#define SEC(NAME) __attribute__((section(NAME), used))
struct bpf_map_def SEC("maps") flip_table = {
	.type = BPF_MAP_TYPE_ARRAY,
	.key_size = sizeof(int),
	.value_size = sizeof(int),
	.max_entries = 1,
};

SEC("func=do_epoll_wait")
int bpf_func__SyS_epoll_pwait(void *ctx)
{
	int ind =0;
	int *flag = bpf_map_lookup_elem(&flip_table, &ind);
	int new_flag;
	if (!flag)
		return 0;
	/* flip flag and store back */
	new_flag = !*flag;
	bpf_map_update_elem(&flip_table, &ind, &new_flag, BPF_ANY);
	return new_flag;
}
char _license[] SEC("license") = "GPL";
int _version SEC("version") = LINUX_VERSION_CODE;
' | $CLANG_EXEC -D__KERNEL__ -D__NR_CPUS__=$NR_CPUS -DLINUX_VERSION_CODE=$LINUX_VERSION_CODE $CLANG_OPTIONS $PERF_BPF_INC_OPTIONS $KERNEL_INC_OPTIONS -Wno-unused-value -Wno-pointer-sign -working-directory $WORKING_DIR -c "$CLANG_SOURCE" -target bpf $CLANG_EMIT_LLVM -O2 -o - $LLVM_OPTIONS_PIPE
llvm compiling command : echo '// SPDX-License-Identifier: GPL-2.0
/*
 * bpf-script-example.c
 * Test basic LLVM building
 */
#ifndef LINUX_VERSION_CODE
# error Need LINUX_VERSION_CODE
# error Example: for 4.2 kernel, put 'clang-opt=-DLINUX_VERSION_CODE=0x40200 into llvm section of ~/.perfconfig'
#endif
#define BPF_ANY 0
#define BPF_MAP_TYPE_ARRAY 2
#define BPF_FUNC_map_lookup_elem 1
#define BPF_FUNC_map_update_elem 2

static void *(*bpf_map_lookup_elem)(void *map, void *key) =
	(void *) BPF_FUNC_map_lookup_elem;
static void *(*bpf_map_update_elem)(void *map, void *key, void *value, int flags) =
	(void *) BPF_FUNC_map_update_elem;

struct bpf_map_def {
	unsigned int type;
	unsigned int key_size;
	unsigned int value_size;
	unsigned int max_entries;
};

#define SEC(NAME) __attribute__((section(NAME), used))
struct bpf_map_def SEC(maps) flip_table = {
	.type = BPF_MAP_TYPE_ARRAY,
	.key_size = sizeof(int),
	.value_size = sizeof(int),
	.max_entries = 1,
};

SEC(func=do_epoll_wait)
int bpf_func__SyS_epoll_pwait(void *ctx)
{
	int ind =0;
	int *flag = bpf_map_lookup_elem(&flip_table, &ind);
	int new_flag;
	if (!flag)
		return 0;
	/* flip flag and store back */
	new_flag = !*flag;
	bpf_map_update_elem(&flip_table, &ind, &new_flag, BPF_ANY);
	return new_flag;
}
char _license[] SEC(license) = GPL;
int _version SEC(version) = LINUX_VERSION_CODE;
' | /usr/lib64/ccache/clang -D__KERNEL__ -D__NR_CPUS__=8 -DLINUX_VERSION_CODE=0x50f07 -xc -g -I/home/acme/lib/perf/include/bpf -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/11/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h  -Wno-unused-value -Wno-pointer-sign -working-directory /lib/modules/5.15.7-200.fc35.x86_64/build -c - -target bpf  -O2 -o - 
libbpf: loading object '[bpf_pinning]' from buffer
libbpf: elf: section(3) func=do_epoll_wait, size 192, link 0, flags 6, type=1
libbpf: sec 'func=do_epoll_wait': found program 'bpf_func__SyS_epoll_pwait' at insn offset 0 (0 bytes), code size 24 insns (192 bytes)
libbpf: elf: section(4) .relfunc=do_epoll_wait, size 32, link 22, flags 0, type=9
libbpf: elf: section(5) maps, size 16, link 0, flags 3, type=1
libbpf: elf: section(6) license, size 4, link 0, flags 3, type=1
libbpf: license of [bpf_pinning] is GPL
libbpf: elf: section(7) version, size 4, link 0, flags 3, type=1
libbpf: kernel version of [bpf_pinning] is 50f07
libbpf: elf: section(13) .BTF, size 575, link 0, flags 0, type=1
libbpf: elf: section(15) .BTF.ext, size 256, link 0, flags 0, type=1
libbpf: elf: section(22) .symtab, size 336, link 1, flags 0, type=2
libbpf: looking for externs among 14 symbols...
libbpf: collected 0 externs total
libbpf: elf: found 1 legacy map definitions (16 bytes) in [bpf_pinning]
libbpf: map 'flip_table' (legacy): at sec_idx 5, offset 0.
libbpf: map 11 is "flip_table"
libbpf: prog 'bpf_func__SyS_epoll_pwait': unrecognized ELF section name 'func=do_epoll_wait'
libbpf: sec '.relfunc=do_epoll_wait': collecting relocation for section(3) 'func=do_epoll_wait'
libbpf: sec '.relfunc=do_epoll_wait': relo #0: insn #4 against 'flip_table'
libbpf: prog 'bpf_func__SyS_epoll_pwait': found map 0 (flip_table, sec 5, off 0) for insn #4
libbpf: sec '.relfunc=do_epoll_wait': relo #1: insn #17 against 'flip_table'
libbpf: prog 'bpf_func__SyS_epoll_pwait': found map 0 (flip_table, sec 5, off 0) for insn #17
bpf: config program 'func=do_epoll_wait'
symbol:do_epoll_wait file:(null) line:0 offset:0 return:0 lazy:(null)
bpf: config 'func=do_epoll_wait' is ok
Looking at the vmlinux_path (8 entries long)
symsrc__init: build id mismatch for vmlinux.
Using /usr/lib/debug/lib/modules/5.15.7-200.fc35.x86_64/vmlinux for symbols
Open Debuginfo file: /usr/lib/debug/.build-id/cb/36dde59d9c2c72bf9dbb845549e6073181579d.debug
Try to find probe point from debuginfo.
Matched function: do_epoll_wait [3917119]
Probe point found: do_epoll_wait+0
Found 1 probe_trace_events.
Opening /sys/kernel/tracing//kprobe_events write=1
Parsing probe_events: p:perf_bpf_probe/func _text+3829568
Group:perf_bpf_probe Event:func probe:p
Error: event "func" already exists.
 Hint: Remove existing event by 'perf probe -d'
       or force duplicates by 'perf probe -f'
       or set 'force=yes' in BPF source.
bpf_probe: failed to apply perf probe events
Failed to add events selected by BPF
test child finished with -1
---- end ----
BPF filter subtest 2: FAILED!
 42.3: BPF prologue generation                                       :
--- start ---
test child forked, pid 1344652
Kernel build dir is set to /lib/modules/5.15.7-200.fc35.x86_64/build
set env: KBUILD_DIR=/lib/modules/5.15.7-200.fc35.x86_64/build
unset env: KBUILD_OPTS
include option is set to -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/11/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h 
set env: NR_CPUS=8
set env: LINUX_VERSION_CODE=0x50f07
set env: CLANG_EXEC=/usr/lib64/ccache/clang
set env: CLANG_OPTIONS=-xc -g
set env: KERNEL_INC_OPTIONS=-nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/11/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h 
set env: PERF_BPF_INC_OPTIONS=-I/home/acme/lib/perf/include/bpf
set env: WORKING_DIR=/lib/modules/5.15.7-200.fc35.x86_64/build
set env: CLANG_SOURCE=-
llvm compiling command template: echo '// SPDX-License-Identifier: GPL-2.0
/*
 * bpf-script-test-prologue.c
 * Test BPF prologue
 */
#ifndef LINUX_VERSION_CODE
# error Need LINUX_VERSION_CODE
# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
#endif
#define SEC(NAME) __attribute__((section(NAME), used))

#include <uapi/linux/fs.h>

/*
 * If CONFIG_PROFILE_ALL_BRANCHES is selected,
 * 'if' is redefined after include kernel header.
 * Recover 'if' for BPF object code.
 */
#ifdef if
# undef if
#endif

#define FMODE_READ		0x1
#define FMODE_WRITE		0x2

static void (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
	(void *) 6;

SEC("func=null_lseek file->f_mode offset orig")
int bpf_func__null_lseek(void *ctx, int err, unsigned long _f_mode,
			 unsigned long offset, unsigned long orig)
{
	fmode_t f_mode = (fmode_t)_f_mode;

	if (err)
		return 0;
	if (f_mode & FMODE_WRITE)
		return 0;
	if (offset & 1)
		return 0;
	if (orig == SEEK_CUR)
		return 0;
	return 1;
}

char _license[] SEC("license") = "GPL";
int _version SEC("version") = LINUX_VERSION_CODE;
' | $CLANG_EXEC -D__KERNEL__ -D__NR_CPUS__=$NR_CPUS -DLINUX_VERSION_CODE=$LINUX_VERSION_CODE $CLANG_OPTIONS $PERF_BPF_INC_OPTIONS $KERNEL_INC_OPTIONS -Wno-unused-value -Wno-pointer-sign -working-directory $WORKING_DIR -c "$CLANG_SOURCE" -target bpf $CLANG_EMIT_LLVM -O2 -o - $LLVM_OPTIONS_PIPE
llvm compiling command : 
libbpf: loading object '[bpf_prologue_test]' from buffer
libbpf: elf: section(3) func=null_lseek file->f_mode offset orig, size 112, link 0, flags 6, type=1
libbpf: sec 'func=null_lseek file->f_mode offset orig': found program 'bpf_func__null_lseek' at insn offset 0 (0 bytes), code size 14 insns (112 bytes)
libbpf: elf: section(4) license, size 4, link 0, flags 3, type=1
libbpf: license of [bpf_prologue_test] is GPL
libbpf: elf: section(5) version, size 4, link 0, flags 3, type=1
libbpf: kernel version of [bpf_prologue_test] is 50f07
libbpf: elf: section(11) .BTF, size 488, link 0, flags 0, type=1
libbpf: elf: section(13) .BTF.ext, size 144, link 0, flags 0, type=1
libbpf: elf: section(20) .symtab, size 312, link 1, flags 0, type=2
libbpf: looking for externs among 13 symbols...
libbpf: collected 0 externs total
libbpf: prog 'bpf_func__null_lseek': unrecognized ELF section name 'func=null_lseek file->f_mode offset orig'
bpf: config program 'func=null_lseek file->f_mode offset orig'
symbol:null_lseek file:(null) line:0 offset:0 return:0 lazy:(null)
parsing arg: file->f_mode into file, f_mode(1)
parsing arg: offset into offset
parsing arg: orig into orig
bpf: config 'func=null_lseek file->f_mode offset orig' is ok
Looking at the vmlinux_path (8 entries long)
symsrc__init: build id mismatch for vmlinux.
Using /usr/lib/debug/lib/modules/5.15.7-200.fc35.x86_64/vmlinux for symbols
Open Debuginfo file: /usr/lib/debug/.build-id/cb/36dde59d9c2c72bf9dbb845549e6073181579d.debug
Try to find probe point from debuginfo.
Opening /sys/kernel/tracing//README write=0
Matched function: null_lseek [763cb6b]
Probe point found: null_lseek+0
Searching 'file' variable in context.
Converting variable file into trace event.
converting f_mode in file
f_mode type is unsigned int.
Searching 'offset' variable in context.
Converting variable offset into trace event.
offset type is long long int.
Searching 'orig' variable in context.
Converting variable orig into trace event.
orig type is int.
Found 1 probe_trace_events.
Opening /sys/kernel/tracing//kprobe_events write=1
Parsing probe_events: p:perf_bpf_probe/func _text+3829568
Group:perf_bpf_probe Event:func probe:p
Error: event "func" already exists.
 Hint: Remove existing event by 'perf probe -d'
       or force duplicates by 'perf probe -f'
       or set 'force=yes' in BPF source.
bpf_probe: failed to apply perf probe events
Failed to add events selected by BPF
test child finished with -1
---- end ----
BPF filter subtest 3: FAILED!
[root@quaco ~]# 

Re: [PATCH bpf-next v4 2/2] perf: stop using deprecated bpf_object__next() API

[Jiri Olsa]

On Wed, Jan 19, 2022 at 03:06:36PM -0800, Christy Lee wrote:

SNIP

> ---
>  tools/perf/util/bpf-loader.c | 72 +++++++++++++++++++++++++++---------
>  tools/perf/util/bpf-loader.h |  1 +
>  2 files changed, 55 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 4631cac3957f..b1822f8af2bb 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -29,9 +29,6 @@
>  
>  #include <internal/xyarray.h>
>  
> -/* temporarily disable libbpf deprecation warnings */
> -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> -
>  static int libbpf_perf_print(enum libbpf_print_level level __attribute__((unused)),
>  			      const char *fmt, va_list args)
>  {
> @@ -49,6 +46,36 @@ struct bpf_prog_priv {
>  	int *type_mapping;
>  };
>  
> +struct bpf_perf_object {
> +	struct bpf_object *obj;
> +	struct list_head list;
> +};
> +
> +static LIST_HEAD(bpf_objects_list);

hum.. I still can't see any code adding/removing bpf_perf_object
objects to this list, and that's why the code is failing to remove
probes

because there are no objects to iterate on, so added probes stay
configured and screw following tests

you need something like below to add and del objects from
bpf_objects_list list

it also simplifies for_each macros to work just over perf_obj,
because I wasn't patient enough to make it work with the extra
bpf_object ;-) I don't mind if you fix that, but this way looks
simpler to me

tests are working for me with this fix, please feel free to
squash it into your change

thanks,
jirka


---
diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index 57b9591f7cbb..d09d25707f1e 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -210,6 +210,11 @@ prepare_bpf(void *obj_buf, size_t obj_buf_sz, const char *name)
 		pr_debug("Compile BPF program failed.\n");
 		return NULL;
 	}
+
+	if (bpf_perf_object__add(obj)) {
+		bpf_object__close(obj);
+		return NULL;
+	}
 	return obj;
 }
 
diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 89e584ac267c..a7a8ad32c847 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -70,11 +70,11 @@ struct bpf_perf_object *bpf_perf_object__next(struct bpf_perf_object *prev)
 	return next;
 }
 
-#define bpf_perf_object__for_each(perf_obj, tmp, obj)                          \
-	for ((perf_obj) = bpf_perf_object__next(NULL),                         \
-	    (tmp) = bpf_perf_object__next(perf_obj), (obj) = NULL;             \
-	     (perf_obj) != NULL; (perf_obj) = (tmp),                           \
-	    (tmp) = bpf_perf_object__next(tmp), (obj) = (perf_obj)->obj)
+#define bpf_perf_object__for_each(perf_obj, tmp)         \
+	for ((perf_obj) = bpf_perf_object__next(NULL),   \
+	     (tmp) = bpf_perf_object__next(perf_obj);    \
+	     (perf_obj) != NULL; (perf_obj) = (tmp),     \
+	    (tmp) = bpf_perf_object__next(tmp) )
 
 static bool libbpf_initialized;
 
@@ -97,6 +97,24 @@ bpf__prepare_load_buffer(void *obj_buf, size_t obj_buf_sz, const char *name)
 	return obj;
 }
 
+int bpf_perf_object__add(struct bpf_object *obj)
+{
+	struct bpf_perf_object *perf_obj = zalloc(sizeof(*perf_obj));
+
+	if (perf_obj) {
+		perf_obj->obj = obj;
+		list_add_tail(&perf_obj->list, &bpf_objects_list);
+	}
+	return perf_obj ? 0 : -ENOMEM;
+}
+
+static void bpf_perf_object__close(struct bpf_perf_object *perf_obj)
+{
+	list_del(&perf_obj->list);
+	bpf_object__close(perf_obj->obj);
+	free(perf_obj);
+}
+
 struct bpf_object *bpf__prepare_load(const char *filename, bool source)
 {
 	struct bpf_object *obj;
@@ -135,17 +153,20 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
 		return obj;
 	}
 
+	if (bpf_perf_object__add(obj)) {
+		bpf_object__close(obj);
+		return ERR_PTR(-BPF_LOADER_ERRNO__COMPILE);
+	}
 	return obj;
 }
 
 void bpf__clear(void)
 {
 	struct bpf_perf_object *perf_obj, *tmp;
-	struct bpf_object *obj;
 
-	bpf_perf_object__for_each(perf_obj, tmp, obj) {
-		bpf__unprobe(obj);
-		bpf_object__close(obj);
+	bpf_perf_object__for_each(perf_obj, tmp) {
+		bpf__unprobe(perf_obj->obj);
+		bpf_perf_object__close(perf_obj);
 	}
 }
 
@@ -1538,11 +1559,10 @@ apply_obj_config_object(struct bpf_object *obj)
 int bpf__apply_obj_config(void)
 {
 	struct bpf_perf_object *perf_obj, *tmp;
-	struct bpf_object *obj;
 	int err;
 
-	bpf_perf_object__for_each(perf_obj, tmp, obj) {
-		err = apply_obj_config_object(obj);
+	bpf_perf_object__for_each(perf_obj, tmp) {
+		err = apply_obj_config_object(perf_obj->obj);
 		if (err)
 			return err;
 	}
@@ -1550,25 +1570,24 @@ int bpf__apply_obj_config(void)
 	return 0;
 }
 
-#define bpf__perf_for_each_map(perf_obj, tmp, obj, map)                        \
-	bpf_perf_object__for_each(perf_obj, tmp, obj)                          \
-		bpf_object__for_each_map(map, obj)
+#define bpf__perf_for_each_map(perf_obj, tmp, map)                   \
+	bpf_perf_object__for_each(perf_obj, tmp)                     \
+		bpf_object__for_each_map(map, perf_obj->obj)
 
-#define bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name)            \
-	bpf__perf_for_each_map(perf_obj, tmp, obj, map)                        \
+#define bpf__perf_for_each_map_named(perf_obj, tmp, map, name)            \
+	bpf__perf_for_each_map(perf_obj, tmp, map)                        \
 		if (bpf_map__name(map) && (strcmp(name, bpf_map__name(map)) == 0))
 
 struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
 {
 	struct bpf_map_priv *tmpl_priv = NULL;
 	struct bpf_perf_object *perf_obj, *tmp;
-	struct bpf_object *obj;
 	struct evsel *evsel = NULL;
 	struct bpf_map *map;
 	int err;
 	bool need_init = false;
 
-	bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name) {
+	bpf__perf_for_each_map_named(perf_obj, tmp, map, name) {
 		struct bpf_map_priv *priv = bpf_map__priv(map);
 
 		if (IS_ERR(priv))
@@ -1604,7 +1623,7 @@ struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
 		evsel = evlist__last(evlist);
 	}
 
-	bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name) {
+	bpf__perf_for_each_map_named(perf_obj, tmp, map, name) {
 		struct bpf_map_priv *priv = bpf_map__priv(map);
 
 		if (IS_ERR(priv))
diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
index 95262b7e936f..78c7d3662910 100644
--- a/tools/perf/util/bpf-loader.h
+++ b/tools/perf/util/bpf-loader.h
@@ -83,6 +83,8 @@ int bpf__strerror_config_obj(struct bpf_object *obj,
 int bpf__apply_obj_config(void);
 int bpf__strerror_apply_obj_config(int err, char *buf, size_t size);
 
+int bpf_perf_object__add(struct bpf_object *obj);
+
 int bpf__setup_stdout(struct evlist *evlist);
 struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name);
 int bpf__strerror_setup_output_event(struct evlist *evlist, int err, char *buf, size_t size);

Re: [PATCH bpf-next v4 2/2] perf: stop using deprecated bpf_object__next() API

[Jiri Olsa]

hi,
just checking, do you plan to send new version for this?

thanks,
jirka

On Sun, Jan 23, 2022 at 06:06:08PM +0100, Jiri Olsa wrote:
> On Wed, Jan 19, 2022 at 03:06:36PM -0800, Christy Lee wrote:
> 
> SNIP
> 
> > ---
> >  tools/perf/util/bpf-loader.c | 72 +++++++++++++++++++++++++++---------
> >  tools/perf/util/bpf-loader.h |  1 +
> >  2 files changed, 55 insertions(+), 18 deletions(-)
> > 
> > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> > index 4631cac3957f..b1822f8af2bb 100644
> > --- a/tools/perf/util/bpf-loader.c
> > +++ b/tools/perf/util/bpf-loader.c
> > @@ -29,9 +29,6 @@
> >  
> >  #include <internal/xyarray.h>
> >  
> > -/* temporarily disable libbpf deprecation warnings */
> > -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > -
> >  static int libbpf_perf_print(enum libbpf_print_level level __attribute__((unused)),
> >  			      const char *fmt, va_list args)
> >  {
> > @@ -49,6 +46,36 @@ struct bpf_prog_priv {
> >  	int *type_mapping;
> >  };
> >  
> > +struct bpf_perf_object {
> > +	struct bpf_object *obj;
> > +	struct list_head list;
> > +};
> > +
> > +static LIST_HEAD(bpf_objects_list);
> 
> hum.. I still can't see any code adding/removing bpf_perf_object
> objects to this list, and that's why the code is failing to remove
> probes
> 
> because there are no objects to iterate on, so added probes stay
> configured and screw following tests
> 
> you need something like below to add and del objects from
> bpf_objects_list list
> 
> it also simplifies for_each macros to work just over perf_obj,
> because I wasn't patient enough to make it work with the extra
> bpf_object ;-) I don't mind if you fix that, but this way looks
> simpler to me
> 
> tests are working for me with this fix, please feel free to
> squash it into your change
> 
> thanks,
> jirka
> 
> 
> ---
> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> index 57b9591f7cbb..d09d25707f1e 100644
> --- a/tools/perf/tests/bpf.c
> +++ b/tools/perf/tests/bpf.c
> @@ -210,6 +210,11 @@ prepare_bpf(void *obj_buf, size_t obj_buf_sz, const char *name)
>  		pr_debug("Compile BPF program failed.\n");
>  		return NULL;
>  	}
> +
> +	if (bpf_perf_object__add(obj)) {
> +		bpf_object__close(obj);
> +		return NULL;
> +	}
>  	return obj;
>  }
>  
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 89e584ac267c..a7a8ad32c847 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -70,11 +70,11 @@ struct bpf_perf_object *bpf_perf_object__next(struct bpf_perf_object *prev)
>  	return next;
>  }
>  
> -#define bpf_perf_object__for_each(perf_obj, tmp, obj)                          \
> -	for ((perf_obj) = bpf_perf_object__next(NULL),                         \
> -	    (tmp) = bpf_perf_object__next(perf_obj), (obj) = NULL;             \
> -	     (perf_obj) != NULL; (perf_obj) = (tmp),                           \
> -	    (tmp) = bpf_perf_object__next(tmp), (obj) = (perf_obj)->obj)
> +#define bpf_perf_object__for_each(perf_obj, tmp)         \
> +	for ((perf_obj) = bpf_perf_object__next(NULL),   \
> +	     (tmp) = bpf_perf_object__next(perf_obj);    \
> +	     (perf_obj) != NULL; (perf_obj) = (tmp),     \
> +	    (tmp) = bpf_perf_object__next(tmp) )
>  
>  static bool libbpf_initialized;
>  
> @@ -97,6 +97,24 @@ bpf__prepare_load_buffer(void *obj_buf, size_t obj_buf_sz, const char *name)
>  	return obj;
>  }
>  
> +int bpf_perf_object__add(struct bpf_object *obj)
> +{
> +	struct bpf_perf_object *perf_obj = zalloc(sizeof(*perf_obj));
> +
> +	if (perf_obj) {
> +		perf_obj->obj = obj;
> +		list_add_tail(&perf_obj->list, &bpf_objects_list);
> +	}
> +	return perf_obj ? 0 : -ENOMEM;
> +}
> +
> +static void bpf_perf_object__close(struct bpf_perf_object *perf_obj)
> +{
> +	list_del(&perf_obj->list);
> +	bpf_object__close(perf_obj->obj);
> +	free(perf_obj);
> +}
> +
>  struct bpf_object *bpf__prepare_load(const char *filename, bool source)
>  {
>  	struct bpf_object *obj;
> @@ -135,17 +153,20 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
>  		return obj;
>  	}
>  
> +	if (bpf_perf_object__add(obj)) {
> +		bpf_object__close(obj);
> +		return ERR_PTR(-BPF_LOADER_ERRNO__COMPILE);
> +	}
>  	return obj;
>  }
>  
>  void bpf__clear(void)
>  {
>  	struct bpf_perf_object *perf_obj, *tmp;
> -	struct bpf_object *obj;
>  
> -	bpf_perf_object__for_each(perf_obj, tmp, obj) {
> -		bpf__unprobe(obj);
> -		bpf_object__close(obj);
> +	bpf_perf_object__for_each(perf_obj, tmp) {
> +		bpf__unprobe(perf_obj->obj);
> +		bpf_perf_object__close(perf_obj);
>  	}
>  }
>  
> @@ -1538,11 +1559,10 @@ apply_obj_config_object(struct bpf_object *obj)
>  int bpf__apply_obj_config(void)
>  {
>  	struct bpf_perf_object *perf_obj, *tmp;
> -	struct bpf_object *obj;
>  	int err;
>  
> -	bpf_perf_object__for_each(perf_obj, tmp, obj) {
> -		err = apply_obj_config_object(obj);
> +	bpf_perf_object__for_each(perf_obj, tmp) {
> +		err = apply_obj_config_object(perf_obj->obj);
>  		if (err)
>  			return err;
>  	}
> @@ -1550,25 +1570,24 @@ int bpf__apply_obj_config(void)
>  	return 0;
>  }
>  
> -#define bpf__perf_for_each_map(perf_obj, tmp, obj, map)                        \
> -	bpf_perf_object__for_each(perf_obj, tmp, obj)                          \
> -		bpf_object__for_each_map(map, obj)
> +#define bpf__perf_for_each_map(perf_obj, tmp, map)                   \
> +	bpf_perf_object__for_each(perf_obj, tmp)                     \
> +		bpf_object__for_each_map(map, perf_obj->obj)
>  
> -#define bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name)            \
> -	bpf__perf_for_each_map(perf_obj, tmp, obj, map)                        \
> +#define bpf__perf_for_each_map_named(perf_obj, tmp, map, name)            \
> +	bpf__perf_for_each_map(perf_obj, tmp, map)                        \
>  		if (bpf_map__name(map) && (strcmp(name, bpf_map__name(map)) == 0))
>  
>  struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
>  {
>  	struct bpf_map_priv *tmpl_priv = NULL;
>  	struct bpf_perf_object *perf_obj, *tmp;
> -	struct bpf_object *obj;
>  	struct evsel *evsel = NULL;
>  	struct bpf_map *map;
>  	int err;
>  	bool need_init = false;
>  
> -	bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name) {
> +	bpf__perf_for_each_map_named(perf_obj, tmp, map, name) {
>  		struct bpf_map_priv *priv = bpf_map__priv(map);
>  
>  		if (IS_ERR(priv))
> @@ -1604,7 +1623,7 @@ struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
>  		evsel = evlist__last(evlist);
>  	}
>  
> -	bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name) {
> +	bpf__perf_for_each_map_named(perf_obj, tmp, map, name) {
>  		struct bpf_map_priv *priv = bpf_map__priv(map);
>  
>  		if (IS_ERR(priv))
> diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
> index 95262b7e936f..78c7d3662910 100644
> --- a/tools/perf/util/bpf-loader.h
> +++ b/tools/perf/util/bpf-loader.h
> @@ -83,6 +83,8 @@ int bpf__strerror_config_obj(struct bpf_object *obj,
>  int bpf__apply_obj_config(void);
>  int bpf__strerror_apply_obj_config(int err, char *buf, size_t size);
>  
> +int bpf_perf_object__add(struct bpf_object *obj);
> +
>  int bpf__setup_stdout(struct evlist *evlist);
>  struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name);
>  int bpf__strerror_setup_output_event(struct evlist *evlist, int err, char *buf, size_t size);

Re: [PATCH bpf-next v4 2/2] perf: stop using deprecated bpf_object__next() API

[Christy Lee]

Sorry, some other priorities came up. If it's urgent, please feel free
to commandeer this commit. Otherwise, I can get to it this weekend.

Christy

On Wed, Feb 2, 2022 at 9:00 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> hi,
> just checking, do you plan to send new version for this?
>
> thanks,
> jirka
>
> On Sun, Jan 23, 2022 at 06:06:08PM +0100, Jiri Olsa wrote:
> > On Wed, Jan 19, 2022 at 03:06:36PM -0800, Christy Lee wrote:
> >
> > SNIP
> >
> > > ---
> > >  tools/perf/util/bpf-loader.c | 72 +++++++++++++++++++++++++++---------
> > >  tools/perf/util/bpf-loader.h |  1 +
> > >  2 files changed, 55 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> > > index 4631cac3957f..b1822f8af2bb 100644
> > > --- a/tools/perf/util/bpf-loader.c
> > > +++ b/tools/perf/util/bpf-loader.c
> > > @@ -29,9 +29,6 @@
> > >
> > >  #include <internal/xyarray.h>
> > >
> > > -/* temporarily disable libbpf deprecation warnings */
> > > -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > > -
> > >  static int libbpf_perf_print(enum libbpf_print_level level __attribute__((unused)),
> > >                           const char *fmt, va_list args)
> > >  {
> > > @@ -49,6 +46,36 @@ struct bpf_prog_priv {
> > >     int *type_mapping;
> > >  };
> > >
> > > +struct bpf_perf_object {
> > > +   struct bpf_object *obj;
> > > +   struct list_head list;
> > > +};
> > > +
> > > +static LIST_HEAD(bpf_objects_list);
> >
> > hum.. I still can't see any code adding/removing bpf_perf_object
> > objects to this list, and that's why the code is failing to remove
> > probes
> >
> > because there are no objects to iterate on, so added probes stay
> > configured and screw following tests
> >
> > you need something like below to add and del objects from
> > bpf_objects_list list
> >
> > it also simplifies for_each macros to work just over perf_obj,
> > because I wasn't patient enough to make it work with the extra
> > bpf_object ;-) I don't mind if you fix that, but this way looks
> > simpler to me
> >
> > tests are working for me with this fix, please feel free to
> > squash it into your change
> >
> > thanks,
> > jirka
> >
> >
> > ---
> > diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> > index 57b9591f7cbb..d09d25707f1e 100644
> > --- a/tools/perf/tests/bpf.c
> > +++ b/tools/perf/tests/bpf.c
> > @@ -210,6 +210,11 @@ prepare_bpf(void *obj_buf, size_t obj_buf_sz, const char *name)
> >               pr_debug("Compile BPF program failed.\n");
> >               return NULL;
> >       }
> > +
> > +     if (bpf_perf_object__add(obj)) {
> > +             bpf_object__close(obj);
> > +             return NULL;
> > +     }
> >       return obj;
> >  }
> >
> > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> > index 89e584ac267c..a7a8ad32c847 100644
> > --- a/tools/perf/util/bpf-loader.c
> > +++ b/tools/perf/util/bpf-loader.c
> > @@ -70,11 +70,11 @@ struct bpf_perf_object *bpf_perf_object__next(struct bpf_perf_object *prev)
> >       return next;
> >  }
> >
> > -#define bpf_perf_object__for_each(perf_obj, tmp, obj)                          \
> > -     for ((perf_obj) = bpf_perf_object__next(NULL),                         \
> > -         (tmp) = bpf_perf_object__next(perf_obj), (obj) = NULL;             \
> > -          (perf_obj) != NULL; (perf_obj) = (tmp),                           \
> > -         (tmp) = bpf_perf_object__next(tmp), (obj) = (perf_obj)->obj)
> > +#define bpf_perf_object__for_each(perf_obj, tmp)         \
> > +     for ((perf_obj) = bpf_perf_object__next(NULL),   \
> > +          (tmp) = bpf_perf_object__next(perf_obj);    \
> > +          (perf_obj) != NULL; (perf_obj) = (tmp),     \
> > +         (tmp) = bpf_perf_object__next(tmp) )
> >
> >  static bool libbpf_initialized;
> >
> > @@ -97,6 +97,24 @@ bpf__prepare_load_buffer(void *obj_buf, size_t obj_buf_sz, const char *name)
> >       return obj;
> >  }
> >
> > +int bpf_perf_object__add(struct bpf_object *obj)
> > +{
> > +     struct bpf_perf_object *perf_obj = zalloc(sizeof(*perf_obj));
> > +
> > +     if (perf_obj) {
> > +             perf_obj->obj = obj;
> > +             list_add_tail(&perf_obj->list, &bpf_objects_list);
> > +     }
> > +     return perf_obj ? 0 : -ENOMEM;
> > +}
> > +
> > +static void bpf_perf_object__close(struct bpf_perf_object *perf_obj)
> > +{
> > +     list_del(&perf_obj->list);
> > +     bpf_object__close(perf_obj->obj);
> > +     free(perf_obj);
> > +}
> > +
> >  struct bpf_object *bpf__prepare_load(const char *filename, bool source)
> >  {
> >       struct bpf_object *obj;
> > @@ -135,17 +153,20 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
> >               return obj;
> >       }
> >
> > +     if (bpf_perf_object__add(obj)) {
> > +             bpf_object__close(obj);
> > +             return ERR_PTR(-BPF_LOADER_ERRNO__COMPILE);
> > +     }
> >       return obj;
> >  }
> >
> >  void bpf__clear(void)
> >  {
> >       struct bpf_perf_object *perf_obj, *tmp;
> > -     struct bpf_object *obj;
> >
> > -     bpf_perf_object__for_each(perf_obj, tmp, obj) {
> > -             bpf__unprobe(obj);
> > -             bpf_object__close(obj);
> > +     bpf_perf_object__for_each(perf_obj, tmp) {
> > +             bpf__unprobe(perf_obj->obj);
> > +             bpf_perf_object__close(perf_obj);
> >       }
> >  }
> >
> > @@ -1538,11 +1559,10 @@ apply_obj_config_object(struct bpf_object *obj)
> >  int bpf__apply_obj_config(void)
> >  {
> >       struct bpf_perf_object *perf_obj, *tmp;
> > -     struct bpf_object *obj;
> >       int err;
> >
> > -     bpf_perf_object__for_each(perf_obj, tmp, obj) {
> > -             err = apply_obj_config_object(obj);
> > +     bpf_perf_object__for_each(perf_obj, tmp) {
> > +             err = apply_obj_config_object(perf_obj->obj);
> >               if (err)
> >                       return err;
> >       }
> > @@ -1550,25 +1570,24 @@ int bpf__apply_obj_config(void)
> >       return 0;
> >  }
> >
> > -#define bpf__perf_for_each_map(perf_obj, tmp, obj, map)                        \
> > -     bpf_perf_object__for_each(perf_obj, tmp, obj)                          \
> > -             bpf_object__for_each_map(map, obj)
> > +#define bpf__perf_for_each_map(perf_obj, tmp, map)                   \
> > +     bpf_perf_object__for_each(perf_obj, tmp)                     \
> > +             bpf_object__for_each_map(map, perf_obj->obj)
> >
> > -#define bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name)            \
> > -     bpf__perf_for_each_map(perf_obj, tmp, obj, map)                        \
> > +#define bpf__perf_for_each_map_named(perf_obj, tmp, map, name)            \
> > +     bpf__perf_for_each_map(perf_obj, tmp, map)                        \
> >               if (bpf_map__name(map) && (strcmp(name, bpf_map__name(map)) == 0))
> >
> >  struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
> >  {
> >       struct bpf_map_priv *tmpl_priv = NULL;
> >       struct bpf_perf_object *perf_obj, *tmp;
> > -     struct bpf_object *obj;
> >       struct evsel *evsel = NULL;
> >       struct bpf_map *map;
> >       int err;
> >       bool need_init = false;
> >
> > -     bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name) {
> > +     bpf__perf_for_each_map_named(perf_obj, tmp, map, name) {
> >               struct bpf_map_priv *priv = bpf_map__priv(map);
> >
> >               if (IS_ERR(priv))
> > @@ -1604,7 +1623,7 @@ struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name)
> >               evsel = evlist__last(evlist);
> >       }
> >
> > -     bpf__perf_for_each_map_named(perf_obj, tmp, obj, map, name) {
> > +     bpf__perf_for_each_map_named(perf_obj, tmp, map, name) {
> >               struct bpf_map_priv *priv = bpf_map__priv(map);
> >
> >               if (IS_ERR(priv))
> > diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
> > index 95262b7e936f..78c7d3662910 100644
> > --- a/tools/perf/util/bpf-loader.h
> > +++ b/tools/perf/util/bpf-loader.h
> > @@ -83,6 +83,8 @@ int bpf__strerror_config_obj(struct bpf_object *obj,
> >  int bpf__apply_obj_config(void);
> >  int bpf__strerror_apply_obj_config(int err, char *buf, size_t size);
> >
> > +int bpf_perf_object__add(struct bpf_object *obj);
> > +
> >  int bpf__setup_stdout(struct evlist *evlist);
> >  struct evsel *bpf__setup_output_event(struct evlist *evlist, const char *name);
> >  int bpf__strerror_setup_output_event(struct evlist *evlist, int err, char *buf, size_t size);
>

Re: [PATCH bpf-next v4 2/2] perf: stop using deprecated bpf_object__next() API

[Andrii Nakryiko]

On Wed, Feb 2, 2022 at 9:24 AM Christy Lee <christyc.y.lee@gmail.com> wrote:
>
> Sorry, some other priorities came up. If it's urgent, please feel free
> to commandeer this commit. Otherwise, I can get to it this weekend.
>
> Christy

Hey Christy,

Do you still plan to send follow up patches or should I pick this up?

>
> On Wed, Feb 2, 2022 at 9:00 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > hi,
> > just checking, do you plan to send new version for this?
> >
> > thanks,
> > jirka
> >
> > On Sun, Jan 23, 2022 at 06:06:08PM +0100, Jiri Olsa wrote:
> > > On Wed, Jan 19, 2022 at 03:06:36PM -0800, Christy Lee wrote:
> > >
> > > SNIP
> > >
> > > > ---
> > > >  tools/perf/util/bpf-loader.c | 72 +++++++++++++++++++++++++++---------
> > > >  tools/perf/util/bpf-loader.h |  1 +
> > > >  2 files changed, 55 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> > > > index 4631cac3957f..b1822f8af2bb 100644
> > > > --- a/tools/perf/util/bpf-loader.c
> > > > +++ b/tools/perf/util/bpf-loader.c
> > > > @@ -29,9 +29,6 @@
> > > >
> > > >  #include <internal/xyarray.h>
> > > >
> > > > -/* temporarily disable libbpf deprecation warnings */
> > > > -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > > > -
> > > >  static int libbpf_perf_print(enum libbpf_print_level level __attribute__((unused)),
> > > >                           const char *fmt, va_list args)
> > > >  {
> > > > @@ -49,6 +46,36 @@ struct bpf_prog_priv {
> > > >     int *type_mapping;
> > > >  };
> > > >
> > > > +struct bpf_perf_object {
> > > > +   struct bpf_object *obj;
> > > > +   struct list_head list;
> > > > +};
> > > > +
> > > > +static LIST_HEAD(bpf_objects_list);
> > >
> > > hum.. I still can't see any code adding/removing bpf_perf_object
> > > objects to this list, and that's why the code is failing to remove
> > > probes
> > >
> > > because there are no objects to iterate on, so added probes stay
> > > configured and screw following tests
> > >
> > > you need something like below to add and del objects from
> > > bpf_objects_list list
> > >
> > > it also simplifies for_each macros to work just over perf_obj,
> > > because I wasn't patient enough to make it work with the extra
> > > bpf_object ;-) I don't mind if you fix that, but this way looks
> > > simpler to me
> > >
> > > tests are working for me with this fix, please feel free to
> > > squash it into your change
> > >
> > > thanks,
> > > jirka
> > >
> > >
> > > ---
> > > diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> > > index 57b9591f7cbb..d09d25707f1e 100644
> > > --- a/tools/perf/tests/bpf.c
> > > +++ b/tools/perf/tests/bpf.c
> > > @@ -210,6 +210,11 @@ prepare_bpf(void *obj_buf, size_t obj_buf_sz, const char *name)
> > >               pr_debug("Compile BPF program failed.\n");
> > >               return NULL;
> > >       }

[...]

Re: [PATCH bpf-next v4 2/2] perf: stop using deprecated bpf_object__next() API

[Andrii Nakryiko]

On Sun, Jan 23, 2022 at 9:06 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Wed, Jan 19, 2022 at 03:06:36PM -0800, Christy Lee wrote:
>
> SNIP
>
> > ---
> >  tools/perf/util/bpf-loader.c | 72 +++++++++++++++++++++++++++---------
> >  tools/perf/util/bpf-loader.h |  1 +
> >  2 files changed, 55 insertions(+), 18 deletions(-)
> >
> > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> > index 4631cac3957f..b1822f8af2bb 100644
> > --- a/tools/perf/util/bpf-loader.c
> > +++ b/tools/perf/util/bpf-loader.c
> > @@ -29,9 +29,6 @@
> >
> >  #include <internal/xyarray.h>
> >
> > -/* temporarily disable libbpf deprecation warnings */
> > -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> > -
> >  static int libbpf_perf_print(enum libbpf_print_level level __attribute__((unused)),
> >                             const char *fmt, va_list args)
> >  {
> > @@ -49,6 +46,36 @@ struct bpf_prog_priv {
> >       int *type_mapping;
> >  };
> >
> > +struct bpf_perf_object {
> > +     struct bpf_object *obj;
> > +     struct list_head list;
> > +};
> > +
> > +static LIST_HEAD(bpf_objects_list);
>
> hum.. I still can't see any code adding/removing bpf_perf_object
> objects to this list, and that's why the code is failing to remove
> probes
>
> because there are no objects to iterate on, so added probes stay
> configured and screw following tests
>
> you need something like below to add and del objects from
> bpf_objects_list list
>
> it also simplifies for_each macros to work just over perf_obj,
> because I wasn't patient enough to make it work with the extra
> bpf_object ;-) I don't mind if you fix that, but this way looks
> simpler to me

yep, I agree

>
> tests are working for me with this fix, please feel free to
> squash it into your change
>

I've just sent v5, I would really appreciate it if you could give the
changes another testing round. Thanks a lot, Jiri!

> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> index 57b9591f7cbb..d09d25707f1e 100644
> --- a/tools/perf/tests/bpf.c
> +++ b/tools/perf/tests/bpf.c
> @@ -210,6 +210,11 @@ prepare_bpf(void *obj_buf, size_t obj_buf_sz, const char *name)
>                 pr_debug("Compile BPF program failed.\n");
>                 return NULL;
>         }
> +
> +       if (bpf_perf_object__add(obj)) {
> +               bpf_object__close(obj);
> +               return NULL;
> +       }

I actually moved this into bpf__prepare_load_buffer(), it felt better
to contain this logic within it. It follows what you did for
bpf__prepare_load() and allowed to keep bpf_perf_object__add() static.

In the end I got a patch which doesn't expose any new function outside
of bpf-loader.c, which I think is the best solution.

>         return obj;
>  }
>

[...]