Skip to contentSkip to conversation

Support perf -vv

PATCH Superseded Jin Yao 4 participants 20 messages Mar 26, 2018 – Mar 27, 2018 perf events

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Jin Yao

author
#0 lore raw

cover Support perf -vv

We keep having bug reports that when users build perf on their own, but they don't install some needed libraries such as libelf, libbfd/libibery.

The perf can build, but it is missing important functionality. And users may complain that perf has issue or bug.

This patch-set support 'perf -vv' which will print the compiled-in status of libraries. Once users think perf missing some functionality, it should be very easy for them to check the libraries status.

For example:

$ ./perf -vv
perf version 4.13.rc5.g9b7a81b
                 dwarf: [  on ]
    dwarf_getlocations: [  on ]
                 glibc: [  on ]
                  gtk2: [  on ]
              libaudit: [ off ]
                libbfd: [  on ]
                libelf: [  on ]
               libnuma: [  on ]
numa_num_possible_cpus: [  on ]
               libperl: [  on ]
             libpython: [  on ]
              libslang: [  on ]
             libcrypto: [  on ]
             libunwind: [  on ]
    libdw-dwarf-unwind: [  on ]
                  zlib: [  on ]
                  lzma: [  on ]
             get_cpuid: [  on ]
                   bpf: [  on ]

Jin Yao (3):
perf config: Add -DNO_GLIBC to CFLAGS
perf version: Print the status of compiled-in libraries
perf: Support perf -vv

 tools/perf/Makefile.config   |   2 +
 tools/perf/builtin-version.c | 125 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/builtin.h         |   1 +
 tools/perf/perf.c            |   6 +++
 4 files changed, 134 insertions(+)
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH 1/3 perf config: Add -DNO_GLIBC to CFLAGS

For most of libraries, in perf.config, they can be recorded with -DHAVE_XXX or -DNO_XXX in CFLAGS according to if they are compiled-in. Then C code could know if the library is compiled-in or not.

While for glibc, no existing -DHAVE_XXX or -DNO_XXX.

This patch adds -DNO_GLIBC to CFLAGS.

Signed-off-by Jin Yao <[email protected]>
---
 tools/perf/Makefile.config | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/Makefile.config +2 -0
index 98ff736..5883dd6 100644
@@ -324,6 +324,8 @@ else
       NO_LIBBPF := 1
       NO_JVMTI := 1
     else
+      CFLAGS += -DNO_GLIBC
+
       ifneq ($(filter s% -static%,$(LDFLAGS),),)
         msg := $(error No static glibc found, please install glibc-static);
       else
Signature · 1 line
2.7.4
author
#2 lore raw

PATCH 2/3 perf version: Print the status of compiled-in libraries

In reply to Jin Yao

This patch checks the values passed by CFLAGS (-DXXX) and then print the status of libraries.

For example, if HAVE_DWARF_SUPPORT is defined, that means the library "dwarf" is compiled-in. The patch will print the status "on" for this library.

Signed-off-by Jin Yao <[email protected]>
---
 tools/perf/builtin-version.c | 125 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/builtin.h         |   1 +
 2 files changed, 126 insertions(+)
tools/perf/builtin-version.c +125 -0
index 37019c5..90a0a7f 100644
@@ -9,3 +9,128 @@ int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
 	printf("perf version %s\n", perf_version_string);
 	return 0;
 }
+
+static void status_print(const char *name, const char *status)
+{
+	printf("%22s: [ %3s ]\n", name, status);
+}
+
+static void library_status(void)
+{
+#ifdef HAVE_DWARF_SUPPORT
+	status_print("dwarf", "on");
+#else
+	status_print("dwarf", "off");
+#endif
+
+#ifdef HAVE_DWARF_GETLOCATIONS
+	status_print("dwarf_getlocations", "on");
+#else
+	status_print("dwarf_getlocations", "off");
+#endif
+
+#ifdef NO_GLIBC
+	status_print("glibc", "off");
+#else
+	status_print("glibc", "on");
+#endif
+
+#ifdef HAVE_GTK2_SUPPORT
+	status_print("gtk2", "on");
+#else
+	status_print("gtk2", "off");
+#endif
+
+#ifdef HAVE_LIBAUDIT_SUPPORT
+	status_print("libaudit", "on");
+#else
+	status_print("libaudit", "off");
+#endif
+
+#ifdef HAVE_LIBBFD_SUPPORT
+	status_print("libbfd", "on");
+#else
+	status_print("libbfd", "off");
+#endif
+
+#ifdef HAVE_LIBELF_SUPPORT
+	status_print("libelf", "on");
+#else
+	status_print("libelf", "off");
+#endif
+
+#ifdef HAVE_LIBNUMA_SUPPORT
+	status_print("libnuma", "on");
+	status_print("numa_num_possible_cpus", "on");
+#else
+	status_print("libnuma", "off");
+	status_print("numa_num_possible_cpus", "off");
+#endif
+
+#ifdef NO_LIBPERL
+	status_print("libperl", "off");
+#else
+	status_print("libperl", "on");
+#endif
+
+#ifdef NO_LIBPYTHON
+	status_print("libpython", "off");
+#else
+	status_print("libpython", "on");
+#endif
+
+#ifdef HAVE_SLANG_SUPPORT
+	status_print("libslang", "on");
+#else
+	status_print("libslang", "off");
+#endif
+
+#ifdef HAVE_LIBCRYPTO_SUPPORT
+	status_print("libcrypto", "on");
+#else
+	status_print("libcrypto", "off");
+#endif
+
+#ifdef HAVE_LIBUNWIND_SUPPORT
+	status_print("libunwind", "on");
+#else
+	status_print("libunwind", "off");
+#endif
+
+#ifdef HAVE_DWARF_SUPPORT
+	status_print("libdw-dwarf-unwind", "on");
+#else
+	status_print("libdw-dwarf-unwind", "off");
+#endif
+
+#ifdef HAVE_ZLIB_SUPPORT
+	status_print("zlib", "on");
+#else
+	status_print("zlib", "off");
+#endif
+
+#ifdef HAVE_LZMA_SUPPORT
+	status_print("lzma", "on");
+#else
+	status_print("lzma", "off");
+#endif
+
+#ifdef HAVE_AUXTRACE_SUPPORT
+	status_print("get_cpuid", "on");
+#else
+	status_print("get_cpuid", "off");
+#endif
+
+#ifdef HAVE_LIBBPF_SUPPORT
+	status_print("bpf", "on");
+#else
+	status_print("bpf", "off");
+#endif
+}
+
+int cmd_version2(int argc, const char **argv)
+{
+	cmd_version(argc, argv);
+	library_status();
+	return 0;
+}
tools/perf/builtin.h +1 -0
index 05745f3..c7508ee 100644
@@ -29,6 +29,7 @@ int cmd_timechart(int argc, const char **argv);
 int cmd_top(int argc, const char **argv);
 int cmd_script(int argc, const char **argv);
 int cmd_version(int argc, const char **argv);
+int cmd_version2(int argc, const char **argv);
 int cmd_probe(int argc, const char **argv);
 int cmd_kmem(int argc, const char **argv);
 int cmd_lock(int argc, const char **argv);
Signature · 1 line
2.7.4
#3 lore raw

