[PATCH v3 1/4] perf: Enable libtracefs dynamic linking

[PATCH v3 1/4] perf: Enable libtracefs dynamic linking

[Ian Rogers]

Currently libtracefs isn't used by perf, but there are potential
improvements by using it as identified Steven Rostedt's e-mail:
https://lore.kernel.org/lkml/20210610154759.1ef958f0@oasis.local.home/

This change is modelled on the dynamic libtraceevent patch by Michael
Petlan:
https://lore.kernel.org/linux-perf-users/20210428092023.4009-1-mpetlan@redhat.com/

v3. Adds file missed in v1 and v2 spotted by Jiri Olsa.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/build/Makefile.feature          |  1 +
 tools/build/feature/Makefile          |  4 ++++
 tools/build/feature/test-libtracefs.c | 10 ++++++++++
 tools/perf/Makefile.config            |  9 +++++++++
 tools/perf/Makefile.perf              |  2 ++
 5 files changed, 26 insertions(+)
 create mode 100644 tools/build/feature/test-libtracefs.c

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 3dd2f68366f9..45a9a59828c3 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -52,6 +52,7 @@ FEATURE_TESTS_BASIC :=                  \
         libslang                        \
         libslang-include-subdir         \
         libtraceevent                   \
+        libtracefs                      \
         libcrypto                       \
         libunwind                       \
         pthread-attr-setaffinity-np     \
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index eff55d287db1..d024b5204ba0 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -36,6 +36,7 @@ FILES=                                          \
          test-libslang.bin                      \
          test-libslang-include-subdir.bin       \
          test-libtraceevent.bin                 \
+         test-libtracefs.bin                    \
          test-libcrypto.bin                     \
          test-libunwind.bin                     \
          test-libunwind-debug-frame.bin         \
@@ -199,6 +200,9 @@ $(OUTPUT)test-libslang-include-subdir.bin:
 $(OUTPUT)test-libtraceevent.bin:
 	$(BUILD) -ltraceevent
 
+$(OUTPUT)test-libtracefs.bin:
+	$(BUILD) -ltracefs
+
 $(OUTPUT)test-libcrypto.bin:
 	$(BUILD) -lcrypto
 
diff --git a/tools/build/feature/test-libtracefs.c b/tools/build/feature/test-libtracefs.c
new file mode 100644
index 000000000000..8eff16c0c10b
--- /dev/null
+++ b/tools/build/feature/test-libtracefs.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <tracefs/tracefs.h>
+
+int main(void)
+{
+	struct tracefs_instance *inst = tracefs_instance_create("dummy");
+
+	tracefs_instance_destroy(inst);
+	return 0;
+}
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 446180401e26..00ec900ddbca 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -1098,6 +1098,15 @@ ifdef LIBTRACEEVENT_DYNAMIC
   endif
 endif
 
+ifdef LIBTRACEFS_DYNAMIC
+  $(call feature_check,libtracefs)
+  ifeq ($(feature-libtracefs), 1)
+    EXTLIBS += -ltracefs
+  else
+    dummy := $(error Error: No libtracefs devel library found, please install libtracefs-dev);
+  endif
+endif
+
 # Among the variables below, these:
 #   perfexecdir
 #   perf_include_dir
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index e04313c4d840..7df13e74450c 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -130,6 +130,8 @@ include ../scripts/utilities.mak
 #
 # Define LIBTRACEEVENT_DYNAMIC to enable libtraceevent dynamic linking
 #
+# Define LIBTRACEFS_DYNAMIC to enable libtracefs dynamic linking
+#
 
 # As per kernel Makefile, avoid funny character set dependencies
 unexport LC_ALL
-- 
2.33.0.464.g1972c5931b-goog

[PATCH v3 2/4] perf: Add define for libtraceevent version

[Ian Rogers]

The definition is derived from pkg-config as discussed in:
https://lore.kernel.org/lkml/20210610155915.20a252d3@oasis.local.home/
The definition is computed using expr rather than passed to be computed
in C code, this avoids complications with quote  in the variable
expansions. For example see the target python/perf.so in Makefile.perf.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/Makefile.config | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 00ec900ddbca..2001c315f0db 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -1093,6 +1093,12 @@ ifdef LIBTRACEEVENT_DYNAMIC
   $(call feature_check,libtraceevent)
   ifeq ($(feature-libtraceevent), 1)
     EXTLIBS += -ltraceevent
