linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf util: Display warning when perf report/annotate is missing some libs
@ 2018-01-11 11:03 Jin Yao
  2018-01-11 15:30 ` Jiri Olsa
  0 siblings, 1 reply; 17+ messages in thread
From: Jin Yao @ 2018-01-11 11:03 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

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

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

For example, perf report doesn't display any symbols and perf annotate
doesn't work.

This patch displays warnings that these libraries are missing in build
when perf report / perf annotate are used.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/builtin-annotate.c |  2 ++
 tools/perf/builtin-report.c   |  2 ++
 tools/perf/util/symbol.c      | 21 +++++++++++++++++++++
 tools/perf/util/symbol.h      |  2 ++
 4 files changed, 27 insertions(+)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index f15731a..609eb7a 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -511,6 +511,8 @@ int cmd_annotate(int argc, const char **argv)
 
 	setup_browser(true);
 
+	build_lib_warning();
+
 	ret = __cmd_annotate(&annotate);
 
 out_delete:
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index dd4df9a..550adb7 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1293,6 +1293,8 @@ int cmd_report(int argc, const char **argv)
 		}
 	}
 
+	build_lib_warning();
+
 	if (symbol__init(&session->header.env) < 0)
 		goto error;
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index cc065d4..4205ba8 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2224,3 +2224,24 @@ int symbol__config_symfs(const struct option *opt __maybe_unused,
 	free(bf);
 	return 0;
 }
+
+void build_lib_warning(void)
+{
+#ifndef HAVE_LIBELF_SUPPORT
+	pr_warning("Symbols are disabled!\n"
+		   "Please install libelf-dev, libelf-devel "
+		   "or elfutils-libelf-devel before building perf.\n");
+#endif
+
+#ifndef HAVE_DWARF_SUPPORT
+	pr_warning("Unwind support is disabled!\n"
+		   "Please install elfutils-devel/libdw-dev "
+		   "before building perf.\n");
+#endif
+
+#ifndef HAVE_LIBBFD_SUPPORT
+	pr_warning("C++ demangling and line numbers are disabled!\n"
+		   "Please install binutils-dev[el]/"
+		   "libiberty-dev before building perf.\n");
+#endif
+}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 0563f33..d88e046 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -383,6 +383,8 @@ int get_sdt_note_list(struct list_head *head, const char *target);
 int cleanup_sdt_note_list(struct list_head *sdt_notes);
 int sdt_notes__get_count(struct list_head *start);
 
+void build_lib_warning(void);
+
 #define SDT_BASE_SCN ".stapsdt.base"
 #define SDT_NOTE_SCN  ".note.stapsdt"
 #define SDT_NOTE_TYPE 3
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-01-11 11:03 [PATCH] perf util: Display warning when perf report/annotate is missing some libs Jin Yao
@ 2018-01-11 15:30 ` Jiri Olsa
  2018-01-12  2:22   ` Jin, Yao
  0 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2018-01-11 15:30 UTC (permalink / raw)
  To: Jin Yao
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

On Thu, Jan 11, 2018 at 07:03:06PM +0800, Jin Yao wrote:
> We keep having bug reports that when users build perf on their own,

we already have same warnings during the build

> but they don't install some needed libraries like libelf, libbfd/libibery.

how about saying that in the symbol column,
instead of poluting report's output, like:

	$ perf report --stdio

	# Overhead  Command  Shared Object     Symbol (disabled)
	# ........  .......  ................  ......................

also your change does not affect tui mode

annotation for some reason does not start at all.. could be
little more verbose ;-)

jirka

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-01-11 15:30 ` Jiri Olsa
@ 2018-01-12  2:22   ` Jin, Yao
  2018-03-21  2:11     ` Jin, Yao
  0 siblings, 1 reply; 17+ messages in thread
From: Jin, Yao @ 2018-01-12  2:22 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin



On 1/11/2018 11:30 PM, Jiri Olsa wrote:
> On Thu, Jan 11, 2018 at 07:03:06PM +0800, Jin Yao wrote:
>> We keep having bug reports that when users build perf on their own,
> 
> we already have same warnings during the build
> 

Yes, there will be warnings displayed during the build if some libraries 
are missing.

While I do find the users (especially the new perf user) don't notice 
the warning. They only care about the success of build.

Once they use this perf version and find something working strangely, 
they will say that perf may have bug. :(

>> but they don't install some needed libraries like libelf, libbfd/libibery.
> 
> how about saying that in the symbol column,
> instead of poluting report's output, like:
> 
> 	$ perf report --stdio
> 
> 	# Overhead  Command  Shared Object     Symbol (disabled)
> 	# ........  .......  ................  ......................
> 

I just think it'd better provide some hints to user. For example, 
"symbol is disabled and you need to install libelf/xxx", say something 
like that.

But it looks the column can't contain too much information (i.e. no more 
space to contain the entire hints).

Any idea? Or just add this warning in verbose mode?

> also your change does not affect tui mode
> 
> annotation for some reason does not start at all.. could be
> little more verbose ;-)
> 
> jirka
> 

Yes, it doesn't affect tui mode.

Or we just add this warning in verbose mode?

e.g. perf report -v?

