linux-kernel - Re: [PATCH -tip v2 2/6] perf bts: Introduce new sub command 'perf bts trace'

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, 21 Dec 2010 19:31:34 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Akihiro Nagai <akihiro.nagai.hw@...achi.com>
Cc:	Arnaldo Carvalho de Melo <acme@...radead.org>,
	Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <peterz@...radead.org>,
	linux-kernel@...r.kernel.org,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	2nddept-manager@....hitachi.co.jp,
	Paul Mackerras <paulus@...ba.org>
Subject: Re: [PATCH -tip v2 2/6] perf bts: Introduce new sub command 'perf
 bts trace'

eOn Tue, Dec 21, 2010 at 06:05:49PM +0900, Akihiro Nagai wrote:
> Introduce new sub command 'perf bts trace'.
> This command can parse and print bts log recorded by
> 'perf bts record'.
> 
> Usage:
>  - First, record the bts log 'perf bts record <command>'
>  - Second, parse and print bts log 'perf bts trace'
> 
> Output:
> 0xffffffff8146fe0e => 0x0000003806200b20
> 0x0000003806200b23 => 0x0000003806204910
> 0xffffffff8146fe0e => 0x0000003806204910
> 0xffffffff8146fe0e => 0x0000003806204936
> 0xffffffff8146fe0e => 0x000000380620493d
> 0x0000003806204981 => 0x00000038062049a3
> 0x00000038062049a7 => 0x0000003806204988
> ...
> 
> Changes in V2:
>  - Update to the latest -tip tree
> 
> Signed-off-by: Akihiro Nagai <akihiro.nagai.hw@...achi.com>
> Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Frederic Weisbecker <fweisbec@...il.com>
> Cc: Paul Mackerras <paulus@...ba.org>
> Cc: Ingo Molnar <mingo@...e.hu>
> Cc: Arnaldo Carvalho de Melo <acme@...radead.org>
> ---
> 
>  tools/perf/Documentation/perf-bts.txt |   14 ++++++--
>  tools/perf/builtin-bts.c              |   56 +++++++++++++++++++++++++++++++--
>  2 files changed, 64 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-bts.txt b/tools/perf/Documentation/perf-bts.txt
> index 55a2fe6..5920dcc 100644
> --- a/tools/perf/Documentation/perf-bts.txt
> +++ b/tools/perf/Documentation/perf-bts.txt
> @@ -3,22 +3,30 @@ perf-bts(1)
>  
>  NAME
>  ----
> -perf-bts - Record branch-trace-store log
> +perf-bts - Record and print branch-trace-store log
>  
>  SYNOPSIS
>  --------
>  [verse]
> -'perf bts' record <command>
> +'perf bts' [<options>] {record|trace}
>  
>  DESCRIPTION
>  -----------
> -This command can record branch-trace-store log.
> +This command can record and print branch-trace-store log.
>  Branch-trace-store is a facility of processors. It can record
>  address of branch to/from on every branch instruction and interrupt.
>  
>  'perf bts record <command>' records branch-trace-store log while specified
>  command is executing. And, save to the file "perf.data".
>  
> +'perf bts trace' parses recorded branch-trace-store log and prints it.
> +
> +OPTIONS
> +-------
> +-i::
> +--input=::
> +        Specify input file name to analyze.
> +
>  SEE ALSO
>  --------
>  linkperf:perf-record[1]
> diff --git a/tools/perf/builtin-bts.c b/tools/perf/builtin-bts.c
> index 587cfad..0d546d8 100644
> --- a/tools/perf/builtin-bts.c
> +++ b/tools/perf/builtin-bts.c
> @@ -1,10 +1,26 @@
>  #include "builtin.h"
>  #include "perf.h"
>  #include "util/parse-options.h"
> +#include "util/session.h"
> +#include "util/cache.h"
> +#include "util/trace-event.h"
> +#include <inttypes.h>
> +
> +/* format string of specifying min width to print address */
> +#if __WORDSIZE == 32
> +#define FMT_ADDR_WIDTH	"10"		/* length of "0x" + 32bit address */
> +#else
> +#define FMT_ADDR_WIDTH	"18"		/* length of "0x" + 64bit address */
> +#endif
> +/* format string to print address */
> +#define FMT_ADDR	"%#0" FMT_ADDR_WIDTH "llx"
> +
> +/* default input file name to analyze */
> +static const char *input_name = "perf.data";
>  
>  static const char * const bts_usage[] = {
> -	"perf bts record <command>",
> -	NULL,
> +	"perf bts [<options>] {record|trace}",
> +	NULL
>  };
>  
>  /* arguments to call 'perf record' */
> @@ -16,11 +32,41 @@ static const char * const record_args[] = {
>  	"-d",
>  };
>  
> -/* dummy struct option to call parse_options() */
>  static const struct option bts_options[] = {
> +	OPT_STRING('i', "input", &input_name, "file", "input file name"),
>  	OPT_END()
>  };
>  
> +static int process_sample_event(event_t *event __unused,
> +	struct sample_data *sample, struct perf_session *session __unused)
> +{
> +	/* sample->ip is 'from address', sample->addr is 'to address' */
> +	printf(FMT_ADDR " => " FMT_ADDR "\n", sample->ip, sample->addr);

It seems this unconditionally prints out the event, but a sample
event can be about anything. If you recorded only branches it's fine,
but if there were other events this will mess up.

Well, when you launch the tool you can iterate into the session->header.attr
and check if there is something else than a branch perf event. And then emit
a warning if so.

That doesn't solve the problem but the user will know there is one.

Actually the best would be to select PERF_SAMPLE_ID in the sample_type
on record and also PERF_FORMAT_ID in the read_format.

Then you can find the PERF_SAMPLE_ID that matches your event. If we
record that in the perf headers we can retrieve which events id are the
branch ones.

But well, that's a secondary problem for now.
--
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/