+    LIBTRACEEVENT_VERSION := $(shell $(PKG_CONFIG) --modversion libtraceevent)
+    LIBTRACEEVENT_VERSION_1 := $(word 1, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
+    LIBTRACEEVENT_VERSION_2 := $(word 2, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
+    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);
   endif
-- 
2.33.0.464.g1972c5931b-goog

[PATCH v3 3/4] perf: Add define for libtracefs version

[Ian Rogers]

This will allow version specific support of libtracefs.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/Makefile.config | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 2001c315f0db..0ae2e3d8b832 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -1108,6 +1108,12 @@ ifdef LIBTRACEFS_DYNAMIC
   $(call feature_check,libtracefs)
   ifeq ($(feature-libtracefs), 1)
     EXTLIBS += -ltracefs
+    LIBTRACEFS_VERSION := $(shell $(PKG_CONFIG) --modversion libtracefs)
+    LIBTRACEFS_VERSION_1 := $(word 1, $(subst ., ,$(LIBTRACEFS_VERSION)))
+    LIBTRACEFS_VERSION_2 := $(word 2, $(subst ., ,$(LIBTRACEFS_VERSION)))
+    LIBTRACEFS_VERSION_3 := $(word 3, $(subst ., ,$(LIBTRACEFS_VERSION)))
+    LIBTRACEFS_VERSION_CPP := $(shell expr $(LIBTRACEFS_VERSION_1) \* 255 \* 255 + $(LIBTRACEFS_VERSION_2) \* 255 + $(LIBTRACEFS_VERSION_3))
+    CFLAGS += -DLIBTRACEFS_VERSION=$(LIBTRACEFS_VERSION_CPP)
   else
     dummy := $(error Error: No libtracefs devel library found, please install libtracefs-dev);
   endif
-- 
2.33.0.464.g1972c5931b-goog

[PATCH v3 4/4] libtraceevent: Increase libtraceevent logging when verbose

[Ian Rogers]

libtraceevent has added more levels of debug printout and with changes
like:
https://lore.kernel.org/linux-trace-devel/20210507095022.1079364-3-tz.stoyanov@gmail.com
previously generated output like "registering plugin" is no longer
displayed. This change makes it so that if perf's verbose debug output
is enabled then the debug and info libtraceevent messages can be
displayed.
The code is conditionally enabled based on the libtraceevent version as
discussed in the RFC:
https://lore.kernel.org/lkml/20210610060643.595673-1-irogers@google.com/

v2. Is a rebase and handles the case of building without
    LIBTRACEEVENT_DYNAMIC.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/debug.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 2c06abf6dcd2..c7a9fa0ffae9 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -24,6 +24,16 @@
 #include "util/parse-sublevel-options.h"
 
 #include <linux/ctype.h>
+#include <traceevent/event-parse.h>
+
+#define MAKE_LIBTRACEEVENT_VERSION(a, b, c) ((a)*255*255+(b)*255+(c))
+#ifndef LIBTRACEEVENT_VERSION
+/*
+ * If LIBTRACEEVENT_VERSION wasn't computed then set to version 1.1.0 that ships
+ * with the Linux kernel tools.
+ */
+#define LIBTRACEEVENT_VERSION MAKE_LIBTRACEEVENT_VERSION(1, 1, 0)
+#endif
 
 int verbose;
 int debug_peo_args;
@@ -228,6 +238,15 @@ int perf_debug_option(const char *str)
 	/* Allow only verbose value in range (0, 10), otherwise set 0. */
 	verbose = (verbose < 0) || (verbose > 10) ? 0 : verbose;
 
+#if MAKE_LIBTRACEEVENT_VERSION(1, 3, 0) <= LIBTRACEEVENT_VERSION
+	if (verbose == 1)
+		tep_set_loglevel(TEP_LOG_INFO);
+	else if (verbose == 2)
+		tep_set_loglevel(TEP_LOG_DEBUG);
+	else if (verbose >= 3)
+		tep_set_loglevel(TEP_LOG_ALL);
+#endif
+
 	return 0;
 }
 
-- 
2.33.0.464.g1972c5931b-goog

Re: [PATCH v3 1/4] perf: Enable libtracefs dynamic linking

[Arnaldo Carvalho de Melo]