Thanks
Jin Yao

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-01-12  2:22   ` Jin, Yao
@ 2018-03-21  2:11     ` Jin, Yao
  2018-03-21 15:38       ` Jiri Olsa
  0 siblings, 1 reply; 17+ messages in thread
From: Jin, Yao @ 2018-03-21  2:11 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

Hi Jiri,

I'm still thinking it's worth displaying the warning when perf missing 
some libraries.

Somebody just told me that perf didn't work well. While after some 
investigations, I found it's just missing some libraries when building 
the perf.

But I have spent some time on getting the root cause. If with this 
patch, it should be very easily to know that.

Thanks
Jin Yao

On 1/12/2018 10:22 AM, Jin, Yao wrote:
> 
> 
> On 1/11/2018 11:30 PM, Jiri Olsa wrote:
>> On Thu, Jan 11, 2018 at 07:03:06PM +0800, Jin Yao wrote:
>>> We keep having bug reports that when users build perf on their own,
>>
>> we already have same warnings during the build
>>
> 
> Yes, there will be warnings displayed during the build if some libraries 
> are missing.
> 
> While I do find the users (especially the new perf user) don't notice 
> the warning. They only care about the success of build.
> 
> Once they use this perf version and find something working strangely, 
> they will say that perf may have bug. :(
> 
>>> but they don't install some needed libraries like libelf, 
>>> libbfd/libibery.
>>
>> how about saying that in the symbol column,
>> instead of poluting report's output, like:
>>
>>     $ perf report --stdio
>>
>>     # Overhead  Command  Shared Object     Symbol (disabled)
>>     # ........  .......  ................  ......................
>>
> 
> I just think it'd better provide some hints to user. For example, 
> "symbol is disabled and you need to install libelf/xxx", say something 
> like that.
> 
> But it looks the column can't contain too much information (i.e. no more 
> space to contain the entire hints).
> 
> Any idea? Or just add this warning in verbose mode?
> 
>> also your change does not affect tui mode
>>
>> annotation for some reason does not start at all.. could be
>> little more verbose ;-)
>>
>> jirka
>>
> 
> Yes, it doesn't affect tui mode.
> 
> Or we just add this warning in verbose mode?
> 
> e.g. perf report -v?
> 
> Thanks
> Jin Yao

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-21  2:11     ` Jin, Yao
@ 2018-03-21 15:38       ` Jiri Olsa
  2018-03-21 15:40         ` Arnaldo Carvalho de Melo
  2018-03-22  1:04         ` Jin, Yao
  0 siblings, 2 replies; 17+ messages in thread
From: Jiri Olsa @ 2018-03-21 15:38 UTC (permalink / raw)
  To: Jin, Yao
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

On Wed, Mar 21, 2018 at 10:11:10AM +0800, Jin, Yao wrote:
> Hi Jiri,
> 
> I'm still thinking it's worth displaying the warning when perf missing some
> libraries.
> 
> Somebody just told me that perf didn't work well. While after some
> investigations, I found it's just missing some libraries when building the
> perf.
> 
> But I have spent some time on getting the root cause. If with this patch, it
> should be very easily to know that.

true.. Arnaldo, any feedback on this one?

> > I just think it'd better provide some hints to user. For example,
> > "symbol is disabled and you need to install libelf/xxx", say something
> > like that.
> > 
> > But it looks the column can't contain too much information (i.e. no more
> > space to contain the entire hints).
> > 
> > Any idea? Or just add this warning in verbose mode?
> > 
> > > also your change does not affect tui mode
> > > 
> > > annotation for some reason does not start at all.. could be
> > > little more verbose ;-)
> > > 
> > > jirka
> > > 
> > 
> > Yes, it doesn't affect tui mode.
> > 
> > Or we just add this warning in verbose mode?
> > 
> > e.g. perf report -v?

how about displaying libraries separately with -vv output,
that would mimic the build message, like:

  $ ./perf -vv
  perf version 4.16.rc6.g18fd48

                   dwarf: [ on  ]
      dwarf_getlocations: [ on  ]
                   glibc: [ on  ]
                    gtk2: [ on  ]
                libaudit: [ on  ]
                  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  ]

and perf -vvv could display the 'make VF=1' info

jirka

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-21 15:38       ` Jiri Olsa
@ 2018-03-21 15:40         ` Arnaldo Carvalho de Melo
  2018-03-21 15:43           ` Arnaldo Carvalho de Melo
  2018-03-22  1:04         ` Jin, Yao
  1 sibling, 1 reply; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-03-21 15:40 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jin, Yao, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
	ak, kan.liang, yao.jin

Em Wed, Mar 21, 2018 at 04:38:07PM +0100, Jiri Olsa escreveu:
> On Wed, Mar 21, 2018 at 10:11:10AM +0800, Jin, Yao wrote:
> > Hi Jiri,
> > 
> > I'm still thinking it's worth displaying the warning when perf missing some
> > libraries.
> > 
> > Somebody just told me that perf didn't work well. While after some
> > investigations, I found it's just missing some libraries when building the
> > perf.
> > 
> > But I have spent some time on getting the root cause. If with this patch, it
> > should be very easily to know that.
> 
> true.. Arnaldo, any feedback on this one?

Lemme re-read the thread...

 
> > > I just think it'd better provide some hints to user. For example,
> > > "symbol is disabled and you need to install libelf/xxx", say something
> > > like that.
> > > 
> > > But it looks the column can't contain too much information (i.e. no more
> > > space to contain the entire hints).
> > > 
> > > Any idea? Or just add this warning in verbose mode?
> > > 
> > > > also your change does not affect tui mode
> > > > 
> > > > annotation for some reason does not start at all.. could be
> > > > little more verbose ;-)
> > > > 
> > > > jirka
> > > > 
> > > 
> > > Yes, it doesn't affect tui mode.
> > > 
> > > Or we just add this warning in verbose mode?
> > > 
> > > e.g. perf report -v?
> 
> how about displaying libraries separately with -vv output,
> that would mimic the build message, like:
> 
>   $ ./perf -vv
>   perf version 4.16.rc6.g18fd48
> 
>                    dwarf: [ on  ]
>       dwarf_getlocations: [ on  ]
>                    glibc: [ on  ]
>                     gtk2: [ on  ]
>                 libaudit: [ on  ]
>                   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  ]
> 
> and perf -vvv could display the 'make VF=1' info
> 
> jirka

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-21 15:40         ` Arnaldo Carvalho de Melo
@ 2018-03-21 15:43           ` Arnaldo Carvalho de Melo
  2018-03-21 15:45             ` Arnaldo Carvalho de Melo
  2018-03-21 16:04             ` Jiri Olsa
  0 siblings, 2 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-03-21 15:43 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jin, Yao, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
	ak, kan.liang, yao.jin

Em Wed, Mar 21, 2018 at 12:40:35PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Mar 21, 2018 at 04:38:07PM +0100, Jiri Olsa escreveu:
> > On Wed, Mar 21, 2018 at 10:11:10AM +0800, Jin, Yao wrote:
> > > Hi Jiri,
> > > 
> > > I'm still thinking it's worth displaying the warning when perf missing some
> > > libraries.
> > > 
> > > Somebody just told me that perf didn't work well. While after some
> > > investigations, I found it's just missing some libraries when building the
> > > perf.
> > > 
> > > But I have spent some time on getting the root cause. If with this patch, it
> > > should be very easily to know that.
> > 
> > true.. Arnaldo, any feedback on this one?
> 
> Lemme re-read the thread...

Well, how about we make it harder to build without key libraries? I.e.
if we detect that what we consider a core set of libraries isn't found
in the system, then we stop the build, warn about it and ask the user to
confirm that the build should proceed by passing some explicit
-DI_KNOW_WHAT_I_AM_DOING___PROCEED=doit

?

- Arnaldo
 
>  
> > > > I just think it'd better provide some hints to user. For example,
> > > > "symbol is disabled and you need to install libelf/xxx", say something
> > > > like that.
> > > > 
> > > > But it looks the column can't contain too much information (i.e. no more
> > > > space to contain the entire hints).
> > > > 
> > > > Any idea? Or just add this warning in verbose mode?
> > > > 
> > > > > also your change does not affect tui mode
> > > > > 
> > > > > annotation for some reason does not start at all.. could be
> > > > > little more verbose ;-)
> > > > > 
> > > > > jirka
> > > > > 
> > > > 
> > > > Yes, it doesn't affect tui mode.
> > > > 
> > > > Or we just add this warning in verbose mode?
> > > > 
> > > > e.g. perf report -v?
> > 
> > how about displaying libraries separately with -vv output,
> > that would mimic the build message, like:
> > 
> >   $ ./perf -vv
> >   perf version 4.16.rc6.g18fd48
> > 
> >                    dwarf: [ on  ]
> >       dwarf_getlocations: [ on  ]
> >                    glibc: [ on  ]
> >                     gtk2: [ on  ]
> >                 libaudit: [ on  ]
> >                   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  ]
> > 
> > and perf -vvv could display the 'make VF=1' info
> > 
> > jirka

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-21 15:43           ` Arnaldo Carvalho de Melo
@ 2018-03-21 15:45             ` Arnaldo Carvalho de Melo
  2018-03-21 16:04             ` Jiri Olsa
  1 sibling, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-03-21 15:45 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jin, Yao, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
	ak, kan.liang, yao.jin

