All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Ian Rogers <irogers@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-perf-users@vger.kernel.org, bpf@vger.kernel.org,
	Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH 2/3] perf build: Use libtraceevent from the system
Date: Thu, 8 Dec 2022 19:04:51 -0300	[thread overview]
Message-ID: <Y5JfgyN59dSeKbUP@kernel.org> (raw)
In-Reply-To: <36CD1041-0CAE-41C1-8086-C17854531B3E@linux.vnet.ibm.com>

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.

- Arnaldo

commit d288f6c03038e8f14f0b53bd4ccc03f664c8c744
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Thu Dec 8 18:26:05 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..b568be802dc873ed 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))
 
@@ -92,7 +93,7 @@ all: $(FILES)
 __BUILD = $(CC) $(CFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.c,$(@F)) $(LDFLAGS)
   BUILD = $(__BUILD) > $(@:.bin=.make.output) 2>&1
   BUILD_BFD = $(BUILD) -DPACKAGE='"perf"' -lbfd -ldl
-  BUILD_ALL = $(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -lslang $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz -llzma -lzstd -lcap
+  BUILD_ALL = $(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -lslang $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz -llzma -lzstd -lcap -ltraceevent
 
 __BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.cpp,$(@F)) $(LDFLAGS)
   BUILDXX = $(__BUILDXX) > $(@:.bin=.make.output) 2>&1
@@ -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/build/feature/test-libtraceevent-tep_field_is_relative.c b/tools/build/feature/test-libtraceevent-tep_field_is_relative.c
new file mode 100644
index 0000000000000000..b0664d727d1bf452
--- /dev/null
+++ b/tools/build/feature/test-libtraceevent-tep_field_is_relative.c
@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <traceevent/event-parse.h>
+
+int main(void)
+{
+	return TEP_FIELD_IS_RELATIVE;
+}
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 9cc3c48f32881c8b..3d0694bece5ed08b 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -1192,9 +1192,14 @@ ifdef LIBTRACEEVENT_DYNAMIC
     LIBTRACEEVENT_VERSION_3 := $(word 3, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
     LIBTRACEEVENT_VERSION_CPP := $(shell expr $(LIBTRACEEVENT_VERSION_1) \* 255 \* 255 + $(LIBTRACEEVENT_VERSION_2) \* 255 + $(LIBTRACEEVENT_VERSION_3))
     CFLAGS += -DLIBTRACEEVENT_VERSION=$(LIBTRACEEVENT_VERSION_CPP)
-  else
-    dummy := $(error Error: No libtraceevent devel library found, please install libtraceevent-devel);
+    $(call feature_check,libtraceevent-tep_field_is_relative)
+    ifeq ($(feature-libtraceevent-tep_field_is_relative), 1)
+      CFLAGS += -DHAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE
+    endif
   endif
+else
+  # Its the in-kernel tools/lib/traceevent/, so we _have_ TEP_FIELD_IS_RELATIVE
+  CFLAGS += -DHAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE
 endif
 
 ifdef LIBTRACEFS_DYNAMIC
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 543c379d2a57a05a..e38c3b4fc5a69160 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2726,8 +2726,7 @@ 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;
-				if (field->flags & TEP_FIELD_IS_RELATIVE)
-					offset += field->offset + field->size;
+				offset += traceevent__field_adjust_relative_offset(field);
 			}
 
 			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 c65cdaf6975eddeb..955b2a110c7f0b33 100644
--- a/tools/perf/util/data-convert-bt.c
+++ b/tools/perf/util/data-convert-bt.c
@@ -319,8 +319,7 @@ static int add_tracepoint_field_value(struct ctf_writer *cw,
 		offset = tmp_val;
 		len = offset >> 16;
 		offset &= 0xffff;
-		if (flags & TEP_FIELD_IS_RELATIVE)
-			offset += fmtf->offset + fmtf->size;
+		offset += traceevent__field_adjust_relative_offset(fmtf);
 	}
 
 	if (flags & TEP_FIELD_IS_ARRAY) {
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 0f617359a82f2891..45a4090c78337b4d 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2776,8 +2776,7 @@ 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;
-		if (field->flags & TEP_FIELD_IS_RELATIVE)
-			offset += field->offset + field->size;
+		offset += traceevent__field_adjust_relative_offset(field);
 	}
 
 	return sample->raw_data + offset;
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index b5941c74a0d64e10..e46d6de537b1c448 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -439,8 +439,7 @@ tracepoint_field(struct pyrf_event *pe, struct tep_format_field *field)
 			offset  = val;
 			len     = offset >> 16;
 			offset &= 0xffff;
-			if (field->flags & TEP_FIELD_IS_RELATIVE)
-				offset += field->offset + field->size;
+			offset += traceevent__field_adjust_relative_offset(field);
 		}
 		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 5b602b6d46854133..05814d0c99a43171 100644