Em Wed, Sep 22, 2021 at 05:10:21PM -0700, Ian Rogers escreveu:
> Currently libtracefs isn't used by perf, but there are potential
> improvements by using it as identified Steven Rostedt's e-mail:
> https://lore.kernel.org/lkml/20210610154759.1ef958f0@oasis.local.home/
> 
> This change is modelled on the dynamic libtraceevent patch by Michael
> Petlan:
> https://lore.kernel.org/linux-perf-users/20210428092023.4009-1-mpetlan@redhat.com/
> 
> v3. Adds file missed in v1 and v2 spotted by Jiri Olsa.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/build/Makefile.feature          |  1 +
>  tools/build/feature/Makefile          |  4 ++++
>  tools/build/feature/test-libtracefs.c | 10 ++++++++++
>  tools/perf/Makefile.config            |  9 +++++++++
>  tools/perf/Makefile.perf              |  2 ++
>  5 files changed, 26 insertions(+)
>  create mode 100644 tools/build/feature/test-libtracefs.c
> 
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 3dd2f68366f9..45a9a59828c3 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -52,6 +52,7 @@ FEATURE_TESTS_BASIC :=                  \
>          libslang                        \
>          libslang-include-subdir         \
>          libtraceevent                   \
> +        libtracefs                      \
>          libcrypto                       \
>          libunwind                       \
>          pthread-attr-setaffinity-np     \
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index eff55d287db1..d024b5204ba0 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -36,6 +36,7 @@ FILES=                                          \
>           test-libslang.bin                      \
>           test-libslang-include-subdir.bin       \
>           test-libtraceevent.bin                 \
> +         test-libtracefs.bin                    \
>           test-libcrypto.bin                     \
>           test-libunwind.bin                     \
>           test-libunwind-debug-frame.bin         \
> @@ -199,6 +200,9 @@ $(OUTPUT)test-libslang-include-subdir.bin:
>  $(OUTPUT)test-libtraceevent.bin:
>  	$(BUILD) -ltraceevent
>  
> +$(OUTPUT)test-libtracefs.bin:
> +	$(BUILD) -ltracefs
> +
>  $(OUTPUT)test-libcrypto.bin:
>  	$(BUILD) -lcrypto
>  
> diff --git a/tools/build/feature/test-libtracefs.c b/tools/build/feature/test-libtracefs.c
> new file mode 100644
> index 000000000000..8eff16c0c10b
> --- /dev/null
> +++ b/tools/build/feature/test-libtracefs.c
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <tracefs/tracefs.h>
> +
> +int main(void)
> +{
> +	struct tracefs_instance *inst = tracefs_instance_create("dummy");
> +
> +	tracefs_instance_destroy(inst);
> +	return 0;
> +}
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 446180401e26..00ec900ddbca 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -1098,6 +1098,15 @@ ifdef LIBTRACEEVENT_DYNAMIC
>    endif
>  endif
>  
> +ifdef LIBTRACEFS_DYNAMIC
> +  $(call feature_check,libtracefs)
> +  ifeq ($(feature-libtracefs), 1)
> +    EXTLIBS += -ltracefs
> +  else
> +    dummy := $(error Error: No libtracefs devel library found, please install libtracefs-dev);
> +  endif
> +endif
> +
>  # Among the variables below, these:
>  #   perfexecdir
>  #   perf_include_dir
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index e04313c4d840..7df13e74450c 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -130,6 +130,8 @@ include ../scripts/utilities.mak
>  #
>  # Define LIBTRACEEVENT_DYNAMIC to enable libtraceevent dynamic linking
>  #
> +# Define LIBTRACEFS_DYNAMIC to enable libtracefs dynamic linking
> +#
>  
>  # As per kernel Makefile, avoid funny character set dependencies
>  unexport LC_ALL
> -- 
> 2.33.0.464.g1972c5931b-goog

-- 

- Arnaldo

Re: [PATCH v3 4/4] libtraceevent: Increase libtraceevent logging when verbose

[Jiri Slaby]

Hi,

On 23. 09. 21, 2:10, Ian Rogers wrote:
> libtraceevent has added more levels of debug printout and with changes
> like:
> https://lore.kernel.org/linux-trace-devel/20210507095022.1079364-3-tz.stoyanov@gmail.com
> previously generated output like "registering plugin" is no longer
> displayed. This change makes it so that if perf's verbose debug output
> is enabled then the debug and info libtraceevent messages can be
> displayed.
> The code is conditionally enabled based on the libtraceevent version as
> discussed in the RFC:
> https://lore.kernel.org/lkml/20210610060643.595673-1-irogers@google.com/
> 
> v2. Is a rebase and handles the case of building without
>      LIBTRACEEVENT_DYNAMIC.

It breaks build with LIBTRACEEVENT_DYNAMIC and version 1.3.0:
 > util/debug.c: In function ‘perf_debug_option’:
 > util/debug.c:243:17: error: implicit declaration of function 
‘tep_set_loglevel’ [-Werror=implicit-function-declaration]
 >   243 |                 tep_set_loglevel(TEP_LOG_INFO);
 >       |                 ^~~~~~~~~~~~~~~~
 > util/debug.c:243:34: error: ‘TEP_LOG_INFO’ undeclared (first use in 
this function); did you mean ‘TEP_PRINT_INFO’?
 >   243 |                 tep_set_loglevel(TEP_LOG_INFO);
 >       |                                  ^~~~~~~~~~~~
 >       |                                  TEP_PRINT_INFO
 > util/debug.c:243:34: note: each undeclared identifier is reported 
only once for each function it appears in
 > util/debug.c:245:34: error: ‘TEP_LOG_DEBUG’ undeclared (first use in 
this function)
 >   245 |                 tep_set_loglevel(TEP_LOG_DEBUG);
 >       |                                  ^~~~~~~~~~~~~
 > util/debug.c:247:34: error: ‘TEP_LOG_ALL’ undeclared (first use in 
this function)
 >   247 |                 tep_set_loglevel(TEP_LOG_ALL);
 >       |                                  ^~~~~~~~~~~

It is because the gcc's command line looks like:
gcc
...
-I/home/abuild/rpmbuild/BUILD/tools/lib/
...
-DLIBTRACEEVENT_VERSION=65790
...

So:


> --- a/tools/perf/util/debug.c
> +++ b/tools/perf/util/debug.c
> @@ -24,6 +24,16 @@
>   #include "util/parse-sublevel-options.h"
>   
>   #include <linux/ctype.h>
> +#include <traceevent/event-parse.h>