Em Wed, Mar 21, 2018 at 12:43:15PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Mar 21, 2018 at 12:40:35PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Mar 21, 2018 at 04:38:07PM +0100, Jiri Olsa escreveu:
> > > On Wed, Mar 21, 2018 at 10:11:10AM +0800, Jin, Yao wrote:
> > > > Hi Jiri,
> > > > 
> > > > I'm still thinking it's worth displaying the warning when perf missing some
> > > > libraries.
> > > > 
> > > > Somebody just told me that perf didn't work well. While after some
> > > > investigations, I found it's just missing some libraries when building the
> > > > perf.
> > > > 
> > > > But I have spent some time on getting the root cause. If with this patch, it
> > > > should be very easily to know that.
> > > 
> > > true.. Arnaldo, any feedback on this one?
> > 
> > Lemme re-read the thread...
> 
> Well, how about we make it harder to build without key libraries? I.e.
> if we detect that what we consider a core set of libraries isn't found
> in the system, then we stop the build, warn about it and ask the user to
> confirm that the build should proceed by passing some explicit
> -DI_KNOW_WHAT_I_AM_DOING___PROCEED=doit
> 
> ?

And for some stuff, that we know people feel strongly about, like PeterZ
with the TUI, the warning has to be subtle, like a one line "Please
build with slang-devel to have interactive annotation".

- Arnaldo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-21 15:43           ` Arnaldo Carvalho de Melo
  2018-03-21 15:45             ` Arnaldo Carvalho de Melo
