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@xxxxxxxxxx> 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);
>