/home/abuild/rpmbuild/BUILD/tools/lib/traceevent/event-parse.h is taken 
here.

> @@ -228,6 +238,15 @@ int perf_debug_option(const char *str)
>   	/* Allow only verbose value in range (0, 10), otherwise set 0. */
>   	verbose = (verbose < 0) || (verbose > 10) ? 0 : verbose;
>   
> +#if MAKE_LIBTRACEEVENT_VERSION(1, 3, 0) <= LIBTRACEEVENT_VERSION

But
-DLIBTRACEEVENT_VERSION=65790
is taken here (which is 1.3.0).

> +	if (verbose == 1)
> +		tep_set_loglevel(TEP_LOG_INFO);
> +	else if (verbose == 2)
> +		tep_set_loglevel(TEP_LOG_DEBUG);
> +	else if (verbose >= 3)
> +		tep_set_loglevel(TEP_LOG_ALL);
> +#endif
> +
>   	return 0;
>   }
>   

regards,
-- 
js
suse labs

[was: libtraceevent: Increase libtraceevent logging when verbose]

[Jiri Slaby]

Ping -- perf build is still broken in 5.16-rc8 due to this.

On 16. 12. 21, 9:07, Jiri Slaby wrote:
> Hi,
> 
> On 23. 09. 21, 2:10, Ian Rogers wrote:
>> libtraceevent has added more levels of debug printout and with changes
>> like:
>> https://lore.kernel.org/linux-trace-devel/20210507095022.1079364-3-tz.stoyanov@gmail.com 
>>
>> previously generated output like "registering plugin" is no longer
>> displayed. This change makes it so that if perf's verbose debug output
>> is enabled then the debug and info libtraceevent messages can be
>> displayed.
>> The code is conditionally enabled based on the libtraceevent version as
>> discussed in the RFC:
>> https://lore.kernel.org/lkml/20210610060643.595673-1-irogers@google.com/
>>
>> v2. Is a rebase and handles the case of building without
>>      LIBTRACEEVENT_DYNAMIC.
> 
> It breaks build with LIBTRACEEVENT_DYNAMIC and version 1.3.0:
>  > util/debug.c: In function ‘perf_debug_option’:
>  > util/debug.c:243:17: error: implicit declaration of function 
> ‘tep_set_loglevel’ [-Werror=implicit-function-declaration]
>  >   243 |                 tep_set_loglevel(TEP_LOG_INFO);
>  >       |                 ^~~~~~~~~~~~~~~~
>  > util/debug.c:243:34: error: ‘TEP_LOG_INFO’ undeclared (first use in 
> this function); did you mean ‘TEP_PRINT_INFO’?
>  >   243 |                 tep_set_loglevel(TEP_LOG_INFO);
>  >       |                                  ^~~~~~~~~~~~
>  >       |                                  TEP_PRINT_INFO
>  > util/debug.c:243:34: note: each undeclared identifier is reported 
> only once for each function it appears in
>  > util/debug.c:245:34: error: ‘TEP_LOG_DEBUG’ undeclared (first use in 
> this function)
>  >   245 |                 tep_set_loglevel(TEP_LOG_DEBUG);
>  >       |                                  ^~~~~~~~~~~~~
>  > util/debug.c:247:34: error: ‘TEP_LOG_ALL’ undeclared (first use in 
> this function)
>  >   247 |                 tep_set_loglevel(TEP_LOG_ALL);
>  >       |                                  ^~~~~~~~~~~
> 
> It is because the gcc's command line looks like:
> gcc
> ...
> -I/home/abuild/rpmbuild/BUILD/tools/lib/
> ...
> -DLIBTRACEEVENT_VERSION=65790
> ...
> 
> So:
> 
> 
>> --- a/tools/perf/util/debug.c
>> +++ b/tools/perf/util/debug.c
>> @@ -24,6 +24,16 @@
>>   #include "util/parse-sublevel-options.h"
>>   #include <linux/ctype.h>
>> +#include <traceevent/event-parse.h>
> 
> /home/abuild/rpmbuild/BUILD/tools/lib/traceevent/event-parse.h is taken 
> here.
> 
>> @@ -228,6 +238,15 @@ int perf_debug_option(const char *str)
>>       /* Allow only verbose value in range (0, 10), otherwise set 0. */
>>       verbose = (verbose < 0) || (verbose > 10) ? 0 : verbose;
>> +#if MAKE_LIBTRACEEVENT_VERSION(1, 3, 0) <= LIBTRACEEVENT_VERSION
> 
> But
> -DLIBTRACEEVENT_VERSION=65790
> is taken here (which is 1.3.0).
> 
>> +    if (verbose == 1)
>> +        tep_set_loglevel(TEP_LOG_INFO);
>> +    else if (verbose == 2)
>> +        tep_set_loglevel(TEP_LOG_DEBUG);
>> +    else if (verbose >= 3)
>> +        tep_set_loglevel(TEP_LOG_ALL);
>> +#endif
>> +
>>       return 0;
>>   }
> 
> regards,


-- 
js
suse labs

Re: [was: libtraceevent: Increase libtraceevent logging when verbose]

