On Sun, Jan 23, 2022 at 9:06 AM Jiri Olsa <jolsa@xxxxxxxxxx> 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;
> }
>
[...]