@ 2018-03-21 16:04             ` Jiri Olsa
  2018-03-21 18:52               ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2018-03-21 16:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jin, Yao, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
	ak, kan.liang, yao.jin

On Wed, Mar 21, 2018 at 12:43:15PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 21, 2018 at 12:40:35PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Mar 21, 2018 at 04:38:07PM +0100, Jiri Olsa escreveu:
> > > On Wed, Mar 21, 2018 at 10:11:10AM +0800, Jin, Yao wrote:
> > > > Hi Jiri,
> > > > 
> > > > I'm still thinking it's worth displaying the warning when perf missing some
> > > > libraries.
> > > > 
> > > > Somebody just told me that perf didn't work well. While after some
> > > > investigations, I found it's just missing some libraries when building the
> > > > perf.
> > > > 
> > > > But I have spent some time on getting the root cause. If with this patch, it
> > > > should be very easily to know that.
> > > 
> > > true.. Arnaldo, any feedback on this one?
> > 
> > Lemme re-read the thread...
> 
> Well, how about we make it harder to build without key libraries? I.e.
> if we detect that what we consider a core set of libraries isn't found
> in the system, then we stop the build, warn about it and ask the user to
> confirm that the build should proceed by passing some explicit
> -DI_KNOW_WHAT_I_AM_DOING___PROCEED=doit

hum, not sure we want to complicate the build even more than it
is now :-\ and IMO it still won't help much in Jin's problem,
if user forces the build anyway

jirka

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-21 16:04             ` Jiri Olsa
@ 2018-03-21 18:52               ` Arnaldo Carvalho de Melo
  2018-03-21 19:02                 ` Jiri Olsa
  2018-03-22  1:31                 ` Jin, Yao
  0 siblings, 2 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-03-21 18:52 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jin, Yao, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
	ak, kan.liang, yao.jin

Em Wed, Mar 21, 2018 at 05:04:46PM +0100, Jiri Olsa escreveu:
> On Wed, Mar 21, 2018 at 12:43:15PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Mar 21, 2018 at 12:40:35PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Wed, Mar 21, 2018 at 04:38:07PM +0100, Jiri Olsa escreveu:
> > > > On Wed, Mar 21, 2018 at 10:11:10AM +0800, Jin, Yao wrote:
> > > > > Hi Jiri,
> > > > > 
> > > > > I'm still thinking it's worth displaying the warning when perf missing some
> > > > > libraries.
> > > > > 
> > > > > Somebody just told me that perf didn't work well. While after some
> > > > > investigations, I found it's just missing some libraries when building the
> > > > > perf.
> > > > > 
> > > > > But I have spent some time on getting the root cause. If with this patch, it
> > > > > should be very easily to know that.

> > > > true.. Arnaldo, any feedback on this one?

> > > Lemme re-read the thread...

> > Well, how about we make it harder to build without key libraries? I.e.
> > if we detect that what we consider a core set of libraries isn't found
> > in the system, then we stop the build, warn about it and ask the user to
> > confirm that the build should proceed by passing some explicit
> > -DI_KNOW_WHAT_I_AM_DOING___PROCEED=doit

> hum, not sure we want to complicate the build even more than it
> is now :-\ and IMO it still won't help much in Jin's problem,
> if user forces the build anyway

Well, if a user _forces_ a build, not taking into consideration a
warning that _is_ emitted and _stops_ the build, about the functionality
it will lose by doing forcing the build, then comes back and complains
that that functionality is not present, then it becomes difficult to
help this user...  :-)

On the other hand, if the user forgets to install an important library,
the warning is emitted but the build proceeds, no explicit action was
performed, just a warning wasn't noticed, and the user complains, then
I'd say: "hey, are you sure library foo devel files were present when
you build it?", i.e. the support back and forth Jin is trying to avoid.

And for users that _saw_ the warning, _knew_ they _didn't_ want that
functionality, to be reminded while running, say 'perf report' that
something they _decided not to have_ isn't present, then that could be
annoying, no?

Lemme try another idea: what if we do something like gcc does and print
the features present when showing the version?

I.e.:

[acme@jouet perf]$ gcc -v
Using built-in specs.
COLLECT_GCC=/usr/bin/gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/7/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-redhat-linux
Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,objc,obj-c++,fortran,ada,go,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-gcc-major-version-only --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --with-isl --enable-libmpx --enable-offload-targets=nvptx-none --without-cuda-driver --enable-gnu-indirect-function --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
Thread model: posix
gcc version 7.3.1 20180303 (Red Hat 7.3.1-5) (GCC) 
[acme@jouet perf]$ 

- Arnaldo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-21 18:52               ` Arnaldo Carvalho de Melo
@ 2018-03-21 19:02                 ` Jiri Olsa
  2018-03-22  1:31                 ` Jin, Yao
  1 sibling, 0 replies; 17+ messages in thread
From: Jiri Olsa @ 2018-03-21 19:02 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jin, Yao, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
	ak, kan.liang, yao.jin

On Wed, Mar 21, 2018 at 03:52:37PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 21, 2018 at 05:04:46PM +0100, Jiri Olsa escreveu:
> > On Wed, Mar 21, 2018 at 12:43:15PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Wed, Mar 21, 2018 at 12:40:35PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > Em Wed, Mar 21, 2018 at 04:38:07PM +0100, Jiri Olsa escreveu:
> > > > > On Wed, Mar 21, 2018 at 10:11:10AM +0800, Jin, Yao wrote:
> > > > > > Hi Jiri,
> > > > > > 
> > > > > > I'm still thinking it's worth displaying the warning when perf missing some
> > > > > > libraries.
> > > > > > 
> > > > > > Somebody just told me that perf didn't work well. While after some
> > > > > > investigations, I found it's just missing some libraries when building the
> > > > > > perf.
> > > > > > 
> > > > > > But I have spent some time on getting the root cause. If with this patch, it
> > > > > > should be very easily to know that.
> 
> > > > > true.. Arnaldo, any feedback on this one?
> 
> > > > Lemme re-read the thread...
> 
> > > Well, how about we make it harder to build without key libraries? I.e.
> > > if we detect that what we consider a core set of libraries isn't found
> > > in the system, then we stop the build, warn about it and ask the user to
> > > confirm that the build should proceed by passing some explicit
> > > -DI_KNOW_WHAT_I_AM_DOING___PROCEED=doit
> 
> > hum, not sure we want to complicate the build even more than it
> > is now :-\ and IMO it still won't help much in Jin's problem,
> > if user forces the build anyway
> 
> Well, if a user _forces_ a build, not taking into consideration a
> warning that _is_ emitted and _stops_ the build, about the functionality
> it will lose by doing forcing the build, then comes back and complains
> that that functionality is not present, then it becomes difficult to
> help this user...  :-)
> 
> On the other hand, if the user forgets to install an important library,
> the warning is emitted but the build proceeds, no explicit action was
> performed, just a warning wasn't noticed, and the user complains, then
> I'd say: "hey, are you sure library foo devel files were present when
> you build it?", i.e. the support back and forth Jin is trying to avoid.
> 
> And for users that _saw_ the warning, _knew_ they _didn't_ want that
> functionality, to be reminded while running, say 'perf report' that
> something they _decided not to have_ isn't present, then that could be
> annoying, no?
> 
> Lemme try another idea: what if we do something like gcc does and print
> the features present when showing the version?
> 
> I.e.:
> 
> [acme@jouet perf]$ gcc -v
> Using built-in specs.
> COLLECT_GCC=/usr/bin/gcc
> COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/7/lto-wrapper
> OFFLOAD_TARGET_NAMES=nvptx-none
> OFFLOAD_TARGET_DEFAULT=1
> Target: x86_64-redhat-linux
> Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,objc,obj-c++,fortran,ada,go,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-gcc-major-version-only --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --with-isl --enable-libmpx --enable-offload-targets=nvptx-none --without-cuda-driver --enable-gnu-indirect-function --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
> Thread model: posix
> gcc version 7.3.1 20180303 (Red Hat 7.3.1-5) (GCC) 
> [acme@jouet perf]$ 
> 
> - Arnaldo

