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

[Jiri Olsa <jolsa@redhat.com>]

On Thu, Dec 16, 2021 at 02:21:08PM -0800, Christy Lee wrote:
> bpf__object_next is deprecated, track bpf_objects directly in
> perf instead.
> 
> 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 528aeb0ab79d..9e3988fd719a 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, so this duplicates libbpf's bpf_objects_list,
how do objects get on this list?

could you please put more comments in changelog
and share how you tested this?

thanks,
jirka

> +
> +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);
>  			}
> @@ -1498,10 +1534,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;
> @@ -1510,26 +1547,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))
> @@ -1565,7 +1601,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
>