[Arnaldo Carvalho de Melo]

Em Mon, Jan 03, 2022 at 09:25:26AM +0100, Jiri Slaby escreveu:
> Ping -- perf build is still broken in 5.16-rc8 due to this.

Back from year end vacation, I'll try to fix this, from a quick read it
uses in tree tools/lib/traceevent headers while building with the distro
provided libtraceevent-devel package.

- Arnaldo
 
> On 16. 12. 21, 9:07, Jiri Slaby wrote:
> > Hi,
> > 
> > On 23. 09. 21, 2:10, Ian Rogers wrote:
> > > libtraceevent has added more levels of debug printout and with changes
> > > like:
> > > https://lore.kernel.org/linux-trace-devel/20210507095022.1079364-3-tz.stoyanov@gmail.com
> > > 
> > > previously generated output like "registering plugin" is no longer
> > > displayed. This change makes it so that if perf's verbose debug output
> > > is enabled then the debug and info libtraceevent messages can be
> > > displayed.
> > > The code is conditionally enabled based on the libtraceevent version as
> > > discussed in the RFC:
> > > https://lore.kernel.org/lkml/20210610060643.595673-1-irogers@google.com/
> > > 
> > > v2. Is a rebase and handles the case of building without
> > >      LIBTRACEEVENT_DYNAMIC.
> > 
> > It breaks build with LIBTRACEEVENT_DYNAMIC and version 1.3.0:
> >  > util/debug.c: In function ‘perf_debug_option’:
> >  > util/debug.c:243:17: error: implicit declaration of function
> > ‘tep_set_loglevel’ [-Werror=implicit-function-declaration]
> >  >   243 |                 tep_set_loglevel(TEP_LOG_INFO);
> >  >       |                 ^~~~~~~~~~~~~~~~
> >  > util/debug.c:243:34: error: ‘TEP_LOG_INFO’ undeclared (first use in
> > this function); did you mean ‘TEP_PRINT_INFO’?
> >  >   243 |                 tep_set_loglevel(TEP_LOG_INFO);
> >  >       |                                  ^~~~~~~~~~~~
> >  >       |                                  TEP_PRINT_INFO
> >  > util/debug.c:243:34: note: each undeclared identifier is reported
> > only once for each function it appears in
> >  > util/debug.c:245:34: error: ‘TEP_LOG_DEBUG’ undeclared (first use in
> > this function)
> >  >   245 |                 tep_set_loglevel(TEP_LOG_DEBUG);
> >  >       |                                  ^~~~~~~~~~~~~
> >  > util/debug.c:247:34: error: ‘TEP_LOG_ALL’ undeclared (first use in
> > this function)
> >  >   247 |                 tep_set_loglevel(TEP_LOG_ALL);
> >  >       |                                  ^~~~~~~~~~~
> > 
> > It is because the gcc's command line looks like:
> > gcc
> > ...
> > -I/home/abuild/rpmbuild/BUILD/tools/lib/
> > ...
> > -DLIBTRACEEVENT_VERSION=65790
> > ...
> > 
> > So:
> > 
> > 
> > > --- a/tools/perf/util/debug.c
> > > +++ b/tools/perf/util/debug.c
> > > @@ -24,6 +24,16 @@
> > >   #include "util/parse-sublevel-options.h"
> > >   #include <linux/ctype.h>
> > > +#include <traceevent/event-parse.h>
> > 
> > /home/abuild/rpmbuild/BUILD/tools/lib/traceevent/event-parse.h is taken
> > here.
> > 
> > > @@ -228,6 +238,15 @@ int perf_debug_option(const char *str)
> > >       /* Allow only verbose value in range (0, 10), otherwise set 0. */
> > >       verbose = (verbose < 0) || (verbose > 10) ? 0 : verbose;
> > > +#if MAKE_LIBTRACEEVENT_VERSION(1, 3, 0) <= LIBTRACEEVENT_VERSION
> > 
> > But
> > -DLIBTRACEEVENT_VERSION=65790
> > is taken here (which is 1.3.0).
> > 
> > > +    if (verbose == 1)
> > > +        tep_set_loglevel(TEP_LOG_INFO);
> > > +    else if (verbose == 2)
> > > +        tep_set_loglevel(TEP_LOG_DEBUG);
> > > +    else if (verbose >= 3)
> > > +        tep_set_loglevel(TEP_LOG_ALL);
> > > +#endif
> > > +
> > >       return 0;
> > >   }
> > 
> > regards,
> 
> 
> -- 
> js
> suse labs

-- 

- Arnaldo

Re: [was: libtraceevent: Increase libtraceevent logging when verbose]

[Steven Rostedt]

On Mon, 3 Jan 2022 12:04:53 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Mon, Jan 03, 2022 at 09:25:26AM +0100, Jiri Slaby escreveu:
> > Ping -- perf build is still broken in 5.16-rc8 due to this.  
> 
> Back from year end vacation, I'll try to fix this, from a quick read it
> uses in tree tools/lib/traceevent headers while building with the distro
> provided libtraceevent-devel package.