yep I guess you overlooked it in my previous reply ;-)

jirka

---
> how about displaying libraries separately with -vv output,
> that would mimic the build message, like:
>
>   $ ./perf -vv
>   perf version 4.16.rc6.g18fd48
>
>                    dwarf: [ on  ]
>       dwarf_getlocations: [ on  ]
>                    glibc: [ on  ]
>                     gtk2: [ on  ]
>                 libaudit: [ on  ]
>                   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  ]
>
> and perf -vvv could display the 'make VF=1' info
>
> jirka

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-21 15:38       ` Jiri Olsa
  2018-03-21 15:40         ` Arnaldo Carvalho de Melo
@ 2018-03-22  1:04         ` Jin, Yao
  2018-03-22  8:51           ` Jiri Olsa
  1 sibling, 1 reply; 17+ messages in thread
From: Jin, Yao @ 2018-03-22  1:04 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin



On 3/21/2018 11:38 PM, Jiri Olsa wrote:
> On Wed, Mar 21, 2018 at 10:11:10AM +0800, Jin, Yao wrote:
>> Hi Jiri,
>>
>> I'm still thinking it's worth displaying the warning when perf missing some
>> libraries.
>>
>> Somebody just told me that perf didn't work well. While after some
>> investigations, I found it's just missing some libraries when building the
>> perf.
>>
>> But I have spent some time on getting the root cause. If with this patch, it
>> should be very easily to know that.
> 
> true.. Arnaldo, any feedback on this one?
> 
>>> I just think it'd better provide some hints to user. For example,
>>> "symbol is disabled and you need to install libelf/xxx", say something
>>> like that.
>>>
>>> But it looks the column can't contain too much information (i.e. no more
>>> space to contain the entire hints).
>>>
>>> Any idea? Or just add this warning in verbose mode?
>>>
>>>> also your change does not affect tui mode
>>>>
>>>> annotation for some reason does not start at all.. could be
>>>> little more verbose ;-)
>>>>
>>>> jirka
>>>>
>>>
>>> Yes, it doesn't affect tui mode.
>>>
>>> Or we just add this warning in verbose mode?
>>>
>>> e.g. perf report -v?
> 
> how about displaying libraries separately with -vv output,
> that would mimic the build message, like:
> 
>    $ ./perf -vv
>    perf version 4.16.rc6.g18fd48
> 
>                     dwarf: [ on  ]
>        dwarf_getlocations: [ on  ]
>                     glibc: [ on  ]
>                      gtk2: [ on  ]
>                  libaudit: [ on  ]
>                    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  ]
> 
> and perf -vvv could display the 'make VF=1' info
> 
> jirka
> 

I'm just afraid that the newbie will not check the -vv on his own when 
he gets trouble in using perf.

In other words, if a user is experienced and he knows -vv yet, I may 
assume that he should know installing all libraries before building the 
perf.

This patch is specific for the perf newbie. It will directly shows the 
error/warning when the user launches the perf binary. It will have a 
little bit helps, I guess. :)

Thanks
Jin Yao

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-21 18:52               ` Arnaldo Carvalho de Melo
  2018-03-21 19:02                 ` Jiri Olsa
