> On 14-Dec-2022, at 3:39 AM, Ian Rogers <irogers@xxxxxxxxxx> wrote: > > 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 Hi Arnaldo, Ian Compile tested with updated tmp.perf/core and results are good Thanks Athira > 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