I'll try to take a look at this too. I'm pretty much done with my office
renovation, and can now do some volunteer work ;-)

-- Steve

Re: [was: libtraceevent: Increase libtraceevent logging when verbose]

[Arnaldo Carvalho de Melo]

Em Mon, Jan 03, 2022 at 09:25:26AM +0100, Jiri Slaby escreveu:
> Ping -- perf build is still broken in 5.16-rc8 due to this.
> 
> On 16. 12. 21, 9:07, Jiri Slaby wrote:
> > Hi,
> > 
> > On 23. 09. 21, 2:10, Ian Rogers wrote:
> > > libtraceevent has added more levels of debug printout and with changes
> > > like:
> > > https://lore.kernel.org/linux-trace-devel/20210507095022.1079364-3-tz.stoyanov@gmail.com
> > > 
> > > previously generated output like "registering plugin" is no longer
> > > displayed. This change makes it so that if perf's verbose debug output
> > > is enabled then the debug and info libtraceevent messages can be
> > > displayed.
> > > The code is conditionally enabled based on the libtraceevent version as
> > > discussed in the RFC:
> > > https://lore.kernel.org/lkml/20210610060643.595673-1-irogers@google.com/
> > > 
> > > v2. Is a rebase and handles the case of building without
> > >      LIBTRACEEVENT_DYNAMIC.
> > 
> > It breaks build with LIBTRACEEVENT_DYNAMIC and version 1.3.0:
> >  > util/debug.c: In function ‘perf_debug_option’:
> >  > util/debug.c:243:17: error: implicit declaration of function
> > ‘tep_set_loglevel’ [-Werror=implicit-function-declaration]
> >  >   243 |                 tep_set_loglevel(TEP_LOG_INFO);
> >  >       |                 ^~~~~~~~~~~~~~~~
> >  > util/debug.c:243:34: error: ‘TEP_LOG_INFO’ undeclared (first use in
> > this function); did you mean ‘TEP_PRINT_INFO’?
> >  >   243 |                 tep_set_loglevel(TEP_LOG_INFO);
> >  >       |                                  ^~~~~~~~~~~~
> >  >       |                                  TEP_PRINT_INFO
> >  > util/debug.c:243:34: note: each undeclared identifier is reported
> > only once for each function it appears in
> >  > util/debug.c:245:34: error: ‘TEP_LOG_DEBUG’ undeclared (first use in
> > this function)
> >  >   245 |                 tep_set_loglevel(TEP_LOG_DEBUG);
> >  >       |                                  ^~~~~~~~~~~~~
> >  > util/debug.c:247:34: error: ‘TEP_LOG_ALL’ undeclared (first use in
> > this function)
> >  >   247 |                 tep_set_loglevel(TEP_LOG_ALL);
> >  >       |                                  ^~~~~~~~~~~
> > 
> > It is because the gcc's command line looks like:
> > gcc
> > ...
> > -I/home/abuild/rpmbuild/BUILD/tools/lib/
> > ...
> > -DLIBTRACEEVENT_VERSION=65790
> > ...
> > 
> > So:
> > 
> > 
> > > --- a/tools/perf/util/debug.c
> > > +++ b/tools/perf/util/debug.c
> > > @@ -24,6 +24,16 @@
> > >   #include "util/parse-sublevel-options.h"
> > >   #include <linux/ctype.h>
> > > +#include <traceevent/event-parse.h>
> > 
> > /home/abuild/rpmbuild/BUILD/tools/lib/traceevent/event-parse.h is taken
> > here.
> > 
> > > @@ -228,6 +238,15 @@ int perf_debug_option(const char *str)
> > >       /* Allow only verbose value in range (0, 10), otherwise set 0. */
> > >       verbose = (verbose < 0) || (verbose > 10) ? 0 : verbose;
> > > +#if MAKE_LIBTRACEEVENT_VERSION(1, 3, 0) <= LIBTRACEEVENT_VERSION
> > 
> > But
> > -DLIBTRACEEVENT_VERSION=65790
> > is taken here (which is 1.3.0).
> > 
> > > +    if (verbose == 1)
> > > +        tep_set_loglevel(TEP_LOG_INFO);
> > > +    else if (verbose == 2)
> > > +        tep_set_loglevel(TEP_LOG_DEBUG);
> > > +    else if (verbose >= 3)
> > > +        tep_set_loglevel(TEP_LOG_ALL);
> > > +#endif
> > > +
> > >       return 0;
> > >   }

Tried reproducing here on fedora 34 but it is at 1.1.1:

make: Leaving directory '/var/home/acme/git/perf/tools/perf'

 Performance counter stats for 'make -k LIBTRACEEVENT_DYNAMIC=1 BUILD_BPF_SKEL=1 CORESIGHT=1 PYTHON=python3 O=/tmp/build/perf -C tools/perf install-bin':

   397,176,418,790      cycles:u                                                    
   493,885,896,752      instructions:u            #    1.24  insn per cycle         

       8.664957770 seconds time elapsed

      97.706894000 seconds user
      23.714293000 seconds sys