@ 2018-03-22  1:31                 ` Jin, Yao
  1 sibling, 0 replies; 17+ messages in thread
From: Jin, Yao @ 2018-03-22  1:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin



On 3/22/2018 2:52 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 21, 2018 at 05:04:46PM +0100, Jiri Olsa escreveu:
>> On Wed, Mar 21, 2018 at 12:43:15PM -0300, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, Mar 21, 2018 at 12:40:35PM -0300, Arnaldo Carvalho de Melo escreveu:
>>>> Em Wed, Mar 21, 2018 at 04:38:07PM +0100, Jiri Olsa escreveu:
>>>>> On Wed, Mar 21, 2018 at 10:11:10AM +0800, Jin, Yao wrote:
>>>>>> Hi Jiri,
>>>>>>
>>>>>> I'm still thinking it's worth displaying the warning when perf missing some
>>>>>> libraries.
>>>>>>
>>>>>> Somebody just told me that perf didn't work well. While after some
>>>>>> investigations, I found it's just missing some libraries when building the
>>>>>> perf.
>>>>>>
>>>>>> But I have spent some time on getting the root cause. If with this patch, it
>>>>>> should be very easily to know that.
> 
>>>>> true.. Arnaldo, any feedback on this one?
> 
>>>> Lemme re-read the thread...
> 
>>> Well, how about we make it harder to build without key libraries? I.e.
>>> if we detect that what we consider a core set of libraries isn't found
>>> in the system, then we stop the build, warn about it and ask the user to
>>> confirm that the build should proceed by passing some explicit
>>> -DI_KNOW_WHAT_I_AM_DOING___PROCEED=doit
> 
>> hum, not sure we want to complicate the build even more than it
>> is now :-\ and IMO it still won't help much in Jin's problem,
>> if user forces the build anyway
> 
> Well, if a user _forces_ a build, not taking into consideration a
> warning that _is_ emitted and _stops_ the build, about the functionality
> it will lose by doing forcing the build, then comes back and complains
> that that functionality is not present, then it becomes difficult to
> help this user...  :-)
> 
> On the other hand, if the user forgets to install an important library,
> the warning is emitted but the build proceeds, no explicit action was
> performed, just a warning wasn't noticed, and the user complains, then
> I'd say: "hey, are you sure library foo devel files were present when
> you build it?", i.e. the support back and forth Jin is trying to avoid.
> 
> And for users that _saw_ the warning, _knew_ they _didn't_ want that
> functionality, to be reminded while running, say 'perf report' that
> something they _decided not to have_ isn't present, then that could be
> annoying, no?
> 
> Lemme try another idea: what if we do something like gcc does and print
> the features present when showing the version?
> 
> I.e.:
> 
> [acme@jouet perf]$ gcc -v
> Using built-in specs.
> COLLECT_GCC=/usr/bin/gcc
> COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/7/lto-wrapper
> OFFLOAD_TARGET_NAMES=nvptx-none
> OFFLOAD_TARGET_DEFAULT=1
> Target: x86_64-redhat-linux
> Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,objc,obj-c++,fortran,ada,go,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-gcc-major-version-only --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --with-isl --enable-libmpx --enable-offload-targets=nvptx-none --without-cuda-driver --enable-gnu-indirect-function --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
> Thread model: posix
> gcc version 7.3.1 20180303 (Red Hat 7.3.1-5) (GCC)
> [acme@jouet perf]$
> 
> - Arnaldo
> 

Is this too complicated for perf newbie to understand?

For my problem, the mistake only occurs on perf newbie. I just think, 
it'd better return a direct and clear message to them. Maybe they don't 
know or don't use -v or -vv to do more investigation by themselves.

Thanks
Jin Yao

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-22  1:04         ` Jin, Yao
@ 2018-03-22  8:51           ` Jiri Olsa
  2018-03-23  3:09             ` Jin, Yao
  0 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2018-03-22  8:51 UTC (permalink / raw)
  To: Jin, Yao
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

On Thu, Mar 22, 2018 at 09:04:10AM +0800, Jin, Yao wrote:
> 
> 
> On 3/21/2018 11:38 PM, Jiri Olsa wrote:
> > On Wed, Mar 21, 2018 at 10:11:10AM +0800, Jin, Yao wrote:
> > > Hi Jiri,
> > > 
> > > I'm still thinking it's worth displaying the warning when perf missing some
> > > libraries.
> > > 
> > > Somebody just told me that perf didn't work well. While after some
> > > investigations, I found it's just missing some libraries when building the
> > > perf.
> > > 
> > > But I have spent some time on getting the root cause. If with this patch, it
> > > should be very easily to know that.
> > 
> > true.. Arnaldo, any feedback on this one?
> > 
> > > > I just think it'd better provide some hints to user. For example,
> > > > "symbol is disabled and you need to install libelf/xxx", say something
> > > > like that.
> > > > 
> > > > But it looks the column can't contain too much information (i.e. no more
> > > > space to contain the entire hints).
> > > > 
> > > > Any idea? Or just add this warning in verbose mode?
> > > > 
> > > > > also your change does not affect tui mode
> > > > > 
> > > > > annotation for some reason does not start at all.. could be
> > > > > little more verbose ;-)
> > > > > 
> > > > > jirka
> > > > > 
> > > > 
> > > > Yes, it doesn't affect tui mode.
> > > > 
> > > > Or we just add this warning in verbose mode?
> > > > 
> > > > e.g. perf report -v?
> > 
> > how about displaying libraries separately with -vv output,
> > that would mimic the build message, like:
> > 
> >    $ ./perf -vv
> >    perf version 4.16.rc6.g18fd48
> > 
> >                     dwarf: [ on  ]
> >        dwarf_getlocations: [ on  ]
> >                     glibc: [ on  ]
> >                      gtk2: [ on  ]
> >                  libaudit: [ on  ]
> >                    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  ]
> > 
> > and perf -vvv could display the 'make VF=1' info
> > 
> > jirka
> > 
> 
> I'm just afraid that the newbie will not check the -vv on his own when he
> gets trouble in using perf.
> 
> In other words, if a user is experienced and he knows -vv yet, I may assume
> that he should know installing all libraries before building the perf.
> 
> This patch is specific for the perf newbie. It will directly shows the
> error/warning when the user launches the perf binary. It will have a little
> bit helps, I guess. :)

I just don't like the idea that when you run perf report,
or annotate it spits out lines for every missing feature

maybe we could detect missing features for given command
and display line about missing features and say something
like:

'Warning: symbol,dwarf support not compiled in (for more details run perf -vv)'

or somwthing like that.. ;-)

jirka

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-22  8:51           ` Jiri Olsa
@ 2018-03-23  3:09             ` Jin, Yao
  2018-03-23 14:50               ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 17+ messages in thread
From: Jin, Yao @ 2018-03-23  3:09 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin



