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