On Tue, Dec 13, 2022 at 1:53 AM Athira Rajeev <atrajeev@xxxxxxxxxxxxxxxxxx> wrote: > > > > > On 12-Dec-2022, at 7:21 PM, Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote: > > > > Em Fri, Dec 09, 2022 at 12:04:18PM +0530, Athira Rajeev escreveu: > >> > >> > >>> On 09-Dec-2022, at 4:02 AM, Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote: > >>> > >>> Em Thu, Dec 08, 2022 at 07:04:52PM -0300, Arnaldo Carvalho de Melo escreveu: > >>>> Em Thu, Dec 08, 2022 at 12:21:20PM +0530, Athira Rajeev escreveu: > >>>>>> On 07-Dec-2022, at 10:57 PM, Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote: > >>>>>> Can you try again? tmp.perf/core? That "tmp." part means its a force > >>>>>> pushed branch, so I just force pushed with some arch specific fixes, now > >>>>>> I'm down to (removing the successful builds and unrelated failures, now > >>>>>> related to libbpf's F_DUPFD_CLOEXEC kaboom): > >>>> > >>>>> Ok Arnaldo, Sure, I will check with updated branch > >>>> > >>>>>> 5 7.38 fedora:34 : FAIL gcc version 11.3.1 20220421 (Red Hat 11.3.1-2) (GCC) > >>>>>> /git/perf-6.1.0-rc6/tools/perf/util/evsel.c: In function ‘evsel__rawptr’: > >>>>>> /git/perf-6.1.0-rc6/tools/perf/util/evsel.c:2787:36: error: ‘TEP_FIELD_IS_RELATIVE’ undeclared (first use in this function); did you mean ‘TEP_FIELD_IS_FLAG’? > >>>>>> 2787 | if (field->flags & TEP_FIELD_IS_RELATIVE) > >>>>>> | ^~~~~~~~~~~~~~~~~~~~~ > >>>>>> | TEP_FIELD_IS_FLAG > >>>> > >>>>> I observed same issue as updated here: > >>>>> https://lore.kernel.org/lkml/10476A85-3F75-4C91-AB5B-E5B136F31297@xxxxxxxxxxxxxxxxxx/ > >>>> > >>>>> Looks like TEP_FIELD_IS_RELATIVE is not defined in header file of the system installed version. > >>>>> whereas it is there in header file in tools/lib/traceevent > >>>> > >>>>> # grep TEP_FIELD_IS_RELATIVE /usr/include/traceevent/event-parse.h > >>>>> # grep TEP_FIELD_IS_RELATIVE ../lib/traceevent/event-parse.h > >>>>> TEP_FIELD_IS_RELATIVE = 256, > >>>> > >>>> Right, I had noticed that as well, so as a prep patch I'm adding the > >>>> patch below, before Ian's. Please check and provide an > >>>> Acked-by/Tested-by/Reviewed-by if possible. > >>> > >>> I ended up with the one below, _after_ Ian's patch as I had some trouble grafting > >>> it before and had already tested it this way multiple times, I'm pushing > >>> this to tmp/perf.core. > >>> > >>> - Arnaldo > >> > >> > >> Hi Arnaldo, Ian > >> > >> Thanks for the fixes. > >> > >> Since we changed “CONFIG_TRACEEVENT” to “CONFIG_LIBTRACEEVENT”, > >> below change is also needed in “arch/powerpc/util/Build” > >> > >> diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc/util/Build > >> index 71e57f28abda..9889245c555c 100644 > >> --- a/tools/perf/arch/powerpc/util/Build > >> +++ b/tools/perf/arch/powerpc/util/Build > >> @@ -1,5 +1,5 @@ > >> perf-y += header.o > >> -perf-$(CONFIG_TRACEEVENT) += kvm-stat.o > >> +perf-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o > >> perf-y += perf_regs.o > >> perf-y += mem-events.o > >> perf-y += sym-handling.o > >> > >> With this change, I could successfully compile in these environment: > >> - Without libtraceevent-devel installed > >> - With libtraceevent-devel installed > >> - With “make NO_LIBTRACEEVENT=1” > >> > >> With above change, > >> Acked-by: Athira Rajeev <atrajeev@xxxxxxxxxxxxxxxxxx> > > > > I did that and the same thing for other architectures, thanks for > > testing! > > > > I'll now give a try at implementing it without > > tools/build/feature/test-libtraceevent-tep_field_is_relative.c, using > > just the version of libtraceevent, as Ian suggested. > > > > It would be great if you could test it again then, > > > > Sure Arnaldo, I will test with updated code. > > Thanks > Athira Thanks Athira and Arnaldo. It is a little strange to me to be using the shell to do a version number test. The intent was to be doing these in the code: #if LIBRTRACEEVENT_VERSION >= MAKE_LIBTRACEEVENT_VERSION(1, 5, 0) vs ... LIBTRACEEVENT_VERSION_WITH_TEP_FIELD_IS_RELATIVE := $(shell expr 1 \* 255 \* 255 + 5 \* 255 + 0) # 1.5.0 ifeq ($(shell test $(LIBTRACEEVENT_VERSION_CPP) -gt $(LIBTRACEEVENT_VERSION_WITH_TEP_FIELD_IS_RELATIVE); echo $$?),0) CFLAGS += -DHAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE endif ... #ifdef HAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE I'm a little selfish as I'm maintaining a bazel build and a single version number to maintain is easier than lots of HAVE_... tests. I'm happy to follow Arnaldo's lead. I think the test should also be greater-equal rather than greater-than: https://git.kernel.org/pub/scm/libs/libtrace/libtraceevent.git/tree/include/traceevent/event-parse.h?h=libtraceevent-v1.5#n128 Thanks, Ian Ian > > Thanks, > > > > - Arnaldo > > > > > > diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build > > index 88553c578ed7a1c4..78ef7115be3d91a7 100644 > > --- a/tools/perf/arch/arm64/util/Build > > +++ b/tools/perf/arch/arm64/util/Build > > @@ -3,7 +3,7 @@ perf-y += machine.o > > perf-y += perf_regs.o > > perf-y += tsc.o > > perf-y += pmu.o > > -perf-$(CONFIG_TRACEEVENT) += kvm-stat.o > > +perf-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o > > perf-$(CONFIG_DWARF) += dwarf-regs.o > > perf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o > > perf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o > > diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc/util/Build > > index 71e57f28abdac7e9..9889245c555c4cfb 100644 > > --- a/tools/perf/arch/powerpc/util/Build > > +++ b/tools/perf/arch/powerpc/util/Build > > @@ -1,5 +1,5 @@ > > perf-y += header.o > > -perf-$(CONFIG_TRACEEVENT) += kvm-stat.o > > +perf-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o > > perf-y += perf_regs.o > > perf-y += mem-events.o > > perf-y += sym-handling.o > > diff --git a/tools/perf/arch/s390/util/Build b/tools/perf/arch/s390/util/Build > > index aa8a5f05c9cb4706..db68840869979f2c 100644 > > --- a/tools/perf/arch/s390/util/Build > > +++ b/tools/perf/arch/s390/util/Build > > @@ -1,5 +1,5 @@ > > perf-y += header.o > > -perf-$(CONFIG_TRACEEVENT) += kvm-stat.o > > +perf-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o > > perf-y += perf_regs.o > > > > perf-$(CONFIG_DWARF) += dwarf-regs.o >