On 3/22/2018 4:51 PM, Jiri Olsa wrote:
> On Thu, Mar 22, 2018 at 09:04:10AM +0800, Jin, Yao wrote:
>>
>>
>> On 3/21/2018 11:38 PM, Jiri Olsa wrote:
>>> On Wed, Mar 21, 2018 at 10:11:10AM +0800, Jin, Yao wrote:
>>>> Hi Jiri,
>>>>
>>>> I'm still thinking it's worth displaying the warning when perf missing some
>>>> libraries.
>>>>
>>>> Somebody just told me that perf didn't work well. While after some
>>>> investigations, I found it's just missing some libraries when building the
>>>> perf.
>>>>
>>>> But I have spent some time on getting the root cause. If with this patch, it
>>>> should be very easily to know that.
>>>
>>> true.. Arnaldo, any feedback on this one?
>>>
>>>>> I just think it'd better provide some hints to user. For example,
>>>>> "symbol is disabled and you need to install libelf/xxx", say something
>>>>> like that.
>>>>>
>>>>> But it looks the column can't contain too much information (i.e. no more
>>>>> space to contain the entire hints).
>>>>>
>>>>> Any idea? Or just add this warning in verbose mode?
>>>>>
>>>>>> also your change does not affect tui mode
>>>>>>
>>>>>> annotation for some reason does not start at all.. could be
>>>>>> little more verbose ;-)
>>>>>>
>>>>>> jirka
>>>>>>
>>>>>
>>>>> Yes, it doesn't affect tui mode.
>>>>>
>>>>> Or we just add this warning in verbose mode?
>>>>>
>>>>> e.g. perf report -v?
>>>
>>> how about displaying libraries separately with -vv output,
>>> that would mimic the build message, like:
>>>
>>>     $ ./perf -vv
>>>     perf version 4.16.rc6.g18fd48
>>>
>>>                      dwarf: [ on  ]
>>>         dwarf_getlocations: [ on  ]
>>>                      glibc: [ on  ]
>>>                       gtk2: [ on  ]
>>>                   libaudit: [ on  ]
>>>                     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  ]
>>>
>>> and perf -vvv could display the 'make VF=1' info
>>>
>>> jirka
>>>
>>
>> I'm just afraid that the newbie will not check the -vv on his own when he
>> gets trouble in using perf.
>>
>> In other words, if a user is experienced and he knows -vv yet, I may assume
>> that he should know installing all libraries before building the perf.
>>
>> This patch is specific for the perf newbie. It will directly shows the
>> error/warning when the user launches the perf binary. It will have a little
>> bit helps, I guess. :)
> 
> I just don't like the idea that when you run perf report,
> or annotate it spits out lines for every missing feature
> 
> maybe we could detect missing features for given command
> and display line about missing features and say something
> like:
> 
> 'Warning: symbol,dwarf support not compiled in (for more details run perf -vv)'
> 
> or somwthing like that.. ;-)
> 
> jirka
> 

Hi Jiri,

I think your idea is very good!

I guess following it's just an example copied from perf building 
process, right?

$ ./perf -vv
perf version 4.16.rc6.g18fd48

                  dwarf: [ on  ]
     dwarf_getlocations: [ on  ]
                  glibc: [ on  ]
                   gtk2: [ on  ]
               libaudit: [ on  ]
                 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  ]

We can check some CFLAGS like "#ifdef HAVE_XXX" in perf code to 
determine if some libraries are compiled in.

For example,

#ifdef HAVE_LIBNUMA_SUPPORT
	printf("libnuma: [ on  ]");
#endif

For some features, such as "numa_num_possible_cpus", which doesn't have 
CFLAGS variables. Maybe we can ignore them in report?

I'd like to upgrade my patch to support perf -vv.