Re: [PATCH v1 2/3] perf version: Print the status of compiled-in libraries

Quoted 24 lines · Jin Yao

This patch checks the values passed by CFLAGS (-DXXX) and then print the status of libraries.

For example, if HAVE_DWARF_SUPPORT is defined, that means the library "dwarf" is compiled-in. The patch will print the status "on" for this library.

Signed-off-by Jin Yao <[email protected]>
---
 tools/perf/builtin-version.c | 125 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/builtin.h         |   1 +
 2 files changed, 126 insertions(+)
tools/perf/builtin-version.c +3 -0
index 37019c5..90a0a7f 100644
@@ -9,3 +9,128 @@ int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
 	printf("perf version %s\n", perf_version_string);
 	return 0;
 }
+
+static void status_print(const char *name, const char *status)
+{

tools/perf/builtin-version.c int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused) · line 24

Quoted 10 lines
tools/perf/builtin-version.c +10 -0
+	printf("%22s: [ %3s ]\n", name, status);
+}
+
+static void library_status(void)
+{
+#ifdef HAVE_DWARF_SUPPORT
+	status_print("dwarf", "on");
+#else
+	status_print("dwarf", "off");
+#endif

could this and all those below be in some generic macro?

#define STATUS(__d, __m) \

#ifdef __d 				\
	status_print(#__m, "on");	\
#else					\
	status_print(#__m, "OFF");	\

#endif

STATUS(HAVE_DWARF_SUPPORT, dwarf)

also please print the 'OFF' and use colors as in the build message

Quoted 19 lines
tools/perf/builtin-version.c +19 -0
+
+#ifdef HAVE_DWARF_GETLOCATIONS
+	status_print("dwarf_getlocations", "on");
+#else
+	status_print("dwarf_getlocations", "off");
+#endif
+
+#ifdef NO_GLIBC
+	status_print("glibc", "off");
+#else
+	status_print("glibc", "on");
+#endif
+
+#ifdef HAVE_GTK2_SUPPORT
+	status_print("gtk2", "on");
+#else
+	status_print("gtk2", "off");
+#endif
+

SNIP

Quoted 13 lines
tools/perf/builtin-version.c +8 -0
+}
+
+int cmd_version2(int argc, const char **argv)
+{
+	cmd_version(argc, argv);
+	library_status();
+	return 0;
+}
tools/perf/builtin.h +0 -0
index 05745f3..c7508ee 100644

tools/perf/builtin.h int cmd_timechart(int argc, const char **argv); · line 32

Quoted 4 lines
tools/perf/builtin.h +1 -0
@@ -29,6 +29,7 @@ int cmd_timechart(int argc, const char **argv);
 int cmd_top(int argc, const char **argv);
 int cmd_script(int argc, const char **argv);
 int cmd_version(int argc, const char **argv);
+int cmd_version2(int argc, const char **argv);

please don't add new command for this, handle this in cmd_version

thanks,
jirka

author
#4 lore raw

Re: [PATCH v1 2/3] perf version: Print the status of compiled-in libraries

Quoted 43 lines · Jiri Olsa
Quoted 24 lines · Jin Yao

This patch checks the values passed by CFLAGS (-DXXX) and then print the status of libraries.

For example, if HAVE_DWARF_SUPPORT is defined, that means the library "dwarf" is compiled-in. The patch will print the status "on" for this library.

Signed-off-by Jin Yao <[email protected]>
---
  tools/perf/builtin-version.c | 125 +++++++++++++++++++++++++++++++++++++++++++
  tools/perf/builtin.h         |   1 +
  2 files changed, 126 insertions(+)
tools/perf/builtin-version.c +3 -0
index 37019c5..90a0a7f 100644
@@ -9,3 +9,128 @@ int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
  	printf("perf version %s\n", perf_version_string);
  	return 0;
  }
+
+static void status_print(const char *name, const char *status)
+{

tools/perf/builtin-version.c int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused) · line 24

Quoted 10 lines
tools/perf/builtin-version.c +10 -0
+	printf("%22s: [ %3s ]\n", name, status);
+}
+
+static void library_status(void)
+{
+#ifdef HAVE_DWARF_SUPPORT
+	status_print("dwarf", "on");
+#else
+	status_print("dwarf", "off");
+#endif

could this and all those below be in some generic macro?

#define STATUS(__d, __m) \

#ifdef __d 				\
	status_print(#__m, "on");	\
#else					\
	status_print(#__m, "OFF");	\
Quoted 7 lines

#endif

STATUS(HAVE_DWARF_SUPPORT, dwarf)

also please print the 'OFF' and use colors as in the build message

Fine, thanks. I will update the code.

Quoted 33 lines
Quoted 19 lines
tools/perf/builtin-version.c +19 -0
+
+#ifdef HAVE_DWARF_GETLOCATIONS
+	status_print("dwarf_getlocations", "on");
+#else
+	status_print("dwarf_getlocations", "off");
+#endif
+
+#ifdef NO_GLIBC
+	status_print("glibc", "off");
+#else
+	status_print("glibc", "on");
+#endif
+
+#ifdef HAVE_GTK2_SUPPORT
+	status_print("gtk2", "on");
+#else
+	status_print("gtk2", "off");
+#endif
+

SNIP

Quoted 11 lines
tools/perf/builtin-version.c +8 -0
+}
+
+int cmd_version2(int argc, const char **argv)
+{
+	cmd_version(argc, argv);
+	library_status();
+	return 0;
+}
tools/perf/builtin.h +0 -1
index 05745f3..c7508ee 100644
--- a/tools/perf/builtin.h
Quoted 9 lines

tools/perf/builtin.h int cmd_timechart(int argc, const char **argv); · line 32

Quoted 6 lines

+++ b/tools/perf/builtin.h

tools/perf/builtin.h +1 -0
@@ -29,6 +29,7 @@ int cmd_timechart(int argc, const char **argv);
  int cmd_top(int argc, const char **argv);
  int cmd_script(int argc, const char **argv);
  int cmd_version(int argc, const char **argv);
+int cmd_version2(int argc, const char **argv);

please don't add new command for this, handle this in cmd_version

Yeah, I know that, adding a new command is not a good idea.

But how can I pass a new parameter to cmd_version to indicate it's the -vv case? Use argv[1]? But looks not good since argc is 1.

Any idea about that?

Thanks
Jin Yao

Quoted 3 lines

thanks,
jirka

author
#5 lore raw

Re: [PATCH v1 2/3] perf version: Print the status of compiled-in libraries

Quoted 103 lines · Yao
Quoted 47 lines · Jiri Olsa
Quoted 26 lines · Jin Yao

This patch checks the values passed by CFLAGS (-DXXX) and then print the status of libraries.

For example, if HAVE_DWARF_SUPPORT is defined, that means the library "dwarf" is compiled-in. The patch will print the status "on" for this library.

Signed-off-by Jin Yao <[email protected]>

---
  tools/perf/builtin-version.c | 125
+++++++++++++++++++++++++++++++++++++++++++
  tools/perf/builtin.h         |   1 +

  2 files changed, 126 insertions(+)
tools/perf/builtin-version.c +0 -0
index 37019c5..90a0a7f 100644
@@ -9,3 +9,128 @@ int cmd_version(int argc __maybe_unused, const char 
 **argv __maybe_unused)
       printf("perf version %s\n", perf_version_string);
       return 0;

  }

tools/perf/builtin-version.c +3 -0
+
+static void status_print(const char *name, const char *status)
+{

tools/perf/builtin-version.c int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused) · line 24

Quoted 10 lines
tools/perf/builtin-version.c +10 -0
+    printf("%22s: [ %3s ]\n", name, status);
+}
+
+static void library_status(void)
+{
+#ifdef HAVE_DWARF_SUPPORT
+    status_print("dwarf", "on");
+#else
+    status_print("dwarf", "off");
+#endif

could this and all those below be in some generic macro?

#define STATUS(__d, __m)        \
#ifdef __d                 \
    status_print(#__m, "on");    \
#else                    \
    status_print(#__m, "OFF");    \
#endif

Quoted 5 lines

STATUS(HAVE_DWARF_SUPPORT, dwarf)

also please print the 'OFF' and use colors as in the build message

Fine, thanks. I will update the code.

Quoted 33 lines
Quoted 19 lines
tools/perf/builtin-version.c +19 -0
+
+#ifdef HAVE_DWARF_GETLOCATIONS
+    status_print("dwarf_getlocations", "on");
+#else
+    status_print("dwarf_getlocations", "off");
+#endif
+
+#ifdef NO_GLIBC
+    status_print("glibc", "off");
+#else
+    status_print("glibc", "on");
+#endif
+
+#ifdef HAVE_GTK2_SUPPORT
+    status_print("gtk2", "on");
+#else
+    status_print("gtk2", "off");
+#endif
+

SNIP

Quoted 11 lines
tools/perf/builtin-version.c +8 -0
+}
+
+int cmd_version2(int argc, const char **argv)
+{
+    cmd_version(argc, argv);
+    library_status();
+    return 0;
+}
tools/perf/builtin.h +0 -1
index 05745f3..c7508ee 100644
--- a/tools/perf/builtin.h
Quoted 9 lines

tools/perf/builtin.h int cmd_timechart(int argc, const char **argv); · line 32

Quoted 6 lines

+++ b/tools/perf/builtin.h

tools/perf/builtin.h +1 -0
@@ -29,6 +29,7 @@ int cmd_timechart(int argc, const char **argv);
   int cmd_top(int argc, const char **argv);
   int cmd_script(int argc, const char **argv);
   int cmd_version(int argc, const char **argv);
+int cmd_version2(int argc, const char **argv);

please don't add new command for this, handle this in cmd_version

Yeah, I know that, adding a new command is not a good idea.

Quoted 8 lines

But how can I pass a new parameter to cmd_version to indicate it's the -vv case? Use argv[1]? But looks not good since argc is 1.

Any idea about that?

Thanks
Jin Yao

Hi Jiri,

Maybe could we just add the information to 'perf -v'? I mean we don't need the 'perf -vv', directly add the information to 'perf -v'. Is it OK?

Thanks
Jin Yao

Quoted 3 lines
Quoted 3 lines

thanks,
jirka

#6 lore raw

Re: [PATCH v1 2/3] perf version: Print the status of compiled-in libraries

In reply to Jin, Yao
Quoted 99 lines · Yao
Quoted 43 lines · Jiri Olsa
Quoted 24 lines · Jin Yao

This patch checks the values passed by CFLAGS (-DXXX) and then print the status of libraries.

For example, if HAVE_DWARF_SUPPORT is defined, that means the library "dwarf" is compiled-in. The patch will print the status "on" for this library.

Signed-off-by Jin Yao <[email protected]>
---
  tools/perf/builtin-version.c | 125 +++++++++++++++++++++++++++++++++++++++++++
  tools/perf/builtin.h         |   1 +
  2 files changed, 126 insertions(+)
tools/perf/builtin-version.c +3 -0
index 37019c5..90a0a7f 100644
@@ -9,3 +9,128 @@ int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
  	printf("perf version %s\n", perf_version_string);
  	return 0;
  }
+
+static void status_print(const char *name, const char *status)
+{

tools/perf/builtin-version.c int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused) · line 24

Quoted 10 lines
tools/perf/builtin-version.c +10 -0
+	printf("%22s: [ %3s ]\n", name, status);
+}
+
+static void library_status(void)
+{
+#ifdef HAVE_DWARF_SUPPORT
+	status_print("dwarf", "on");
+#else
+	status_print("dwarf", "off");
+#endif

could this and all those below be in some generic macro?

#define STATUS(__d, __m) \

#ifdef __d 				\
	status_print(#__m, "on");	\
#else					\
	status_print(#__m, "OFF");	\
Quoted 7 lines

#endif

STATUS(HAVE_DWARF_SUPPORT, dwarf)

also please print the 'OFF' and use colors as in the build message

Fine, thanks. I will update the code.

Quoted 33 lines
Quoted 19 lines
tools/perf/builtin-version.c +19 -0
+
+#ifdef HAVE_DWARF_GETLOCATIONS
+	status_print("dwarf_getlocations", "on");
+#else
+	status_print("dwarf_getlocations", "off");
+#endif
+
+#ifdef NO_GLIBC
+	status_print("glibc", "off");
+#else
+	status_print("glibc", "on");
+#endif
+
+#ifdef HAVE_GTK2_SUPPORT
+	status_print("gtk2", "on");
+#else
+	status_print("gtk2", "off");
+#endif
+

SNIP

Quoted 11 lines
tools/perf/builtin-version.c +8 -0
+}
+
+int cmd_version2(int argc, const char **argv)
+{
+	cmd_version(argc, argv);
+	library_status();
+	return 0;
+}
tools/perf/builtin.h +0 -1
index 05745f3..c7508ee 100644
--- a/tools/perf/builtin.h
Quoted 9 lines

tools/perf/builtin.h int cmd_timechart(int argc, const char **argv); · line 32

Quoted 6 lines

+++ b/tools/perf/builtin.h

tools/perf/builtin.h +1 -0
@@ -29,6 +29,7 @@ int cmd_timechart(int argc, const char **argv);
  int cmd_top(int argc, const char **argv);
  int cmd_script(int argc, const char **argv);
  int cmd_version(int argc, const char **argv);
+int cmd_version2(int argc, const char **argv);

please don't add new command for this, handle this in cmd_version

Quoted 6 lines

Yeah, I know that, adding a new command is not a good idea.

But how can I pass a new parameter to cmd_version to indicate it's the -vv case? Use argv[1]? But looks not good since argc is 1.

Any idea about that?

I guess the based would be to have 'perf version -v', where in cmd_version you'd call standard option processing..

the 'perf -vv' shortcut could for example be done through some global var like in attached patch... untested

jirka

---

tools/perf/builtin-version.c +2 -0
index 37019c5d675f..1fe458792cc1 100644
@@ -4,6 +4,8 @@
 #include <linux/compiler.h>
 #include <stdio.h>
 
+int version_verbose;
+
 int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
 {
 	printf("perf version %s\n", perf_version_string);
tools/perf/perf.c +8 -1
index 1b3fc8ec0fa2..a5c95c6c0de7 100644
@@ -185,7 +185,14 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			break;
 		}
 
-		if (!strcmp(cmd, "-v")) {
+		if (strstarts(cmd, "-v")) {
+			int i;
+
+			for (i = 2; cmd[i]; i++) {
+				if (cmd[i] == 'v')
+					version_verbose++;
+			}
+
 			(*argv)[0] = "--version";
 			break;
 		}
tools/perf/perf.h +2 -0
index 8fec1abd0f1f..9c45ba179635 100644
@@ -13,6 +13,8 @@ void test_attr__init(void);
 void test_attr__open(struct perf_event_attr *attr, pid_t pid, int cpu,
 		     int fd, int group_fd, unsigned long flags);
 
+extern int version_verbose;
+
 #define HAVE_ATTR_TEST
 #include "perf-sys.h"
author
#7 lore raw

Re: [PATCH v1 2/3] perf version: Print the status of compiled-in libraries

Quoted 156 lines · Jiri Olsa
Quoted 99 lines · Yao
Quoted 43 lines · Jiri Olsa
Quoted 24 lines · Jin Yao

This patch checks the values passed by CFLAGS (-DXXX) and then print the status of libraries.

For example, if HAVE_DWARF_SUPPORT is defined, that means the library "dwarf" is compiled-in. The patch will print the status "on" for this library.

Signed-off-by Jin Yao <[email protected]>
---
   tools/perf/builtin-version.c | 125 +++++++++++++++++++++++++++++++++++++++++++
   tools/perf/builtin.h         |   1 +
   2 files changed, 126 insertions(+)
tools/perf/builtin-version.c +3 -0
index 37019c5..90a0a7f 100644
@@ -9,3 +9,128 @@ int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
   	printf("perf version %s\n", perf_version_string);
   	return 0;
   }
+
+static void status_print(const char *name, const char *status)
+{

tools/perf/builtin-version.c int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused) · line 24

Quoted 10 lines
tools/perf/builtin-version.c +10 -0
+	printf("%22s: [ %3s ]\n", name, status);
+}
+
+static void library_status(void)
+{
+#ifdef HAVE_DWARF_SUPPORT
+	status_print("dwarf", "on");
+#else
+	status_print("dwarf", "off");
+#endif

could this and all those below be in some generic macro?

#define STATUS(__d, __m) \

#ifdef __d 				\
	status_print(#__m, "on");	\
#else					\
	status_print(#__m, "OFF");	\
Quoted 7 lines

#endif

STATUS(HAVE_DWARF_SUPPORT, dwarf)

also please print the 'OFF' and use colors as in the build message

Fine, thanks. I will update the code.

Quoted 33 lines
Quoted 19 lines
tools/perf/builtin-version.c +19 -0
+
+#ifdef HAVE_DWARF_GETLOCATIONS
+	status_print("dwarf_getlocations", "on");
+#else
+	status_print("dwarf_getlocations", "off");
+#endif
+
+#ifdef NO_GLIBC
+	status_print("glibc", "off");
+#else
+	status_print("glibc", "on");
+#endif
+
+#ifdef HAVE_GTK2_SUPPORT
+	status_print("gtk2", "on");
+#else
+	status_print("gtk2", "off");
+#endif
+

SNIP

Quoted 11 lines
tools/perf/builtin-version.c +8 -0
+}
+
+int cmd_version2(int argc, const char **argv)
+{
+	cmd_version(argc, argv);
+	library_status();
+	return 0;
+}
tools/perf/builtin.h +0 -1
index 05745f3..c7508ee 100644
--- a/tools/perf/builtin.h
Quoted 9 lines

tools/perf/builtin.h int cmd_timechart(int argc, const char **argv); · line 32

Quoted 6 lines

+++ b/tools/perf/builtin.h

tools/perf/builtin.h +1 -0
@@ -29,6 +29,7 @@ int cmd_timechart(int argc, const char **argv);
   int cmd_top(int argc, const char **argv);
   int cmd_script(int argc, const char **argv);
   int cmd_version(int argc, const char **argv);
+int cmd_version2(int argc, const char **argv);

please don't add new command for this, handle this in cmd_version

Quoted 6 lines

Yeah, I know that, adding a new command is not a good idea.

But how can I pass a new parameter to cmd_version to indicate it's the -vv case? Use argv[1]? But looks not good since argc is 1.

Any idea about that?

I guess the based would be to have 'perf version -v', where in cmd_version you'd call standard option processing..

the 'perf -vv' shortcut could for example be done through some global var like in attached patch... untested

jirka

---

tools/perf/builtin-version.c +2 -0
index 37019c5d675f..1fe458792cc1 100644
@@ -4,6 +4,8 @@
  #include <linux/compiler.h>
  #include <stdio.h>
 
+int version_verbose;
+
  int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
  {
  	printf("perf version %s\n", perf_version_string);
tools/perf/perf.c +8 -1
index 1b3fc8ec0fa2..a5c95c6c0de7 100644
@@ -185,7 +185,14 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
  			break;
  		}
 
-		if (!strcmp(cmd, "-v")) {
+		if (strstarts(cmd, "-v")) {
+			int i;
+
+			for (i = 2; cmd[i]; i++) {
+				if (cmd[i] == 'v')
+					version_verbose++;
+			}
+
  			(*argv)[0] = "--version";
  			break;
  		}
tools/perf/perf.h +0 -0
index 8fec1abd0f1f..9c45ba179635 100644

tools/perf/perf.h void test_attr__init(void); · line 20

Quoted 8 lines
tools/perf/perf.h +2 -0
@@ -13,6 +13,8 @@ void test_attr__init(void);
  void test_attr__open(struct perf_event_attr *attr, pid_t pid, int cpu,
  		     int fd, int group_fd, unsigned long flags);
 
+extern int version_verbose;
+
  #define HAVE_ATTR_TEST
  #include "perf-sys.h"

Using global variable, hmm, that's fine, looks no better way. :)

I will follow Ingo's suggestion. Use 'perf -v --build-options' and map it to 'perf -vv'.

Thanks
Jin Yao

author
#8 lore raw

Re: [PATCH v1 2/3] perf version: Print the status of compiled-in libraries

In reply to Jiri Olsa
Quoted 38 lines · Jiri Olsa
Quoted 24 lines · Jin Yao

This patch checks the values passed by CFLAGS (-DXXX) and then print the status of libraries.

For example, if HAVE_DWARF_SUPPORT is defined, that means the library "dwarf" is compiled-in. The patch will print the status "on" for this library.

Signed-off-by Jin Yao <[email protected]>
---
  tools/perf/builtin-version.c | 125 +++++++++++++++++++++++++++++++++++++++++++
  tools/perf/builtin.h         |   1 +
  2 files changed, 126 insertions(+)
tools/perf/builtin-version.c +3 -0
index 37019c5..90a0a7f 100644
@@ -9,3 +9,128 @@ int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
  	printf("perf version %s\n", perf_version_string);
  	return 0;
  }
+
+static void status_print(const char *name, const char *status)
+{

tools/perf/builtin-version.c int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused) · line 24

Quoted 10 lines
tools/perf/builtin-version.c +10 -0
+	printf("%22s: [ %3s ]\n", name, status);
+}
+
+static void library_status(void)
+{
+#ifdef HAVE_DWARF_SUPPORT
+	status_print("dwarf", "on");
+#else
+	status_print("dwarf", "off");
+#endif

could this and all those below be in some generic macro?

Quoted 10 lines

#define STATUS(__d, __m) \

#ifdef __d 				\
	status_print(#__m, "on");	\
#else					\
	status_print(#__m, "OFF");	\

#endif

STATUS(HAVE_DWARF_SUPPORT, dwarf)

Hi Jiri,

I have tried this macro definition, but unfortunately the compilation is failed.

error: '#' is not followed by a macro parameter
#define STATUS(__d, __m) \

I just guess the '#' in #ifdef confuses the gcc. Looks we can't define #ifdef/#endif block in a macro.

Thanks
Jin Yao

Quoted 47 lines

also please print the 'OFF' and use colors as in the build message

Quoted 19 lines
tools/perf/builtin-version.c +19 -0
+
+#ifdef HAVE_DWARF_GETLOCATIONS
+	status_print("dwarf_getlocations", "on");
+#else
+	status_print("dwarf_getlocations", "off");
+#endif
+
+#ifdef NO_GLIBC
+	status_print("glibc", "off");
+#else
+	status_print("glibc", "on");
+#endif
+
+#ifdef HAVE_GTK2_SUPPORT
+	status_print("gtk2", "on");
+#else
+	status_print("gtk2", "off");
+#endif
+

SNIP

Quoted 13 lines
tools/perf/builtin-version.c +8 -0
+}
+
+int cmd_version2(int argc, const char **argv)
+{
+	cmd_version(argc, argv);
+	library_status();
+	return 0;
+}
tools/perf/builtin.h +0 -0
index 05745f3..c7508ee 100644

tools/perf/builtin.h int cmd_timechart(int argc, const char **argv); · line 32

Quoted 4 lines
tools/perf/builtin.h +1 -0
@@ -29,6 +29,7 @@ int cmd_timechart(int argc, const char **argv);
  int cmd_top(int argc, const char **argv);
  int cmd_script(int argc, const char **argv);
  int cmd_version(int argc, const char **argv);
+int cmd_version2(int argc, const char **argv);

please don't add new command for this, handle this in cmd_version

thanks,
jirka

#9 lore raw

Re: [PATCH v1 2/3] perf version: Print the status of compiled-in libraries

Quoted 54 lines · Yao
Quoted 38 lines · Jiri Olsa
Quoted 24 lines · Jin Yao

This patch checks the values passed by CFLAGS (-DXXX) and then print the status of libraries.

For example, if HAVE_DWARF_SUPPORT is defined, that means the library "dwarf" is compiled-in. The patch will print the status "on" for this library.

Signed-off-by Jin Yao <[email protected]>
---
  tools/perf/builtin-version.c | 125 +++++++++++++++++++++++++++++++++++++++++++
  tools/perf/builtin.h         |   1 +
  2 files changed, 126 insertions(+)
tools/perf/builtin-version.c +3 -0
index 37019c5..90a0a7f 100644
@@ -9,3 +9,128 @@ int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
  	printf("perf version %s\n", perf_version_string);
  	return 0;
  }
+
+static void status_print(const char *name, const char *status)
+{

tools/perf/builtin-version.c int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused) · line 24

Quoted 10 lines
tools/perf/builtin-version.c +10 -0
+	printf("%22s: [ %3s ]\n", name, status);
+}
+
+static void library_status(void)
+{
+#ifdef HAVE_DWARF_SUPPORT
+	status_print("dwarf", "on");
+#else
+	status_print("dwarf", "off");
+#endif

could this and all those below be in some generic macro?

Quoted 10 lines

#define STATUS(__d, __m) \

#ifdef __d 				\
	status_print(#__m, "on");	\
#else					\
	status_print(#__m, "OFF");	\

#endif

STATUS(HAVE_DWARF_SUPPORT, dwarf)

Hi Jiri,

Quoted 9 lines

I have tried this macro definition, but unfortunately the compilation is failed.

error: '#' is not followed by a macro parameter
#define STATUS(__d, __m) \

I just guess the '#' in #ifdef confuses the gcc. Looks we can't define #ifdef/#endif block in a macro.

ah crap.. right ;-) how about we take the IS_BUILTIN thingie from include/linux/kconfig.h and use it as in attached test change.. it gives me:

[jolsa@krava perf]$ ./perf version
perf version 4.16.rc6.g1ee9a60
dwarf: [ on ]
krava: [ OFF ]

we could put those macros into tools/include/tools/config.h, Arnaldo? together with the comments from kconfig.h that I cut out..

thanks,
jirka

---

tools/perf/builtin-version.c +25 -0
index 1fe458792cc1..b392f9e80fd0 100644
@@ -6,8 +6,33 @@
 
 int version_verbose;
 
+
+#define __ARG_PLACEHOLDER_1 0,
+#define __take_second_arg(__ignored, val, ...) val
+
+#define __is_defined(x)                 ___is_defined(x)
+#define ___is_defined(val)              ____is_defined(__ARG_PLACEHOLDER_##val) 
+#define ____is_defined(arg1_or_junk)    __take_second_arg(arg1_or_junk 1, 0)
+
+#define IS_BUILTIN(option) __is_defined(option)
+
+
+static void status_print(const char *name, const char *status)
+{
+	printf("%22s: [ %3s ]\n", name, status);
+}
+
 int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
 {
 	printf("perf version %s\n", perf_version_string);
+
+#define STATUS(__d, __m)			\
+	if (IS_BUILTIN(__d))			\
+		status_print(#__m, "on");	\
+	else					\
+		status_print(#__m, "OFF");
+
+	STATUS(HAVE_DWARF_SUPPORT, dwarf)
+	STATUS(HAVE_KRAVA_SUPPORT, krava)
 	return 0;
 }
author
#10 lore raw

Re: [PATCH v1 2/3] perf version: Print the status of compiled-in libraries

Quoted 111 lines · Jiri Olsa
Quoted 54 lines · Yao
Quoted 38 lines · Jiri Olsa
Quoted 24 lines · Jin Yao

This patch checks the values passed by CFLAGS (-DXXX) and then print the status of libraries.

For example, if HAVE_DWARF_SUPPORT is defined, that means the library "dwarf" is compiled-in. The patch will print the status "on" for this library.

Signed-off-by Jin Yao <[email protected]>
---
   tools/perf/builtin-version.c | 125 +++++++++++++++++++++++++++++++++++++++++++
   tools/perf/builtin.h         |   1 +
   2 files changed, 126 insertions(+)
tools/perf/builtin-version.c +3 -0
index 37019c5..90a0a7f 100644
@@ -9,3 +9,128 @@ int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
   	printf("perf version %s\n", perf_version_string);
   	return 0;
   }
+
+static void status_print(const char *name, const char *status)
+{

tools/perf/builtin-version.c int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused) · line 24

Quoted 10 lines
tools/perf/builtin-version.c +10 -0
+	printf("%22s: [ %3s ]\n", name, status);
+}
+
+static void library_status(void)
+{
+#ifdef HAVE_DWARF_SUPPORT
+	status_print("dwarf", "on");
+#else
+	status_print("dwarf", "off");
+#endif

could this and all those below be in some generic macro?

Quoted 10 lines

#define STATUS(__d, __m) \

#ifdef __d 				\
	status_print(#__m, "on");	\
#else					\
	status_print(#__m, "OFF");	\

#endif

STATUS(HAVE_DWARF_SUPPORT, dwarf)

Hi Jiri,

Quoted 9 lines

I have tried this macro definition, but unfortunately the compilation is failed.

error: '#' is not followed by a macro parameter
#define STATUS(__d, __m) \

I just guess the '#' in #ifdef confuses the gcc. Looks we can't define #ifdef/#endif block in a macro.

ah crap.. right ;-) how about we take the IS_BUILTIN thingie from include/linux/kconfig.h and use it as in attached test change.. it gives me:

[jolsa@krava perf]$ ./perf version
perf version 4.16.rc6.g1ee9a60
dwarf: [ on ]
krava: [ OFF ]

we could put those macros into tools/include/tools/config.h, Arnaldo? together with the comments from kconfig.h that I cut out..

thanks,
jirka

---

tools/perf/builtin-version.c +17 -0
index 1fe458792cc1..b392f9e80fd0 100644
@@ -6,8 +6,33 @@
 
  int version_verbose;
 
+
+#define __ARG_PLACEHOLDER_1 0,
+#define __take_second_arg(__ignored, val, ...) val
+
+#define __is_defined(x)                 ___is_defined(x)
+#define ___is_defined(val)              ____is_defined(__ARG_PLACEHOLDER_##val)
+#define ____is_defined(arg1_or_junk)    __take_second_arg(arg1_or_junk 1, 0)
+
+#define IS_BUILTIN(option) __is_defined(option)
+
+
+static void status_print(const char *name, const char *status)
+{
+	printf("%22s: [ %3s ]\n", name, status);
+}
+
  int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
  {
  	printf("perf version %s\n", perf_version_string);
+

tools/perf/builtin-version.c line 38

Quoted 10 lines
tools/perf/builtin-version.c +8 -0
+#define STATUS(__d, __m)			\
+	if (IS_BUILTIN(__d))			\
+		status_print(#__m, "on");	\
+	else					\
+		status_print(#__m, "OFF");
+
+	STATUS(HAVE_DWARF_SUPPORT, dwarf)
+	STATUS(HAVE_KRAVA_SUPPORT, krava)
  	return 0;
  }

Hi Jiri,

It looks we need to copy the IS_BUILTIN and associated macro definitions to a common header file.

I guess you will post this patch if Arnaldo agree with this idea, right?

I plan to post v2 of this patch series after this 'IS_BUILTIN' patch gets merged.

Thanks
Jin Yao

#11 lore raw

Re: [PATCH v1 2/3] perf version: Print the status of compiled-in libraries

On Tue, Mar 27, 2018 at 09:17:57PM +0800, Jin, Yao wrote:

SNIP

Quoted 26 lines
Quoted 13 lines
(quoted diff) +9 -0
+#define IS_BUILTIN(option) __is_defined(option)
+
+
+static void status_print(const char *name, const char *status)
+{
+	printf("%22s: [ %3s ]\n", name, status);
+}
+
  int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused)
  {
  	printf("perf version %s\n", perf_version_string);
+

Review

Quoted 10 lines
(quoted diff) +8 -0
+#define STATUS(__d, __m)			\
+	if (IS_BUILTIN(__d))			\
+		status_print(#__m, "on");	\
+	else					\
+		status_print(#__m, "OFF");
+
+	STATUS(HAVE_DWARF_SUPPORT, dwarf)
+	STATUS(HAVE_KRAVA_SUPPORT, krava)
  	return 0;
  }

Hi Jiri,

Quoted 7 lines

It looks we need to copy the IS_BUILTIN and associated macro definitions to a common header file.

I guess you will post this patch if Arnaldo agree with this idea, right?

I plan to post v2 of this patch series after this 'IS_BUILTIN' patch gets merged.

please post patch below with your patchset
and feel free to change it if needed

thanks,
jirka

---

Quoted 1 line

From bdd36e208bde1576bba54679734174c9a2064822 Mon Sep 17 00:00:00 2001

From: Jiri Olsa <[email protected]>
Date: Tue, 27 Mar 2018 15:30:26 +0200
Subject: [PATCH] tools include: Add config.h header file

Adding IS_BUILTIN macro and its dependencies into
tools world.

It's taken from kernel's include/linux/kconfig.h, which can't be taken completely due to its kconfig dependencies.

Signed-off-by Jiri Olsa <[email protected]>
---
 tools/include/tools/config.h | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 tools/include/tools/config.h
tools/include/tools/config.h new file +34 -0
new file mode 100644
index 000000000000..08ade7df8132
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _TOOLS_CONFIG_H
+#define _TOOLS_CONFIG_H
+
+/* Subset of include/linux/kconfig.h */
+
+#define __ARG_PLACEHOLDER_1 0,
+#define __take_second_arg(__ignored, val, ...) val
+
+/*
+ * Helper macros to use CONFIG_ options in C/CPP expressions. Note that
+ * these only work with boolean and tristate options.
+ */
+
+/*
+ * Getting something that works in C and CPP for an arg that may or may
+ * not be defined is tricky.  Here, if we have "#define CONFIG_BOOGER 1"
+ * we match on the placeholder define, insert the "0," for arg1 and generate
+ * the triplet (0, 1, 0).  Then the last step cherry picks the 2nd arg (a one).
+ * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when
+ * the last step cherry picks the 2nd arg, we get a zero.
+ */
+#define __is_defined(x)			___is_defined(x)
+#define ___is_defined(val)		____is_defined(__ARG_PLACEHOLDER_##val)
+#define ____is_defined(arg1_or_junk)	__take_second_arg(arg1_or_junk 1, 0)
+
+/*
+ * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
+ * otherwise. For boolean options, this is equivalent to
+ * IS_ENABLED(CONFIG_FOO).
+ */
+#define IS_BUILTIN(option) __is_defined(option)
+
+#endif /* _TOOLS_CONFIG_H */
Signature · 1 line
2.13.6
#12 lore raw

Re: [PATCH v1 2/3] perf version: Print the status of compiled-in libraries

In reply to Jin Yao
Quoted 8 lines · Jin Yao
tools/perf/builtin-version.c +8 -0
+#ifdef HAVE_DWARF_SUPPORT
+#ifdef HAVE_DWARF_GETLOCATIONS
+#ifdef NO_GLIBC
+#ifdef HAVE_GTK2_SUPPORT
+#ifdef HAVE_LIBAUDIT_SUPPORT
+#ifdef HAVE_LIBBFD_SUPPORT
+#ifdef HAVE_LIBELF_SUPPORT
+#ifdef HAVE_LIBNUMA_SUPPORT

tools/perf/builtin-version.c int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused) · line 124

Quoted 10 lines
tools/perf/builtin-version.c +10 -0
+#ifdef NO_LIBPERL
+#ifdef NO_LIBPYTHON
+#ifdef HAVE_SLANG_SUPPORT
+#ifdef HAVE_LIBCRYPTO_SUPPORT
+#ifdef HAVE_LIBUNWIND_SUPPORT
+#ifdef HAVE_DWARF_SUPPORT
+#ifdef HAVE_ZLIB_SUPPORT
+#ifdef HAVE_LZMA_SUPPORT
+#ifdef HAVE_AUXTRACE_SUPPORT
+#ifdef HAVE_LIBBPF_SUPPORT

BTW., it would be nice at this point to fix those 3 outliers that have a negation in their library support status macro:

tools/perf/builtin-version.c int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused) · line 76

Quoted 3 lines
tools/perf/builtin-version.c +3 -0
+#ifdef NO_GLIBC
+#ifdef NO_LIBPERL
+#ifdef NO_LIBPYTHON

... and invert them back to the HAVE_* side of the logic:

Review

Quoted 3 lines
(quoted diff) +3 -0
+#ifdef HAVE_GLIBC
+#ifdef HAVE_LIBPERL
+#ifdef HAVE_LIBPYTHON

That should make all related code more consistent and more readable.

Thanks,

Ingo

author
#13 lore raw

Re: [PATCH v1 2/3] perf version: Print the status of compiled-in libraries

Quoted 37 lines · Ingo Molnar
Quoted 8 lines · Jin Yao
tools/perf/builtin-version.c +8 -0
+#ifdef HAVE_DWARF_SUPPORT
+#ifdef HAVE_DWARF_GETLOCATIONS
+#ifdef NO_GLIBC
+#ifdef HAVE_GTK2_SUPPORT
+#ifdef HAVE_LIBAUDIT_SUPPORT
+#ifdef HAVE_LIBBFD_SUPPORT
+#ifdef HAVE_LIBELF_SUPPORT
+#ifdef HAVE_LIBNUMA_SUPPORT

tools/perf/builtin-version.c int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused) · line 124

Quoted 10 lines
tools/perf/builtin-version.c +10 -0
+#ifdef NO_LIBPERL
+#ifdef NO_LIBPYTHON
+#ifdef HAVE_SLANG_SUPPORT
+#ifdef HAVE_LIBCRYPTO_SUPPORT
+#ifdef HAVE_LIBUNWIND_SUPPORT
+#ifdef HAVE_DWARF_SUPPORT
+#ifdef HAVE_ZLIB_SUPPORT
+#ifdef HAVE_LZMA_SUPPORT
+#ifdef HAVE_AUXTRACE_SUPPORT
+#ifdef HAVE_LIBBPF_SUPPORT

BTW., it would be nice at this point to fix those 3 outliers that have a negation in their library support status macro:

tools/perf/builtin-version.c int cmd_version(int argc __maybe_unused, const char **argv __maybe_unused) · line 76

Quoted 3 lines
tools/perf/builtin-version.c +3 -0
+#ifdef NO_GLIBC
+#ifdef NO_LIBPERL
+#ifdef NO_LIBPYTHON

... and invert them back to the HAVE_* side of the logic:

Review

Quoted 3 lines
(quoted diff) +3 -0
+#ifdef HAVE_GLIBC
+#ifdef HAVE_LIBPERL
+#ifdef HAVE_LIBPYTHON

That should make all related code more consistent and more readable.

Quoted 4 lines

Thanks,

Ingo

Yes, it's better, thanks!
I will try to convert them to the HAVE_*.

Thanks
Jin Yao

author
#14 lore raw

PATCH 3/3 perf: Support perf -vv

In reply to Jin Yao

We keep having bug reports that when users build perf on their own, but they don't install some needed libraries such as libelf, libbfd/libibery.

The perf can build, but it is missing important functionality.

This patch provides a new option '-vv' which will print the compiled-in status of libraries.

For example:

$ ./perf -vv
perf version 4.13.rc5.g9b7a81b
                 dwarf: [  on ]
    dwarf_getlocations: [  on ]
                 glibc: [  on ]
                  gtk2: [  on ]
              libaudit: [ off ]
                libbfd: [  on ]
                libelf: [  on ]
               libnuma: [  on ]
numa_num_possible_cpus: [  on ]
               libperl: [  on ]
             libpython: [  on ]
              libslang: [  on ]
             libcrypto: [  on ]
             libunwind: [  on ]
    libdw-dwarf-unwind: [  on ]
                  zlib: [  on ]
                  lzma: [  on ]
             get_cpuid: [  on ]
                   bpf: [  on ]
Signed-off-by Jin Yao <[email protected]>
---
 tools/perf/perf.c | 6 ++++++
 1 file changed, 6 insertions(+)
tools/perf/perf.c +6 -0
index 1b3fc8e..300c83d 100644
@@ -64,6 +64,7 @@ static struct cmd_struct commands[] = {
 	{ "top",	cmd_top,	0 },
 	{ "annotate",	cmd_annotate,	0 },
 	{ "version",	cmd_version,	0 },
+	{ "version2",	cmd_version2,	0 },
 	{ "script",	cmd_script,	0 },
 	{ "sched",	cmd_sched,	0 },
 #ifdef HAVE_LIBELF_SUPPORT

@@ -190,6 +191,11 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			break;
 		}
 
+		if (!strcmp(cmd, "-vv")) {
+			(*argv)[0] = "--version2";
+			break;
+		}
+
 		/*
 		 * Check remaining flags.
 		 */
Signature · 1 line
2.7.4
#15 lore raw

Re: [PATCH v1 3/3] perf: Support perf -vv

Quoted 10 lines · Jin Yao

+++ b/tools/perf/perf.c

tools/perf/perf.c +1 -0
@@ -64,6 +64,7 @@ static struct cmd_struct commands[] = {
 	{ "top",	cmd_top,	0 },
 	{ "annotate",	cmd_annotate,	0 },
 	{ "version",	cmd_version,	0 },
+	{ "version2",	cmd_version2,	0 },
 	{ "script",	cmd_script,	0 },
 	{ "sched",	cmd_sched,	0 },
 #ifdef HAVE_LIBELF_SUPPORT

tools/perf/perf.c static int handle_options(const char ***argv, int *argc, int *envchanged) · line 197

Quoted 7 lines
tools/perf/perf.c +4 -0
@@ -190,6 +191,11 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			break;
 		}
 
+		if (!strcmp(cmd, "-vv")) {
+			(*argv)[0] = "--version2";
+			break;
+		}

That is just lazy hackery, please use the regular option library to add options and document them!

Also, '--version2' is a sloppy name, the right solution is to do what Git does, it has a --build-options sub-option to 'perf version':

triton:~/tip> git version -h
usage: git version [<options>]

--build-options also print build options

  triton:~/tip> git version --build-options
  git version 2.14.1
  sizeof-long: 8

This should be done for perf as well, and _then_ maybe can we add a helper alias that maps '-vv' to 'version --build-options'.

Thanks,

Ingo

author
#16 lore raw

Re: [PATCH v1 3/3] perf: Support perf -vv

Quoted 37 lines · Ingo Molnar
Quoted 10 lines · Jin Yao

+++ b/tools/perf/perf.c

tools/perf/perf.c +1 -0
@@ -64,6 +64,7 @@ static struct cmd_struct commands[] = {
  	{ "top",	cmd_top,	0 },
  	{ "annotate",	cmd_annotate,	0 },
  	{ "version",	cmd_version,	0 },
+	{ "version2",	cmd_version2,	0 },
  	{ "script",	cmd_script,	0 },
  	{ "sched",	cmd_sched,	0 },
  #ifdef HAVE_LIBELF_SUPPORT

tools/perf/perf.c static int handle_options(const char ***argv, int *argc, int *envchanged) · line 197

Quoted 7 lines
tools/perf/perf.c +4 -0
@@ -190,6 +191,11 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
  			break;
  		}
 
+		if (!strcmp(cmd, "-vv")) {
+			(*argv)[0] = "--version2";
+			break;
+		}

That is just lazy hackery, please use the regular option library to add options and document them!

Also, '--version2' is a sloppy name, the right solution is to do what Git does, it has a --build-options sub-option to 'perf version':

triton:~/tip> git version -h
usage: git version [<options>]

--build-options also print build options

   triton:~/tip> git version --build-options
   git version 2.14.1
   sizeof-long: 8
Quoted 7 lines

This should be done for perf as well, and _then_ maybe can we add a helper alias that maps '-vv' to 'version --build-options'.

Thanks,

Ingo

Something like 'perf version --build-options' and that will be mapped to '-vv'.

It's a good idea, thanks!

Thanks
Jin Yao

#17 lore raw
In reply to Jin Yao
Quoted 10 lines · Jin Yao

We keep having bug reports that when users build perf on their own, but they don't install some needed libraries such as libelf, libbfd/libibery.

The perf can build, but it is missing important functionality. And users may complain that perf has issue or bug.

This patch-set support 'perf -vv' which will print the compiled-in status of libraries. Once users think perf missing some functionality, it should be very easy for them to check the libraries status.

I don't think this solves the problem. How should the user know that they need to run perf -vv. Also normal users don't know that libelf is needed for symbols for example.

We need a warning that is visible together with the symbols and that clearly describes the problem.

-Andi

Quoted 39 lines

For example:

$ ./perf -vv
perf version 4.13.rc5.g9b7a81b
                 dwarf: [  on ]
    dwarf_getlocations: [  on ]
                 glibc: [  on ]
                  gtk2: [  on ]
              libaudit: [ off ]
                libbfd: [  on ]
                libelf: [  on ]
               libnuma: [  on ]
numa_num_possible_cpus: [  on ]
               libperl: [  on ]
             libpython: [  on ]
              libslang: [  on ]
             libcrypto: [  on ]
             libunwind: [  on ]
    libdw-dwarf-unwind: [  on ]
                  zlib: [  on ]
                  lzma: [  on ]
             get_cpuid: [  on ]
                   bpf: [  on ]

Jin Yao (3):
perf config: Add -DNO_GLIBC to CFLAGS
perf version: Print the status of compiled-in libraries
perf: Support perf -vv

 tools/perf/Makefile.config   |   2 +
 tools/perf/builtin-version.c | 125 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/builtin.h         |   1 +
 tools/perf/perf.c            |   6 +++
 4 files changed, 134 insertions(+)
Signature · 1 line
2.7.4
#18 lore raw
Quoted 12 lines · Andi Kleen
Quoted 10 lines · Jin Yao

We keep having bug reports that when users build perf on their own, but they don't install some needed libraries such as libelf, libbfd/libibery.

The perf can build, but it is missing important functionality. And users may complain that perf has issue or bug.

This patch-set support 'perf -vv' which will print the compiled-in status of libraries. Once users think perf missing some functionality, it should be very easy for them to check the libraries status.

Quoted 6 lines

I don't think this solves the problem. How should the user know that they need to run perf -vv. Also normal users don't know that libelf is needed for symbols for example.

We need a warning that is visible together with the symbols and that clearly describes the problem.

IIRC we decided to go *with* the message in the perf report or other affected command that would suggest to run 'perf -vv' for more details

jirka

Quoted 42 lines

-Andi

Quoted 39 lines

For example:

$ ./perf -vv
perf version 4.13.rc5.g9b7a81b
                 dwarf: [  on ]
    dwarf_getlocations: [  on ]
                 glibc: [  on ]
                  gtk2: [  on ]
              libaudit: [ off ]
                libbfd: [  on ]
                libelf: [  on ]
               libnuma: [  on ]
numa_num_possible_cpus: [  on ]
               libperl: [  on ]
             libpython: [  on ]
              libslang: [  on ]
             libcrypto: [  on ]
             libunwind: [  on ]
    libdw-dwarf-unwind: [  on ]
                  zlib: [  on ]
                  lzma: [  on ]
             get_cpuid: [  on ]
                   bpf: [  on ]

Jin Yao (3):
perf config: Add -DNO_GLIBC to CFLAGS
perf version: Print the status of compiled-in libraries
perf: Support perf -vv

 tools/perf/Makefile.config   |   2 +
 tools/perf/builtin-version.c | 125 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/builtin.h         |   1 +
 tools/perf/perf.c            |   6 +++
 4 files changed, 134 insertions(+)
Signature · 1 line
2.7.4
author
#19 lore raw
Quoted 17 lines · Jiri Olsa
Quoted 16 lines · Andi Kleen
Quoted 10 lines · Jin Yao

We keep having bug reports that when users build perf on their own, but they don't install some needed libraries such as libelf, libbfd/libibery.

The perf can build, but it is missing important functionality. And users may complain that perf has issue or bug.

This patch-set support 'perf -vv' which will print the compiled-in status of libraries. Once users think perf missing some functionality, it should be very easy for them to check the libraries status.

I don't think this solves the problem. How should the user know that they need to run perf -vv. Also normal users don't know that libelf is needed for symbols for example.

Quoted 9 lines
Quoted 2 lines

We need a warning that is visible together with the symbols and that clearly describes the problem.

IIRC we decided to go *with* the message in the perf report or other affected command that would suggest to run 'perf -vv' for more details

jirka

Hi Andi,

For how the users know they should run 'perf -vv', as the discussion with Jiri and Arnaldo, the message will be displayed if necessary when users perform perf report or other perf commands. That could be implemented in a follow-up patch (not in this patch-set).

For let user know what the library they need, maybe we can add more information in 'perf -vv', for example, add something like,

"Please install libelf-dev, libelf-devel or elfutils-libelf-devel before building perf"

Thanks
Jin Yao

Quoted 42 lines
Quoted 42 lines

-Andi

Quoted 39 lines

For example:

$ ./perf -vv
perf version 4.13.rc5.g9b7a81b
                  dwarf: [  on ]
     dwarf_getlocations: [  on ]
                  glibc: [  on ]
                   gtk2: [  on ]
               libaudit: [ off ]
                 libbfd: [  on ]
                 libelf: [  on ]
                libnuma: [  on ]
numa_num_possible_cpus: [  on ]
                libperl: [  on ]
              libpython: [  on ]
               libslang: [  on ]
              libcrypto: [  on ]
              libunwind: [  on ]
     libdw-dwarf-unwind: [  on ]
                   zlib: [  on ]
                   lzma: [  on ]
              get_cpuid: [  on ]
                    bpf: [  on ]

Jin Yao (3):
perf config: Add -DNO_GLIBC to CFLAGS
perf version: Print the status of compiled-in libraries
perf: Support perf -vv

  tools/perf/Makefile.config   |   2 +
  tools/perf/builtin-version.c | 125 +++++++++++++++++++++++++++++++++++++++++++
  tools/perf/builtin.h         |   1 +
  tools/perf/perf.c            |   6 +++
  4 files changed, 134 insertions(+)
Signature · 1 line
2.7.4