Re: [PATCH] tools/perf: Fix perf tool build error in util/pfm.c

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Feb 08, 2023 at 10:44:10AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Feb 08, 2023 at 10:40:57AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Tue, Feb 07, 2023 at 09:58:48PM -0800, Ian Rogers escreveu:
> > > On Tue, Feb 7, 2023 at 7:02 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > >
> > > > Em Tue, Feb 07, 2023 at 03:04:47PM +0100, Thomas Richter escreveu:
> > > > > I have downloaded linux-next and build the perf tool using
> > > > >
> > > > >  # make LIBPFM4=1
> > > > >
> > > > > to have libpfm4 support built into perf. The build fails:
> > > > >
> > > > >  # make LIBPFM4=1
> > > >
> > > > Ian,
> > > >
> > > >         Can you please take a look? Should we go this way or have some
> > > > feature test to check how that function signature should look?
> > > >
> > > > - Arnaldo
> > > 
> > > Tested-by: Ian Rogers <irogers@google.com>
> > > 
> > > Wrt tests, there is make_with_libpfm4 in tools/perf/tests/make.

I realized I hadn't libbpf-devel installed on the machine I run 'make -C
tools/perf build-test' :-\ Fixed.

- Arnaldo

> > I meant if we should support older versions or state that to build with
> > libbpf we need to have the version with this new signature.
> 
> I see, the signature change wasn't in libpfm, nevermind, testing now.
> 
> - Arnaldo
>  
> > I'll apply and test build it here.
> > 
> > - Arnaldo
> >  
> > > Thanks,
> > > Ian
> > > 
> > > > > ....
> > > > > INSTALL libbpf_headers
> > > > >   CC      util/pfm.o
> > > > > util/pfm.c: In function ‘print_libpfm_event’:
> > > > > util/pfm.c:189:9: error: too many arguments to function ‘print_cb->print_event’
> > > > >   189 |         print_cb->print_event(print_state,
> > > > >       |         ^~~~~~~~
> > > > > util/pfm.c:220:25: error: too many arguments to function ‘print_cb->print_event’
> > > > >   220 |                         print_cb->print_event(print_state,
> > > > >
> > > > > The build error is caused by
> > > > > commit d9dc8874d6ce ("perf pmu-events: Remove now unused event and metric variables")
> > > > > which changes the function prototype of
> > > > >   struct print_callbacks {
> > > > >       ...
> > > > >       void (*print_event)(...);  --> last two parameters removed.
> > > > >   };
> > > > >
> > > > > but does not adjust the usage of this function prototype in util/pfm.c.
> > > > > In file util/pfm.c function print_event() is still invoked with 13 parameters
> > > > > instead of 11. The compile fails.
> > > > >
> > > > > When I adjust the file util/pfm.c as in this patch, the build works file.
> > > > > Please check this patch for correctness, I have just fixed the compile
> > > > > issue.
> > > > >
> > > > > Fixes d9dc8874d6ce ("perf pmu-events: Remove now unused event and metric variables")
> > > > > Cc: irogers@google.com
> > > > > Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> > > > > ---
> > > > >  tools/perf/util/pfm.c | 6 ++----
> > > > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/tools/perf/util/pfm.c b/tools/perf/util/pfm.c
> > > > > index ac3227ba769c..b59ba825ddc9 100644
> > > > > --- a/tools/perf/util/pfm.c
> > > > > +++ b/tools/perf/util/pfm.c
> > > > > @@ -193,8 +193,7 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
> > > > >                       /*scale_unit=*/NULL,
> > > > >                       /*deprecated=*/NULL, "PFM event",
> > > > >                       info->desc, /*long_desc=*/NULL,
> > > > > -                     /*encoding_desc=*/buf->buf,
> > > > > -                     /*metric_name=*/NULL, /*metric_expr=*/NULL);
> > > > > +                     /*encoding_desc=*/buf->buf);
> > > > >
> > > > >       pfm_for_each_event_attr(j, info) {
> > > > >               pfm_event_attr_info_t ainfo;
> > > > > @@ -224,8 +223,7 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
> > > > >                                       /*scale_unit=*/NULL,
> > > > >                                       /*deprecated=*/NULL, "PFM event",
> > > > >                                       ainfo.desc, /*long_desc=*/NULL,
> > > > > -                                     /*encoding_desc=*/buf->buf,
> > > > > -                                     /*metric_name=*/NULL, /*metric_expr=*/NULL);
> > > > > +                                     /*encoding_desc=*/buf->buf);
> > > > >               }
> > > > >       }
> > > > >  }
> > > > > --
> > > > > 2.39.1
> > > > >
> > > >
> > > > --
> > > >
> > > > - Arnaldo
> > 
> > -- 
> > 
> > - Arnaldo
> 
> -- 
> 
> - Arnaldo

-- 

- Arnaldo