Re: [GIT PULL 0/8] perf/pt -> Intel PT/BTS

[Adrian Hunter <adrian.hunter@intel.com>]

On 30/06/15 07:58, Ingo Molnar wrote:
> 
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
>> Hi Ingo,
>>
>> 	Please consider pulling, there are several other patches after this,
>> but I think that this may be acceptable to showcase the capabilities already
>> present, look at the output of 'perf script' and callchains for userspace
>> without using any extra debugging info (no need for DWARF, CFI, nothing),
>> really cool capabilities... :-)
>>
>> 	Adrian wrote some docs and I tested it both on a Ivy Bridge machine
>> where there is only BTS and on a Broadwell machine with the whole shebang,
>> adding the output of the commands to the csets, to further showcase what is
>> there already.
>>
>> 	This is on top of my last perf-core-for-mingo tag.
>>
>> 	Up to you, please let us know what you think and we'll continue working
>> on having this in an acceptable form for merging,
>>
>> Regards,
>>
>> - Arnaldo
>>
>> P.S. Kudos for Adrian for the patience with this process, way more is needed to
>> polish this, but the promise is there, cool stuff indeed! :-)
>>
>> The following changes since commit 36c8bb56a9f718a9a5f35d1834ca9dcec95deb4a:
>>
>>   perf symbols: Check access permission when reading symbol files (2015-06-26 12:11:53 -0300)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-pt-for-mingo
>>
>> for you to fetch changes up to 04759f172270afb28c8004f5cad62ed55710a499:
>>
>>   perf tools: Add Intel BTS support (2015-06-26 18:36:11 -0300)
>>
>> ----------------------------------------------------------------
>> Put Intel PT and BTS into initial use (Adrian Hunter)
>>
>> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>>
>> ----------------------------------------------------------------
>> Adrian Hunter (8):
>>       perf auxtrace: Add Intel PT as an AUX area tracing type
>>       perf tools: Add Intel PT packet decoder
>>       perf tools: Add Intel PT instruction decoder
>>       perf tools: Add Intel PT log
>>       perf tools: Add Intel PT decoder
>>       perf tools: Add Intel PT support
>>       perf tools: Take Intel PT into use
>>       perf tools: Add Intel BTS support
>>
>>  tools/build/Makefile.build                         |    2 +
>>  tools/perf/.gitignore                              |    2 +
>>  tools/perf/Documentation/intel-bts.txt             |   86 +
>>  tools/perf/Documentation/intel-pt.txt              |  588 ++++++
>>  tools/perf/MANIFEST                                |    7 +
>>  tools/perf/Makefile.perf                           |   12 +-
>>  tools/perf/arch/x86/util/Build                     |    5 +
>>  tools/perf/arch/x86/util/auxtrace.c                |   83 +
>>  tools/perf/arch/x86/util/intel-bts.c               |  458 +++++
>>  tools/perf/arch/x86/util/intel-pt.c                |  752 ++++++++
>>  tools/perf/arch/x86/util/pmu.c                     |   18 +
>>  tools/perf/util/Build                              |    3 +
>>  tools/perf/util/auxtrace.c                         |    9 +-
>>  tools/perf/util/auxtrace.h                         |    2 +
>>  tools/perf/util/intel-bts.c                        |  791 ++++++++
>>  tools/perf/util/intel-bts.h                        |   43 +
>>  tools/perf/util/intel-pt-decoder/Build             |   14 +
>>  .../perf/util/intel-pt-decoder/intel-pt-decoder.c  | 1759 ++++++++++++++++++
>>  .../perf/util/intel-pt-decoder/intel-pt-decoder.h  |  102 ++
>>  .../util/intel-pt-decoder/intel-pt-insn-decoder.c  |  246 +++
>>  .../util/intel-pt-decoder/intel-pt-insn-decoder.h  |   65 +
>>  tools/perf/util/intel-pt-decoder/intel-pt-log.c    |  155 ++
>>  tools/perf/util/intel-pt-decoder/intel-pt-log.h    |   52 +
>>  .../util/intel-pt-decoder/intel-pt-pkt-decoder.c   |  400 +++++
>>  .../util/intel-pt-decoder/intel-pt-pkt-decoder.h   |   64 +
>>  tools/perf/util/intel-pt.c                         | 1889 ++++++++++++++++++++
>>  tools/perf/util/intel-pt.h                         |   51 +
>>  tools/perf/util/pmu.c                              |    4 -
>>  28 files changed, 7655 insertions(+), 7 deletions(-)
>>  create mode 100644 tools/perf/Documentation/intel-bts.txt
>>  create mode 100644 tools/perf/Documentation/intel-pt.txt
>>  create mode 100644 tools/perf/arch/x86/util/auxtrace.c
>>  create mode 100644 tools/perf/arch/x86/util/intel-bts.c
>>  create mode 100644 tools/perf/arch/x86/util/intel-pt.c
>>  create mode 100644 tools/perf/arch/x86/util/pmu.c
>>  create mode 100644 tools/perf/util/intel-bts.c
>>  create mode 100644 tools/perf/util/intel-bts.h
>>  create mode 100644 tools/perf/util/intel-pt-decoder/Build
>>  create mode 100644 tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
>>  create mode 100644 tools/perf/util/intel-pt-decoder/intel-pt-decoder.h
>>  create mode 100644 tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
>>  create mode 100644 tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.h
>>  create mode 100644 tools/perf/util/intel-pt-decoder/intel-pt-log.c
>>  create mode 100644 tools/perf/util/intel-pt-decoder/intel-pt-log.h
>>  create mode 100644 tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
>>  create mode 100644 tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.h
>>  create mode 100644 tools/perf/util/intel-pt.c
>>  create mode 100644 tools/perf/util/intel-pt.h
> 
> Yeah, so I did a 'newbie test':
> 
> I pulled the tree and saw that it has a tools/perf/Documentation/intel-bts.txt 
> file and started reading it.
> 
> Based on its text:
> 
>   The Intel BTS kernel driver creates a new PMU for Intel BTS.  The perf record
>   option is:
> 
>         -e intel_bts//
> 
>   Currently Intel BTS is limited to per-thread tracing so the --per-thread option
>   is also needed.
> 
> I tried the following command which failed:
> 
>   triton:~/tip> perf record -e intel_bts// --per-thread sleep 1
>   invalid or unsupported event: 'intel_bts//'
>   Run 'perf list' for a list of valid events
> 
>    usage: perf record [<options>] [<command>]
>       or: perf record [<options>] -- <command> [<options>]
> 
>       -e, --event <event>   event selector. use 'perf list' to list available events
> 
> That's a really ... unhelpful message. If I typoed something I want to know that. 
> If the kernel does not support something, I want to know about that too. Tooling 
> telling me: "maybe you typoed something, maybe it's not supported, I really don't 
> care" is not very productive.

That is not entirely true. The message says "Run 'perf list' for a list of
valid events" which will tell you if the event is valid. So you can tell the
difference between a typo and unsupported event.

> 
> So this was with a distro kernel, and in the hope that I'm missing some magic new 
> kernel feature, I tried it the latest -tip kernel, but it still gives me the same 
> failure.
> 
> So the test newbie user got stuck after wasting some time.
> 
> Me as a kernel developer could probably figure it out, but that's not the point: 
> if newbies cannot discover and use our new features then it's as if they didn't 
> exist, and I'm not pulling non-existent features! ;-)
> 
> Could we please improve all this?

'perf list' shows the event wasn't supported, so I am not sure what more the
"newbie" could expect.  Do you have any suggestions?