Thanks
Jin Yao

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-23  3:09             ` Jin, Yao
@ 2018-03-23 14:50               ` Arnaldo Carvalho de Melo
  2018-03-23 15:17                 ` Jiri Olsa
  0 siblings, 1 reply; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-03-23 14:50 UTC (permalink / raw)
  To: Jin, Yao
  Cc: Jiri Olsa, jolsa, peterz, mingo, alexander.shishkin,
	Linux-kernel, ak, kan.liang, yao.jin

Em Fri, Mar 23, 2018 at 11:09:03AM +0800, Jin, Yao escreveu:
> On 3/22/2018 4:51 PM, Jiri Olsa wrote:
> > On Thu, Mar 22, 2018 at 09:04:10AM +0800, Jin, Yao wrote:
> > > On 3/21/2018 11:38 PM, Jiri Olsa wrote:
> > > > On Wed, Mar 21, 2018 at 10:11:10AM +0800, Jin, Yao wrote:
> > > > > Hi Jiri,
> > > > > 
> > > > > I'm still thinking it's worth displaying the warning when perf missing some
> > > > > libraries.
> > > > > 
> > > > > Somebody just told me that perf didn't work well. While after some
> > > > > investigations, I found it's just missing some libraries when building the
> > > > > perf.
> > > > > 
> > > > > But I have spent some time on getting the root cause. If with this patch, it
> > > > > should be very easily to know that.
> > > > 
> > > > true.. Arnaldo, any feedback on this one?
> > > > 
> > > > > > I just think it'd better provide some hints to user. For example,
> > > > > > "symbol is disabled and you need to install libelf/xxx", say something
> > > > > > like that.
> > > > > > 
> > > > > > But it looks the column can't contain too much information (i.e. no more
> > > > > > space to contain the entire hints).
> > > > > > 
> > > > > > Any idea? Or just add this warning in verbose mode?
> > > > > > 
> > > > > > > also your change does not affect tui mode
> > > > > > > 
> > > > > > > annotation for some reason does not start at all.. could be
> > > > > > > little more verbose ;-)
> > > > > > > 
> > > > > > > jirka
> > > > > > > 
> > > > > > 
> > > > > > Yes, it doesn't affect tui mode.
> > > > > > 
> > > > > > Or we just add this warning in verbose mode?
> > > > > > 
> > > > > > e.g. perf report -v?
> > > > 
> > > > how about displaying libraries separately with -vv output,
> > > > that would mimic the build message, like:
> > > > 
> > > >     $ ./perf -vv
> > > >     perf version 4.16.rc6.g18fd48
> > > > 
> > > >                      dwarf: [ on  ]
> > > >         dwarf_getlocations: [ on  ]
> > > >                      glibc: [ on  ]
> > > >                       gtk2: [ on  ]
> > > >                   libaudit: [ on  ]
> > > >                     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  ]
> > > > 
> > > > and perf -vvv could display the 'make VF=1' info
> > > > 
> > > > jirka
> > > > 
> > > 
> > > I'm just afraid that the newbie will not check the -vv on his own when he
> > > gets trouble in using perf.
> > > 
> > > In other words, if a user is experienced and he knows -vv yet, I may assume
> > > that he should know installing all libraries before building the perf.
> > > 
> > > This patch is specific for the perf newbie. It will directly shows the
> > > error/warning when the user launches the perf binary. It will have a little
> > > bit helps, I guess. :)
> > 
> > I just don't like the idea that when you run perf report,
> > or annotate it spits out lines for every missing feature
> > 
> > maybe we could detect missing features for given command
> > and display line about missing features and say something
> > like:
> > 
> > 'Warning: symbol,dwarf support not compiled in (for more details run perf -vv)'
> > 
> > or somwthing like that.. ;-)
> > 
> > jirka
> > 
> 
> Hi Jiri,
> 
> I think your idea is very good!
> 
> I guess following it's just an example copied from perf building process,
> right?
> 
> $ ./perf -vv
> perf version 4.16.rc6.g18fd48
> 
>                  dwarf: [ on  ]
>     dwarf_getlocations: [ on  ]
>                  glibc: [ on  ]
>                   gtk2: [ on  ]
>               libaudit: [ on  ]
>                 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  ]
> 
> We can check some CFLAGS like "#ifdef HAVE_XXX" in perf code to determine if
> some libraries are compiled in.
> 
> For example,
> 
> #ifdef HAVE_LIBNUMA_SUPPORT
> 	printf("libnuma: [ on  ]");
> #endif
> 
> For some features, such as "numa_num_possible_cpus", which doesn't have
> CFLAGS variables. Maybe we can ignore them in report?
> 
> I'd like to upgrade my patch to support perf -vv.

Please go ahead! :-) We're all on the same page now, I think.

- Arnaldo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] perf util: Display warning when perf report/annotate is missing some libs
  2018-03-23 14:50               ` Arnaldo Carvalho de Melo
@ 2018-03-23 15:17                 ` Jiri Olsa
  0 siblings, 0 replies; 17+ messages in thread
From: Jiri Olsa @ 2018-03-23 15:17 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jin, Yao, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
	ak, kan.liang, yao.jin

On Fri, Mar 23, 2018 at 11:50:49AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> > > I just don't like the idea that when you run perf report,
> > > or annotate it spits out lines for every missing feature
> > > 
> > > maybe we could detect missing features for given command
> > > and display line about missing features and say something
> > > like:
> > > 
> > > 'Warning: symbol,dwarf support not compiled in (for more details run perf -vv)'
> > > 
> > > or somwthing like that.. ;-)
> > > 
> > > jirka
> > > 
> > 
> > Hi Jiri,
> > 
> > I think your idea is very good!
> > 
> > I guess following it's just an example copied from perf building process,
> > right?

yes

> > 
> > $ ./perf -vv
> > perf version 4.16.rc6.g18fd48
> > 
> >                  dwarf: [ on  ]
> >     dwarf_getlocations: [ on  ]
> >                  glibc: [ on  ]
> >                   gtk2: [ on  ]
> >               libaudit: [ on  ]
> >                 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  ]
> > 
> > We can check some CFLAGS like "#ifdef HAVE_XXX" in perf code to determine if
> > some libraries are compiled in.
> > 
> > For example,
> > 
> > #ifdef HAVE_LIBNUMA_SUPPORT
> > 	printf("libnuma: [ on  ]");
> > #endif

please display also the OFF status, to mirror the build output

#ifdef HAVE_LIBNUMA_SUPPORT
	printf("libnuma: [ on  ]");
#else
	printf("libnuma: [ OFF ]");
#endif

or in some other smarter way..

> > 
> > For some features, such as "numa_num_possible_cpus", which doesn't have
> > CFLAGS variables. Maybe we can ignore them in report?
> > 
> > I'd like to upgrade my patch to support perf -vv.
> 
> Please go ahead! :-) We're all on the same page now, I think.

yes ;-)

jirka

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-03-23 15:17 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-11 11:03 [PATCH] perf util: Display warning when perf report/annotate is missing some libs Jin Yao
2018-01-11 15:30 ` Jiri Olsa
2018-01-12  2:22   ` Jin, Yao
2018-03-21  2:11     ` Jin, Yao
2018-03-21 15:38       ` Jiri Olsa
2018-03-21 15:40         ` Arnaldo Carvalho de Melo
2018-03-21 15:43           ` Arnaldo Carvalho de Melo
2018-03-21 15:45             ` Arnaldo Carvalho de Melo
2018-03-21 16:04             ` Jiri Olsa
2018-03-21 18:52               ` Arnaldo Carvalho de Melo
2018-03-21 19:02                 ` Jiri Olsa
2018-03-22  1:31                 ` Jin, Yao
2018-03-22  1:04         ` Jin, Yao
2018-03-22  8:51           ` Jiri Olsa
2018-03-23  3:09             ` Jin, Yao
2018-03-23 14:50               ` Arnaldo Carvalho de Melo
2018-03-23 15:17                 ` Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).