On branch perf/urgent
Your branch is ahead of 'quaco/perf/urgent' by 369 commits.
  (use "git push" to publish your local commits)

nothing to commit, working tree clean
19: 'import perf' in python                                         : Ok
⬢[acme@toolbox perf]$ git diff
⬢[acme@toolbox perf]$ rpm -q libtraceevent-devel
libtraceevent-devel-1.1.1-2.fc34.x86_64

Lemme see in a container so that I can also test the fix.

- Arnaldo

Re: [was: libtraceevent: Increase libtraceevent logging when verbose]

[Jiri Slaby]

On 03. 01. 22, 16:15, Arnaldo Carvalho de Melo wrote:
> Tried reproducing here on fedora 34 but it is at 1.1.1:

FWIW, it's openSUSE Tumbleweed:
$ rpm -q libtraceevent1
libtraceevent1-1.3.0-1.3.x86_64

thanks,
-- 
js
suse labs

Re: [was: libtraceevent: Increase libtraceevent logging when verbose]

[Ian Rogers]

On Mon, Jan 3, 2022 at 10:17 PM Jiri Slaby <jirislaby@kernel.org> wrote:
>
> On 03. 01. 22, 16:15, Arnaldo Carvalho de Melo wrote:
> > Tried reproducing here on fedora 34 but it is at 1.1.1:
>
> FWIW, it's openSUSE Tumbleweed:
> $ rpm -q libtraceevent1
> libtraceevent1-1.3.0-1.3.x86_64

I was able to reproduce in an opensuse tumbleweed container.

$ pkg-config --modversion libtraceevent
1.3.0

$ make -C tools/perf O=/tmp/perf V=1 LIBTRACEEVENT_DYNAMIC=1
...
util/debug.c:245:34: error: 'TEP_LOG_DEBUG' undeclared (first use in
this function)
  245 |                 tep_set_loglevel(TEP_LOG_DEBUG);
      |                                  ^~~~~~~~~~~~~
util/debug.c:247:34: error: 'TEP_LOG_ALL' undeclared (first use in
this function)
  247 |                 tep_set_loglevel(TEP_LOG_ALL);
      |                                  ^~~~~~~~~~~

When I look at libtraceevent tagged as 1.3.0 I see TEP_LOG_ALL and
TEP_LOG_DEBUG:
https://git.kernel.org/pub/scm/libs/libtrace/libtraceevent.git/tree/src/event-parse.h?h=libtraceevent-1.3.0#n760

and checking opensuse's event-parse.h I see the values there. This
makes it look like we're using the kernel libtraceevent header file
(version 1.1.0) rather than the system one.

The command line is:

gcc -Wp,-MD,/tmp/perf/util/.debug.o.d -Wp,-MT,/tmp/perf/util/debug.o
-Wbad-function-cast -Wdeclaration-after-statement -Wformat-security
-Wformat-y2k -Winit-self -Wmissing-declarations -Wmissing-prototypes
-Wno-system-headers -Wold-style-definition -Wpacked -Wredundant-decls
-Wstrict-prototypes -Wswitch-default -Wswitch-enum -Wundef
-Wwrite-strings -Wformat -Wno-type-limits -Wstrict-aliasing=3 -Wshadow
-DHAVE_SYSCALL_TABLE_SUPPORT -DHAVE_ARCH_X86_64_SUPPORT
-I/tmp/perf/arch/x86/include/generated -DHAVE_PERF_REGS_SUPPORT
-DHAVE_ARCH_REGS_QUERY_REGISTER_OFFSET -Werror -O6
-fno-omit-frame-pointer -ggdb3 -funwind-tables -Wall -Wextra
-std=gnu99 -fstack-protector-all -D_FORTIFY_SOURCE=2
-D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
-I/kernel-src/tools/lib/perf/include
-I/kernel-src/tools/perf/util/include
-I/kernel-src/tools/perf/arch/x86/include -I/kernel-src/tools/include/
-I/kernel-src/tools/arch/x86/include/uapi
-I/kernel-src/tools/include/uapi -I/kernel-src/tools/arch/x86/include/
-I/kernel-src/tools/arch/x86/ -I/tmp/perf//util -I/tmp/perf/
-I/kernel-src/tools/perf/util -I/kernel-src/tools/perf
-I/kernel-src/tools/lib/ -DHAVE_PTHREAD_ATTR_SETAFFINITY_NP
-DHAVE_PTHREAD_BARRIER -DHAVE_EVENTFD_SUPPORT
-DHAVE_GET_CURRENT_DIR_NAME -DHAVE_GETTID -DHAVE_FILE_HANDLE
-DHAVE_GLIBC_SUPPORT -DHAVE_AIO_SUPPORT -DHAVE_SCHED_GETCPU_SUPPORT
-DHAVE_SETNS_SUPPORT -DHAVE_LIBELF_SUPPORT
-DHAVE_ELF_GETPHDRNUM_SUPPORT -DHAVE_GELF_GETNOTE_SUPPORT
-DHAVE_ELF_GETSHDRSTRNDX_SUPPORT -DHAVE_LIBBPF_SUPPORT -DHAVE_JITDUMP
-DHAVE_LIBPERL_SUPPORT -DHAVE_TIMERFD_SUPPORT -DNO_LIBPYTHON
-DNO_DEMANGLE -DHAVE_ZLIB_SUPPORT -DHAVE_BACKTRACE_SUPPORT
-DHAVE_KVM_STAT_SUPPORT -DHAVE_AUXTRACE_SUPPORT
-DLIBTRACEEVENT_VERSION=65790 -I/tmp/perf/ -D"BUILD_STR(s)=#s" -c -o
/tmp/perf/util/debug.o util/debug.c

