Re: [PATCH 3/4] perf tools: Move common bpf functions to bpf_counter.h

[Namhyung Kim]

On Thu, Jul 1, 2021 at 12:09 PM Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
>
> Em Fri, Jun 25, 2021 at 12:18:25AM -0700, Namhyung Kim escreveu:
> > Some helper functions will be used for cgroup counting too.
> > Move them to a header file for sharing.
> >
> > Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> > ---
> >  tools/perf/util/bpf_counter.c | 52 -----------------------------------
> >  tools/perf/util/bpf_counter.h | 52 +++++++++++++++++++++++++++++++++++
> >  2 files changed, 52 insertions(+), 52 deletions(-)
> >
> > diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
> > index 974f10e356f0..1af81e882eb6 100644
> > --- a/tools/perf/util/bpf_counter.c
> > +++ b/tools/perf/util/bpf_counter.c
> > @@ -7,12 +7,8 @@
> >  #include <unistd.h>
> >  #include <sys/file.h>
> >  #include <sys/time.h>
> > -#include <sys/resource.h>
> >  #include <linux/err.h>
> >  #include <linux/zalloc.h>
> > -#include <bpf/bpf.h>
> > -#include <bpf/btf.h>
> > -#include <bpf/libbpf.h>
> >  #include <api/fs/fs.h>
> >  #include <perf/bpf_perf.h>
> >
> > @@ -37,13 +33,6 @@ static inline void *u64_to_ptr(__u64 ptr)
> >       return (void *)(unsigned long)ptr;
> >  }
> >
> > -static void set_max_rlimit(void)
> > -{
> > -     struct rlimit rinf = { RLIM_INFINITY, RLIM_INFINITY };
> > -
> > -     setrlimit(RLIMIT_MEMLOCK, &rinf);
> > -}
> > -
> >  static struct bpf_counter *bpf_counter_alloc(void)
> >  {
> >       struct bpf_counter *counter;
> > @@ -297,33 +286,6 @@ struct bpf_counter_ops bpf_program_profiler_ops = {
> >       .install_pe = bpf_program_profiler__install_pe,
> >  };
> >
> > -static __u32 bpf_link_get_id(int fd)
> > -{
> > -     struct bpf_link_info link_info = {0};
>
> Moving this from bpf_counter.c to the header made this code be compiled
> in places where it wasn't before, as bpf_counter.c is built only when:
>
> perf-$(CONFIG_PERF_BPF_SKEL) += bpf_counter.o
>
> For instance, this got broken:
>
>   23    33.62 debian:9                      : FAIL clang version 3.8.1-24 (tags/RELEASE_381/final)
>     In file included from builtin-stat.c:71:
>     /git/perf-5.13.0/tools/perf/util/bpf_counter.h:92:37: error: missing field 'id' initializer [-Werror,-Wmissing-field-initializers]
>             struct bpf_link_info link_info = {0};
>                                                ^
>     /git/perf-5.13.0/tools/perf/util/bpf_counter.h:101:37: error: missing field 'id' initializer [-Werror,-Wmissing-field-initializers]
>             struct bpf_link_info link_info = {0};
>                                                ^
>     /git/perf-5.13.0/tools/perf/util/bpf_counter.h:110:35: error: missing field 'id' initializer [-Werror,-Wmissing-field-initializers]
>             struct bpf_map_info map_info = {0};
>
> ITs mostly older systems, but I'll fix it anyway.

Thanks a lot for fixing all the messes.
Assuming you're fixed the preparation patches.
I'll just update the last one next time.

Thanks,
Namhyung


>
> > -     __u32 link_info_len = sizeof(link_info);
> > -
> > -     bpf_obj_get_info_by_fd(fd, &link_info, &link_info_len);
> > -     return link_info.id;
> > -}
> > -
> > -static __u32 bpf_link_get_prog_id(int fd)
> > -{
> > -     struct bpf_link_info link_info = {0};
> > -     __u32 link_info_len = sizeof(link_info);
> > -
> > -     bpf_obj_get_info_by_fd(fd, &link_info, &link_info_len);
> > -     return link_info.prog_id;
> > -}
> > -
> > -static __u32 bpf_map_get_id(int fd)
> > -{
> > -     struct bpf_map_info map_info = {0};
> > -     __u32 map_info_len = sizeof(map_info);
> > -
> > -     bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len);
> > -     return map_info.id;
> > -}
> > -
> >  static bool bperf_attr_map_compatible(int attr_map_fd)
> >  {
> >       struct bpf_map_info map_info = {0};
> > @@ -385,20 +347,6 @@ static int bperf_lock_attr_map(struct target *target)
> >       return map_fd;
> >  }
> >
> > -/* trigger the leader program on a cpu */
> > -static int bperf_trigger_reading(int prog_fd, int cpu)
> > -{
> > -     DECLARE_LIBBPF_OPTS(bpf_test_run_opts, opts,
> > -                         .ctx_in = NULL,
> > -                         .ctx_size_in = 0,
> > -                         .flags = BPF_F_TEST_RUN_ON_CPU,
> > -                         .cpu = cpu,
> > -                         .retval = 0,
> > -             );
> > -
> > -     return bpf_prog_test_run_opts(prog_fd, &opts);
> > -}
> > -
> >  static int bperf_check_target(struct evsel *evsel,
> >                             struct target *target,
> >                             enum bperf_filter_type *filter_type,
> > diff --git a/tools/perf/util/bpf_counter.h b/tools/perf/util/bpf_counter.h
> > index d6d907c3dcf9..185555a9c1db 100644
> > --- a/tools/perf/util/bpf_counter.h
> > +++ b/tools/perf/util/bpf_counter.h
> > @@ -3,6 +3,10 @@
> >  #define __PERF_BPF_COUNTER_H 1
> >
> >  #include <linux/list.h>
> > +#include <sys/resource.h>
> > +#include <bpf/bpf.h>
> > +#include <bpf/btf.h>
> > +#include <bpf/libbpf.h>
> >
> >  struct evsel;
> >  struct target;
> > @@ -76,4 +80,52 @@ static inline int bpf_counter__install_pe(struct evsel *evsel __maybe_unused,
> >
> >  #endif /* HAVE_BPF_SKEL */
> >
> > +static inline void set_max_rlimit(void)
> > +{
> > +     struct rlimit rinf = { RLIM_INFINITY, RLIM_INFINITY };
> > +
> > +     setrlimit(RLIMIT_MEMLOCK, &rinf);
> > +}
> > +
> > +static inline __u32 bpf_link_get_id(int fd)
> > +{
> > +     struct bpf_link_info link_info = {0};
> > +     __u32 link_info_len = sizeof(link_info);
> > +
> > +     bpf_obj_get_info_by_fd(fd, &link_info, &link_info_len);
> > +     return link_info.id;
> > +}
> > +
> > +static inline __u32 bpf_link_get_prog_id(int fd)
> > +{
> > +     struct bpf_link_info link_info = {0};
> > +     __u32 link_info_len = sizeof(link_info);
> > +
> > +     bpf_obj_get_info_by_fd(fd, &link_info, &link_info_len);
> > +     return link_info.prog_id;
> > +}
> > +
> > +static inline __u32 bpf_map_get_id(int fd)
> > +{
> > +     struct bpf_map_info map_info = {0};
> > +     __u32 map_info_len = sizeof(map_info);
> > +
> > +     bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len);
> > +     return map_info.id;
> > +}
> > +
> > +/* trigger the leader program on a cpu */
> > +static inline int bperf_trigger_reading(int prog_fd, int cpu)
> > +{
> > +     DECLARE_LIBBPF_OPTS(bpf_test_run_opts, opts,
> > +                         .ctx_in = NULL,
> > +                         .ctx_size_in = 0,
> > +                         .flags = BPF_F_TEST_RUN_ON_CPU,
> > +                         .cpu = cpu,
> > +                         .retval = 0,
> > +             );
> > +
> > +     return bpf_prog_test_run_opts(prog_fd, &opts);
> > +}
> > +
> >  #endif /* __PERF_BPF_COUNTER_H */
> > --
> > 2.32.0.93.g670b81a890-goog
> >
>
> --
>
> - Arnaldo