--- a/tools/perf/util/scripting-engines/trace-event-perl.c
+++ b/tools/perf/util/scripting-engines/trace-event-perl.c
@@ -392,8 +392,7 @@ static void perl_process_tracepoint(struct perf_sample *sample,
 			if (field->flags & TEP_FIELD_IS_DYNAMIC) {
 				offset = *(int *)(data + field->offset);
 				offset &= 0xffff;
-				if (field->flags & TEP_FIELD_IS_RELATIVE)
-					offset += field->offset + field->size;
+				offset += traceevent__field_adjust_relative_offset(field);
 			} 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 d685a7399ee2e6c1..4bceba8dde6e05a0 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -993,8 +993,7 @@ static void python_process_tracepoint(struct perf_sample *sample,
 				offset  = val;
 				len     = offset >> 16;
 				offset &= 0xffff;
-				if (field->flags & TEP_FIELD_IS_RELATIVE)
-					offset += field->offset + field->size;
+				offset += traceevent__field_adjust_relative_offset(field);
 			}
 			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 2e7330867e2efd42..dbabe83be43f6095 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -23,6 +23,7 @@
 #include "strlist.h"
 #include "strbuf.h"
 #include <traceevent/event-parse.h>
+#include "trace-event.h"
 #include "mem-events.h"
 #include "annotate.h"
 #include "event.h"
@@ -2543,9 +2544,7 @@ 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;
-		if (field->flags & TEP_FIELD_IS_RELATIVE)
-			offset += field->offset + field->size;
-
+		offset += traceevent__field_adjust_relative_offset(field);
 		/* record max width for output */
 		if (size > hde->dynamic_len)
 			hde->dynamic_len = size;
diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
index 8f39f5bcb2c268fd..78246e6fb25a620b 100644
--- a/tools/perf/util/trace-event.h
+++ b/tools/perf/util/trace-event.h
@@ -2,6 +2,7 @@
 #ifndef _PERF_UTIL_TRACE_EVENT_H
 #define _PERF_UTIL_TRACE_EVENT_H
 
+#include <linux/compiler.h>
 #include <traceevent/event-parse.h>
 #include "parse-events.h"
 
@@ -129,4 +130,14 @@ int common_lock_depth(struct scripting_context *context);
 #define SAMPLE_FLAGS_BUF_SIZE 64
 int perf_sample__sprintf_flags(u32 flags, char *str, size_t sz);
 
+
+static inline int traceevent__field_adjust_relative_offset(struct tep_format_field *field __maybe_unused)
+{
+#ifdef HAVE_LIBTRACEEVENT_TEP_FIELD_IS_RELATIVE
+	if (field->flags & TEP_FIELD_IS_RELATIVE)
+		return field->offset + field->size;
+#endif
+	return 0;
+}
+
 #endif /* _PERF_UTIL_TRACE_EVENT_H */

  reply	other threads:[~2022-12-08 22:06 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-05 22:59 [PATCH 0/3] libtraceevent from system and build fix Ian Rogers
2022-12-05 22:59 ` [PATCH 1/3] perf build: Fixes for LIBTRACEEVENT_DYNAMIC Ian Rogers
2022-12-05 22:59 ` [PATCH 2/3] perf build: Use libtraceevent from the system Ian Rogers
2022-12-06 16:15   ` Arnaldo Carvalho de Melo
2022-12-06 16:20     ` Arnaldo Carvalho de Melo
2022-12-06 16:23       ` Arnaldo Carvalho de Melo
2022-12-06 16:30         ` Arnaldo Carvalho de Melo
2022-12-06 16:37           ` Arnaldo Carvalho de Melo
2022-12-06 16:41             ` Arnaldo Carvalho de Melo
2022-12-06 17:01               ` Arnaldo Carvalho de Melo
2022-12-06 17:07                 ` Arnaldo Carvalho de Melo
2022-12-06 17:13                   ` Arnaldo Carvalho de Melo
2022-12-06 17:31                     ` Arnaldo Carvalho de Melo
2022-12-06 22:22                       ` Ian Rogers
2022-12-07 14:21                         ` [ALMOST ready] " Arnaldo Carvalho de Melo
2022-12-07 14:31                           ` Arnaldo Carvalho de Melo
2022-12-07 14:33                             ` Arnaldo Carvalho de Melo
2022-12-07 14:39                             ` Steven Rostedt
2022-12-07 16:02                               ` Arnaldo Carvalho de Melo
2022-12-07 16:56                                 ` Arnaldo Carvalho de Melo
2022-12-07 17:00                                 ` Ian Rogers
2022-12-07 14:37                           ` Arnaldo Carvalho de Melo
2022-12-07 13:38                       ` Athira Rajeev
2022-12-07 17:27                         ` Arnaldo Carvalho de Melo
2022-12-07 17:31                           ` Arnaldo Carvalho de Melo
2022-12-07 17:43                             ` Arnaldo Carvalho de Melo
2022-12-08  6:51                           ` Athira Rajeev
2022-12-08 22:04                             ` Arnaldo Carvalho de Melo [this message]
2022-12-08 22:32                               ` Arnaldo Carvalho de Melo
2022-12-08 23:00                                 ` Ian Rogers
2022-12-08 23:05                                   ` Ian Rogers
2022-12-12 14:13                                     ` Arnaldo Carvalho de Melo
2022-12-12 14:28                                     ` Arnaldo Carvalho de Melo
2022-12-09  6:34                                 ` Athira Rajeev
2022-12-12 13:51                                   ` Arnaldo Carvalho de Melo
2022-12-13  9:53                                     ` Athira Rajeev
2022-12-13 22:09                                       ` Ian Rogers
2022-12-13 22:33                                         ` Arnaldo Carvalho de Melo
2022-12-13 22:47                                           ` Ian Rogers
2022-12-15  7:10                                         ` Athira Rajeev
2022-12-07 13:33   ` Athira Rajeev
2022-12-07 13:46   ` Athira Rajeev
2022-12-07 16:16     ` Ian Rogers
2022-12-07 16:52     ` Arnaldo Carvalho de Melo
2022-12-05 22:59 ` [PATCH 3/3] perf build: Fix python/perf.so library's name Ian Rogers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y5JfgyN59dSeKbUP@kernel.org \
    --to=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=atrajeev@linux.vnet.ibm.com \
    --cc=bpf@vger.kernel.org \
    --cc=eranian@google.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.