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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 30 Jun 2015 12:56:48 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Adrian Hunter <adrian.hunter@...el.com>
Cc:	Arnaldo Carvalho de Melo <acme@...nel.org>,
	linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...hat.com>,
	David Ahern <dsahern@...il.com>,
	Namhyung Kim <namhyung@...nel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Stephane Eranian <eranian@...gle.com>,
	Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [GIT PULL 0/8] perf/pt -> Intel PT/BTS


* Adrian Hunter <adrian.hunter@...el.com> wrote:

> > 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.

Yeah, but my point is: why doesn't the tool do this disambiguation for me? Tools 
are hard enough to use as-is already, no need to put artificial roadblocks in the 
path of first time users.

> > 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?

So I think a first time user would expect a clear message from the computer: what 
was wrong with what he wrote and what should he do to fix it.

Btw., here's the 'perf list' output from a system running the latest -tip kernel:

  vega:~> uname -a
  Linux vega 4.1.0-02935-g390ad45394a3-dirty #567 SMP PREEMPT Mon Jun 29 11:44:48 CEST 2015 x86_64 x86_64 x86_64 GNU/Linux
  vega:~> perf list | grep -i bts
  vega:~> 

so is there any kernel feature dependency? It's unclear. If yes, it should be 
mentioned in the document, and in the tooling output as well. If not then we have 
a bug somewhere.

I.e. you need to smooth the first time user's rocky path to first use as much as 
technically possible. Every single such helping step will literally double the 
number of users who will be able to successfully make use of the new feature.

As a positive example take a look at the newbie's road to 'perf trace':

  vega:~> trace
  Error:  No permissions to read /sys/kernel/debug/tracing/events/raw_syscalls/sys_(enter|exit)
  Hint:   Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'

Aha, useful message, I need to run this as root:

  # trace

     0.000 ( 0.000 ms): sleep/28926  ... [continued]: nanosleep()) = 0
     0.051 ( 0.007 ms): sleep/28926 close(fd: 1                                                           ) = 0
     0.063 ( 0.005 ms): sleep/28926 close(fd: 2                                                           ) = 0
     0.072 ( 0.000 ms): sleep/28926 exit_group(                                      

Ok?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/