The #include is:
#include <traceevent/event-parse.h>

The system header is at:
/usr/include/traceevent/event-parse.h
The kernel one is at /kernel-src/tools/lib/traceevent/event-parse.h.
Adding a #error to that confirms it is indeed being picked up.

Checking with:
https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html
it is true from case 3 that -I:
3. Directories specified with -I options are scanned in left-to-right order.
has priority over system:
4. Directories specified with -isystem options are scanned in
left-to-right order.
5. Standard system directories are scanned.

Which means we need to lower the priority of
/kernel-src/tools/lib/traceevent or raise the priority of
/usr/include/traceevent/. I thought I might be able to hack this with
EXTRA_CFLAGS="-I/usr/include"
but it still fails even though the -I is by my reading on the left of tools/lib:

gcc -Wp,-MD,/tmp/perf/util/.debug.o.d -Wp,-MT,/tmp/perf/util/debug.o
-I/usr/include -Wbad-function-cast -Wdeclaration-after-statement
-Wformat-security -Wformat-y2k -Winit-self -Wmissing-declarations
-Wmissing-prototypes -Wno-system-headers -Wold-style-definition
-Wpacked -Wredundant-decls -Wstrict-prototypes -Wswitch-default
-Wswitch-enum -Wundef -Wwrite-strings -Wformat -Wno-type-limits
-Wstrict-aliasing=3 -Wshadow -DHAVE_SYSCALL_TABLE_SUPPORT
-DHAVE_ARCH_X86_64_SUPPORT -I/tmp/perf/arch/x86/include/generated
-DHAVE_PERF_REGS_SUPPORT -DHAVE_ARCH_REGS_QUERY_REGISTER_OFFSET
-Werror -O6 -fno-omit-frame-pointer -ggdb3 -funwind-tables -Wall
-Wextra -std=gnu99 -fstack-protector-all -D_FORTIFY_SOURCE=2
-D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
-I/kernel-src/tools/lib/perf/include
-I/kernel-src/tools/perf/util/include
-I/kernel-src/tools/perf/arch/x86/include -I/kernel-src/tools/include/
-I/kernel-src/tools/arch/x86/include/uapi
-I/kernel-src/tools/include/uapi -I/kernel-src/tools/arch/x86/include/
-I/kernel-src/tools/arch/x86/ -I/tmp/perf//util -I/tmp/perf/
-I/kernel-src/tools/perf/util -I/kernel-src/tools/perf
-I/kernel-src/tools/lib/ -DHAVE_PTHREAD_ATTR_SETAFFINITY_NP
-DHAVE_PTHREAD_BARRIER -DHAVE_EVENTFD_SUPPORT
-DHAVE_GET_CURRENT_DIR_NAME -DHAVE_GETTID -DHAVE_FILE_HANDLE
-DHAVE_GLIBC_SUPPORT -DHAVE_AIO_SUPPORT -DHAVE_SCHED_GETCPU_SUPPORT
-DHAVE_SETNS_SUPPORT -DHAVE_LIBELF_SUPPORT
-DHAVE_ELF_GETPHDRNUM_SUPPORT -DHAVE_GELF_GETNOTE_SUPPORT
-DHAVE_ELF_GETSHDRSTRNDX_SUPPORT -DHAVE_LIBBPF_SUPPORT -DHAVE_JITDUMP
-DHAVE_LIBPERL_SUPPORT -DHAVE_TIMERFD_SUPPORT -DNO_LIBPYTHON
-DNO_DEMANGLE -DHAVE_ZLIB_SUPPORT -DHAVE_BACKTRACE_SUPPORT
-DHAVE_KVM_STAT_SUPPORT -DHAVE_AUXTRACE_SUPPORT
-DLIBTRACEEVENT_VERSION=65790 -I/tmp/perf/ -D"BUILD_STR(s)=#s" -c -o
/tmp/perf/util/debug.o util/debug.c

Probably some sysroot related magic. Anyway, doing this 1 line fix
solves it for me:
-----
$ git diff
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 96ad944ca6a8..0627286380fa 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -346,7 +346,7 @@ endif

 INC_FLAGS += -I$(src-perf)/util
 INC_FLAGS += -I$(src-perf)
-INC_FLAGS += -I$(srctree)/tools/lib/
+INC_FLAGS += -idirafter$(srctree)/tools/lib/

 CORE_CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
 -----

I'll send out a patch.

Sorry for the trouble. Thanks,
Ian