Re: [PATCH 2/3] perf build: Use libtraceevent from the system

[Athira Rajeev <atrajeev@linux.vnet.ibm.com>]

> On 09-Dec-2022, at 4:02 AM, Arnaldo Carvalho de Melo <acme@kernel.org> 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@kernel.org> 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@linux.vnet.ibm.com/
>> 
>>> 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@linux.vnet.ibm.com>

Thanks
Athira

> 
> commit a0ec3ef4590e7e82339b6f318e671a6b69fd94e8
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Thu Dec 8 19:28:58 2022 -0300
> 
>    perf tools: Add test to check if libtracevent has TEP_FIELD_IS_RELATIVE
> 
>    Some distros have older versions of libtraceevent where
>    TEP_FIELD_IS_RELATIVE and its associated semantics are not present, so
>    we need to test for its presence and cope when it isn't present when
>    building with LIBTRACEEVENT_DYNAMIC=1.
> 
>    Reported-by: Athira Jajeev <atrajeev@linux.vnet.ibm.com>
>    Cc: Adrian Hunter <adrian.hunter@intel.com>
>    Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>    Cc: Ian Rogers <irogers@google.com>
>    Cc: Ingo Molnar <mingo@redhat.com>
>    Cc: Jiri Olsa <jolsa@kernel.org>
>    Cc: Mark Rutland <mark.rutland@arm.com>
>    Cc: Namhyung Kim <namhyung@kernel.org>
>    Cc: Nick Desaulniers <ndesaulniers@google.com>
>    Cc: Peter Zijlstra <peterz@infradead.org>,
>    Cc: Stephane Eranian <eranian@google.com>
>    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 38f8851bd7cbdb0e..c6c9b7c7723d4290 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -71,7 +71,8 @@ FEATURE_TESTS_BASIC :=                  \
>         libzstd				\
>         disassembler-four-args		\
>         disassembler-init-styled	\
> -        file-handle
> +        file-handle			\
> +        libtraceevent-tep_field_is_relative
> 
> # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
> # of all feature tests
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 690fe97be1904399..24246df35ac30eeb 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -81,7 +81,8 @@ FILES=                                          \
>          test-libzstd.bin			\
>          test-clang-bpf-co-re.bin		\
>          test-file-handle.bin			\
> -         test-libpfm4.bin
> +         test-libpfm4.bin			\
> +         test-libtraceevent-tep_field_is_relative
> 
> FILES := $(addprefix $(OUTPUT),$(FILES))
> 
> @@ -159,6 +160,9 @@ $(OUTPUT)test-dwarf_getlocations.bin:
> $(OUTPUT)test-libelf-getphdrnum.bin:
> 	$(BUILD) -lelf
> 
> +$(OUTPUT)test-libtraceevent-tep_field_is_relative.bin:
> +	$(BUILD) -ltraceevent
> +
> $(OUTPUT)test-libelf-gelf_getnote.bin:
> 	$(BUILD) -lelf
> 
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 680228e19c1aabdb..e29975664f06bd61 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -1197,6 +1197,10 @@ ifneq ($(NO_LIBTRACEEVENT),1)
>     LIBTRACEEVENT_VERSION_CPP := $(shell expr $(LIBTRACEEVENT_VERSION_1) \* 255 \* 255 + $(LIBTRACEEVENT_VERSION_2) \* 255 + $(LIBTRACEEVENT_VERSION_3))
>     CFLAGS += -DLIBTRACEEVENT_VERSION=$(LIBTRACEEVENT_VERSION_CPP)
>     $(call detected,CONFIG_LIBTRACEEVENT)
> +    $(call feature_check,libtraceevent-tep_field_is_relative)
> +    ifeq ($(feature-libtraceevent-tep_field_is_relative), 1)
> +      CFLAGS += -DHAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE
> +    endif
>   else
>     dummy := $(warning Warning: libtraceevent is missing limiting functionality, please install libtraceevent-dev/libtraceevent-devel)
>   endif
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 6909cd9f48d11644..c9802ab549d7782f 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -2729,8 +2729,10 @@ static size_t trace__fprintf_tp_fields(struct trace *trace, struct evsel *evsel,
> 				offset = format_field__intval(field, sample, evsel->needs_swap);
> 				syscall_arg.len = offset >> 16;
> 				offset &= 0xffff;
> +#ifdef HAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE
> 				if (field->flags & TEP_FIELD_IS_RELATIVE)
> 					offset += field->offset + field->size;
> +#endif
> 			}
> 
> 			val = (uintptr_t)(sample->raw_data + offset);
> diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
> index 8031b586e81384e7..b842273458b862bd 100644
> --- a/tools/perf/util/data-convert-bt.c
> +++ b/tools/perf/util/data-convert-bt.c
> @@ -322,8 +322,10 @@ static int add_tracepoint_field_value(struct ctf_writer *cw,
> 		offset = tmp_val;
> 		len = offset >> 16;
> 		offset &= 0xffff;
> +#ifdef HAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE
> 		if (flags & TEP_FIELD_IS_RELATIVE)
> 			offset += fmtf->offset + fmtf->size;
> +#endif
> 	}
> 
> 	if (flags & TEP_FIELD_IS_ARRAY) {
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index ca911856c4b1d3c8..77b2cf5a214ec1e4 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -2784,8 +2784,10 @@ void *evsel__rawptr(struct evsel *evsel, struct perf_sample *sample, const char
> 	if (field->flags & TEP_FIELD_IS_DYNAMIC) {
> 		offset = *(int *)(sample->raw_data + field->offset);
> 		offset &= 0xffff;
> +#ifdef HAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE
> 		if (field->flags & TEP_FIELD_IS_RELATIVE)
> 			offset += field->offset + field->size;
> +#endif
> 	}
> 
> 	return sample->raw_data + offset;
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 6fb84b7455b8392d..7320f7f777fe0e56 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -442,8 +442,10 @@ tracepoint_field(struct pyrf_event *pe, struct tep_format_field *field)
> 			offset  = val;
> 			len     = offset >> 16;
> 			offset &= 0xffff;
> +#ifdef HAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE
> 			if (field->flags & TEP_FIELD_IS_RELATIVE)
> 				offset += field->offset + field->size;
> +#endif
> 		}
> 		if (field->flags & TEP_FIELD_IS_STRING &&
> 		    is_printable_array(data + offset, len)) {
> diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c
> index 0bacb49408f84adf..c097b7934fd4ebe9 100644
> --- a/tools/perf/util/scripting-engines/trace-event-perl.c
> +++ b/tools/perf/util/scripting-engines/trace-event-perl.c
> @@ -393,8 +393,10 @@ static void perl_process_tracepoint(struct perf_sample *sample,
> 			if (field->flags & TEP_FIELD_IS_DYNAMIC) {
> 				offset = *(int *)(data + field->offset);
> 				offset &= 0xffff;
> +#ifdef HAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE
> 				if (field->flags & TEP_FIELD_IS_RELATIVE)
> 					offset += field->offset + field->size;
> +#endif
> 			} else
> 				offset = field->offset;
> 			XPUSHs(sv_2mortal(newSVpv((char *)data + offset, 0)));
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index fabba21919b8fa51..e930f5f1f36d2831 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -994,8 +994,10 @@ static void python_process_tracepoint(struct perf_sample *sample,
> 				offset  = val;
> 				len     = offset >> 16;
> 				offset &= 0xffff;
> +#ifdef HAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE
> 				if (field->flags & TEP_FIELD_IS_RELATIVE)
> 					offset += field->offset + field->size;
> +#endif
> 			}
> 			if (field->flags & TEP_FIELD_IS_STRING &&
> 			    is_printable_array(data + offset, len)) {
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index c7a97b33e13477e1..0ecc2cb137920d2d 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -2568,9 +2568,10 @@ static int64_t __sort__hde_cmp(struct perf_hpp_fmt *fmt,
> 		tep_read_number_field(field, a->raw_data, &dyn);
> 		offset = dyn & 0xffff;
> 		size = (dyn >> 16) & 0xffff;
> +#ifdef HAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE
> 		if (field->flags & TEP_FIELD_IS_RELATIVE)
> 			offset += field->offset + field->size;
> -
> +#endif
> 		/* record max width for output */
> 		if (size > hde->dynamic_len)
> 			hde->dynamic_len = size;