linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type
@ 2014-02-03 11:44 Jiri Olsa
  2014-02-03 11:44 ` [PATCH 2/3] perf tools: Add call-graph option support into .perfconfig Jiri Olsa
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Jiri Olsa @ 2014-02-03 11:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Corey Ashford, David Ahern, Ingo Molnar, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo,
	Andi Kleen, Adrian Hunter

We use PERF_SAMPLE_PERIOD sample type only for frequency
setup -F (default) option. The -c does not need store period,
because it's always the same.

In -c case the report code uses '1' as  period. Fixing
it to perf_event_attr::sample_period.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evsel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 55407c5..c6f8ce9 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1220,7 +1220,7 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 	memset(data, 0, sizeof(*data));
 	data->cpu = data->pid = data->tid = -1;
 	data->stream_id = data->id = data->time = -1ULL;
-	data->period = 1;
+	data->period = evsel->attr.sample_period;
 	data->weight = 0;
 
 	if (event->header.type != PERF_RECORD_SAMPLE) {
-- 
1.8.3.1


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

* [PATCH 2/3] perf tools: Add call-graph option support into .perfconfig
  2014-02-03 11:44 [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type Jiri Olsa
@ 2014-02-03 11:44 ` Jiri Olsa
  2014-02-22 17:56   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2014-02-03 11:44 ` [PATCH 3/3] perf tools: Add readable output for callchain debug Jiri Olsa
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Jiri Olsa @ 2014-02-03 11:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Corey Ashford, David Ahern, Ingo Molnar, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo,
	Andi Kleen, Adrian Hunter

Adding call-graph option support into .perfconfig file,
so it's now possible use call-graph option like:

  [top]
        call-graph = fp

  [record]
        call-graph = dwarf,8192

Above options ONLY setup the unwind method. To enable
perf record/top to actually use it the command line
option -g/-G must be specified.

The --call-graph option overloads .perfconfig setup.

Assuming above configuration:

  $ perf record -g ls
  - enables dwarf unwind with user stack size dump 8192 bytes

  $ perf top -G
  - enables frame pointer unwind

  $ perf record --call-graph=fp ls
  - enables frame pointer unwind

  $ perf top --call-graph=dwarf,4096 ls
  - enables dwarf unwind with user stack size dump 4096 bytes

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-record.c | 16 ++++++++++++++++
 tools/perf/builtin-top.c    | 12 ++++++++++++
 tools/perf/perf.h           |  1 +
 tools/perf/util/evsel.c     |  2 +-
 4 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3c394bf..3f7ec8a 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -755,6 +755,8 @@ int record_parse_callchain_opt(const struct option *opt,
 	struct record_opts *opts = opt->value;
 	int ret;
 
+	opts->call_graph_enabled = !unset;
+
 	/* --no-call-graph */
 	if (unset) {
 		opts->call_graph = CALLCHAIN_NONE;
@@ -775,6 +777,8 @@ int record_callchain_opt(const struct option *opt,
 {
 	struct record_opts *opts = opt->value;
 
+	opts->call_graph_enabled = !unset;
+
 	if (opts->call_graph == CALLCHAIN_NONE)
 		opts->call_graph = CALLCHAIN_FP;
 
@@ -782,6 +786,16 @@ int record_callchain_opt(const struct option *opt,
 	return 0;
 }
 
+static int perf_record_config(const char *var, const char *value, void *cb)
+{
+	struct record *rec = cb;
+
+	if (!strcmp(var, "record.call-graph"))
+		return record_parse_callchain(value, &rec->opts);
+
+	return perf_default_config(var, value, cb);
+}
+
 static const char * const record_usage[] = {
 	"perf record [<options>] [<command>]",
 	"perf record [<options>] -- <command> [<options>]",
@@ -913,6 +927,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (rec->evlist == NULL)
 		return -ENOMEM;
 
+	perf_config(perf_record_config, rec);
+
 	argc = parse_options(argc, argv, record_options, record_usage,
 			    PARSE_OPT_STOP_AT_NON_OPTION);
 	if (!argc && target__none(&rec->opts.target))
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 76cd510..ed99ec4 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -991,6 +991,16 @@ parse_callchain_opt(const struct option *opt, const char *arg, int unset)
 	return record_parse_callchain_opt(opt, arg, unset);
 }
 
+static int perf_top_config(const char *var, const char *value, void *cb)
+{
+	struct perf_top *top = cb;
+
+	if (!strcmp(var, "top.call-graph"))
+		return record_parse_callchain(value, &top->record_opts);
+
+	return perf_default_config(var, value, cb);
+}
+
 static int
 parse_percent_limit(const struct option *opt, const char *arg,
 		    int unset __maybe_unused)
@@ -1115,6 +1125,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (top.evlist == NULL)
 		return -ENOMEM;
 
+	perf_config(perf_top_config, &top);
+
 	argc = parse_options(argc, argv, options, top_usage, 0);
 	if (argc)
 		usage_with_options(top_usage, options);
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 7daa806..6dabc15 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -257,6 +257,7 @@ enum perf_call_graph_mode {
 struct record_opts {
 	struct target target;
 	int	     call_graph;
+	bool         call_graph_enabled;
 	bool	     group;
 	bool	     inherit_stat;
 	bool	     no_buffering;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index c6f8ce9..8201abe 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -595,7 +595,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 		attr->mmap_data = track;
 	}
 
-	if (opts->call_graph) {
+	if (opts->call_graph_enabled) {
 		perf_evsel__set_sample_bit(evsel, CALLCHAIN);
 
 		if (opts->call_graph == CALLCHAIN_DWARF) {
-- 
1.8.3.1


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

* [PATCH 3/3] perf tools: Add readable output for callchain debug
  2014-02-03 11:44 [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type Jiri Olsa
  2014-02-03 11:44 ` [PATCH 2/3] perf tools: Add call-graph option support into .perfconfig Jiri Olsa
@ 2014-02-03 11:44 ` Jiri Olsa
  2014-02-03 15:09   ` Arnaldo Carvalho de Melo
  2014-02-22 17:56   ` [tip:perf/core] perf record: " tip-bot for Jiri Olsa
  2014-02-05  1:27 ` [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type Namhyung Kim
  2014-02-22 17:56 ` [tip:perf/core] " tip-bot for Jiri Olsa
  3 siblings, 2 replies; 11+ messages in thread
From: Jiri Olsa @ 2014-02-03 11:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Corey Ashford, David Ahern, Ingo Molnar, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo,
	Andi Kleen, Adrian Hunter

Adding people readable output for callchain debug,
to get following '-v' output:

  $ perf record -v -g ls
  callchain: type DWARF
  callchain: stack dump size 4096
  ...

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-record.c | 4 +++-
 tools/perf/perf.h           | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3f7ec8a..679758f 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -741,7 +741,9 @@ int record_parse_callchain(const char *arg, struct record_opts *opts)
 
 static void callchain_debug(struct record_opts *opts)
 {
-	pr_debug("callchain: type %d\n", opts->call_graph);
+	static const char *str[CALLCHAIN_MAX] = { "NONE", "FP", "DWARF" };
+
+	pr_debug("callchain: type %s\n", str[opts->call_graph]);
 
 	if (opts->call_graph == CALLCHAIN_DWARF)
 		pr_debug("callchain: stack dump size %d\n",
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 6dabc15..c30e1ae 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -251,7 +251,8 @@ void pthread__unblock_sigwinch(void);
 enum perf_call_graph_mode {
 	CALLCHAIN_NONE,
 	CALLCHAIN_FP,
-	CALLCHAIN_DWARF
+	CALLCHAIN_DWARF,
+	CALLCHAIN_MAX
 };
 
 struct record_opts {
-- 
1.8.3.1


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

* Re: [PATCH 3/3] perf tools: Add readable output for callchain debug
  2014-02-03 11:44 ` [PATCH 3/3] perf tools: Add readable output for callchain debug Jiri Olsa
@ 2014-02-03 15:09   ` Arnaldo Carvalho de Melo
  2014-02-03 16:39     ` Jiri Olsa
  2014-02-22 17:56   ` [tip:perf/core] perf record: " tip-bot for Jiri Olsa
  1 sibling, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-02-03 15:09 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Corey Ashford, David Ahern, Ingo Molnar,
	Namhyung Kim, Paul Mackerras, Peter Zijlstra, Andi Kleen,
	Adrian Hunter

Em Mon, Feb 03, 2014 at 12:44:43PM +0100, Jiri Olsa escreveu:
> Adding people readable output for callchain debug,
> to get following '-v' output:
> 
>   $ perf record -v -g ls
>   callchain: type DWARF
>   callchain: stack dump size 4096
>   ...

Applied, but then I tried:

[acme@zoo linux]$ perf evlist -v
cycles: sample_freq=4000, size: 96, sample_type:
IP|TID|TIME|CALLCHAIN|PERIOD, disabled: 1, inherit: 1, mmap: 1, comm: 1,
freq: 1, enable_on_exec: 1, sample_id_all: 1, exclude_guest: 1
[acme@zoo linux]$ 


I.e. using the tool that provides info about 'perf.data' files, and
there I couldn't figure it out which kind of callchains was used, can
you fix it there too?

- Arnaldo
 
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/builtin-record.c | 4 +++-
>  tools/perf/perf.h           | 3 ++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 3f7ec8a..679758f 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -741,7 +741,9 @@ int record_parse_callchain(const char *arg, struct record_opts *opts)
>  
>  static void callchain_debug(struct record_opts *opts)
>  {
> -	pr_debug("callchain: type %d\n", opts->call_graph);
> +	static const char *str[CALLCHAIN_MAX] = { "NONE", "FP", "DWARF" };
> +
> +	pr_debug("callchain: type %s\n", str[opts->call_graph]);
>  
>  	if (opts->call_graph == CALLCHAIN_DWARF)
>  		pr_debug("callchain: stack dump size %d\n",
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index 6dabc15..c30e1ae 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -251,7 +251,8 @@ void pthread__unblock_sigwinch(void);
>  enum perf_call_graph_mode {
>  	CALLCHAIN_NONE,
>  	CALLCHAIN_FP,
> -	CALLCHAIN_DWARF
> +	CALLCHAIN_DWARF,
> +	CALLCHAIN_MAX
>  };
>  
>  struct record_opts {
> -- 
> 1.8.3.1

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

* Re: [PATCH 3/3] perf tools: Add readable output for callchain debug
  2014-02-03 15:09   ` Arnaldo Carvalho de Melo
@ 2014-02-03 16:39     ` Jiri Olsa
  0 siblings, 0 replies; 11+ messages in thread
From: Jiri Olsa @ 2014-02-03 16:39 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Corey Ashford, David Ahern, Ingo Molnar,
	Namhyung Kim, Paul Mackerras, Peter Zijlstra, Andi Kleen,
	Adrian Hunter

On Mon, Feb 03, 2014 at 01:09:15PM -0200, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 03, 2014 at 12:44:43PM +0100, Jiri Olsa escreveu:
> > Adding people readable output for callchain debug,
> > to get following '-v' output:
> > 
> >   $ perf record -v -g ls
> >   callchain: type DWARF
> >   callchain: stack dump size 4096
> >   ...
> 
> Applied, but then I tried:
> 
> [acme@zoo linux]$ perf evlist -v
> cycles: sample_freq=4000, size: 96, sample_type:
> IP|TID|TIME|CALLCHAIN|PERIOD, disabled: 1, inherit: 1, mmap: 1, comm: 1,
> freq: 1, enable_on_exec: 1, sample_id_all: 1, exclude_guest: 1
> [acme@zoo linux]$ 
> 
> 
> I.e. using the tool that provides info about 'perf.data' files, and
> there I couldn't figure it out which kind of callchains was used, can
> you fix it there too?

todo updated ;-)

jirka

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

* Re: [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type
  2014-02-03 11:44 [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type Jiri Olsa
  2014-02-03 11:44 ` [PATCH 2/3] perf tools: Add call-graph option support into .perfconfig Jiri Olsa
  2014-02-03 11:44 ` [PATCH 3/3] perf tools: Add readable output for callchain debug Jiri Olsa
@ 2014-02-05  1:27 ` Namhyung Kim
  2014-02-05 14:33   ` Jiri Olsa
  2014-02-22 17:56 ` [tip:perf/core] " tip-bot for Jiri Olsa
  3 siblings, 1 reply; 11+ messages in thread
From: Namhyung Kim @ 2014-02-05  1:27 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Corey Ashford, David Ahern, Ingo Molnar,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo,
	Andi Kleen, Adrian Hunter

Hi Jiri,

On Mon,  3 Feb 2014 12:44:41 +0100, Jiri Olsa wrote:
> We use PERF_SAMPLE_PERIOD sample type only for frequency
> setup -F (default) option. The -c does not need store period,
> because it's always the same.
>
> In -c case the report code uses '1' as  period. Fixing
> it to perf_event_attr::sample_period.

All 3 patches look good.  But I found something strange.  When we
setup/config evsel attrs following code is used:

 util/evsel.c::perf_evsel__config()

	/*
	 * We default some events to a 1 default interval. But keep
	 * it a weak assumption overridable by the user.
	 */
	if (!attr->sample_period || (opts->user_freq != UINT_MAX &&
				     opts->user_interval != ULLONG_MAX)) {
		if (opts->freq) {
			perf_evsel__set_sample_bit(evsel, PERIOD);
			attr->freq		= 1;
			attr->sample_freq	= opts->freq;
		} else {
			attr->sample_period = opts->default_interval;
		}
	}


But shouldn't it be "||" instead of "&&" for checking
user_freq/interval?  The opts->freq was setup by code below

  util/record.c::record_opts__config_freq()

	bool user_freq = opts->user_freq != UINT_MAX;
	unsigned int max_rate;

	if (opts->user_interval != ULLONG_MAX)
		opts->default_interval = opts->user_interval;
	if (user_freq)
		opts->freq = opts->user_freq;

	/*
	 * User specified count overrides default frequency.
	 */
	if (opts->default_interval)
		opts->freq = 0;
	else if (opts->freq) {
		opts->default_interval = opts->freq;
	} else {
		pr_err("frequency and count are zero, aborting\n");
		return -1;
	}


Thanks,
Namhyung

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

* Re: [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type
  2014-02-05  1:27 ` [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type Namhyung Kim
@ 2014-02-05 14:33   ` Jiri Olsa
  2014-02-06  6:24     ` Namhyung Kim
  0 siblings, 1 reply; 11+ messages in thread
From: Jiri Olsa @ 2014-02-05 14:33 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: linux-kernel, Corey Ashford, David Ahern, Ingo Molnar,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo,
	Andi Kleen, Adrian Hunter

On Wed, Feb 05, 2014 at 10:27:30AM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Mon,  3 Feb 2014 12:44:41 +0100, Jiri Olsa wrote:
> > We use PERF_SAMPLE_PERIOD sample type only for frequency
> > setup -F (default) option. The -c does not need store period,
> > because it's always the same.
> >
> > In -c case the report code uses '1' as  period. Fixing
> > it to perf_event_attr::sample_period.
> 
> All 3 patches look good.  But I found something strange.  When we
> setup/config evsel attrs following code is used:
> 
>  util/evsel.c::perf_evsel__config()
> 
> 	/*
> 	 * We default some events to a 1 default interval. But keep
> 	 * it a weak assumption overridable by the user.
> 	 */
> 	if (!attr->sample_period || (opts->user_freq != UINT_MAX &&
> 				     opts->user_interval != ULLONG_MAX)) {
> 		if (opts->freq) {
> 			perf_evsel__set_sample_bit(evsel, PERIOD);
> 			attr->freq		= 1;
> 			attr->sample_freq	= opts->freq;
> 		} else {
> 			attr->sample_period = opts->default_interval;
> 		}
> 	}

yes, I think thats right.. we should use || instead of &&

It will allow to change period for event types with predefined
attr->sample_period like tracepoints.

However, I tried with tracepoints and even with this fix
and following command line:

  # perf record -e syscalls:sys_enter_read -c 2 ls

you'll still get samples with period 1. The reason is in
kernel code:

static void perf_swevent_event(struct perf_event *event, u64 nr,
                               struct perf_sample_data *data,
                               struct pt_regs *regs)
{
...
        if ((event->attr.sample_type & PERF_SAMPLE_PERIOD) && !event->attr.freq) {
                data->period = nr;
                return perf_swevent_overflow(event, 1, data, regs);

bacause above condition is true for tracepoints.

It looks like a bug, but I'm not sure how handy it'd be
set period other than 1 for tracepoints thought.. ;)

Maybe it's not that big issue in comparison of screwing
up other software events processing.

I haven't checked other software events.

jirka

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

* Re: [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type
  2014-02-05 14:33   ` Jiri Olsa
@ 2014-02-06  6:24     ` Namhyung Kim
  0 siblings, 0 replies; 11+ messages in thread
From: Namhyung Kim @ 2014-02-06  6:24 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Corey Ashford, David Ahern, Ingo Molnar,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo,
	Andi Kleen, Adrian Hunter

Hi Jiri,

On Wed, 5 Feb 2014 15:33:29 +0100, Jiri Olsa wrote:
> On Wed, Feb 05, 2014 at 10:27:30AM +0900, Namhyung Kim wrote:
>> Hi Jiri,
>> 
>> On Mon,  3 Feb 2014 12:44:41 +0100, Jiri Olsa wrote:
>> > We use PERF_SAMPLE_PERIOD sample type only for frequency
>> > setup -F (default) option. The -c does not need store period,
>> > because it's always the same.
>> >
>> > In -c case the report code uses '1' as  period. Fixing
>> > it to perf_event_attr::sample_period.
>> 
>> All 3 patches look good.  But I found something strange.  When we
>> setup/config evsel attrs following code is used:
>> 
>>  util/evsel.c::perf_evsel__config()
>> 
>> 	/*
>> 	 * We default some events to a 1 default interval. But keep
>> 	 * it a weak assumption overridable by the user.
>> 	 */
>> 	if (!attr->sample_period || (opts->user_freq != UINT_MAX &&
>> 				     opts->user_interval != ULLONG_MAX)) {
>> 		if (opts->freq) {
>> 			perf_evsel__set_sample_bit(evsel, PERIOD);
>> 			attr->freq		= 1;
>> 			attr->sample_freq	= opts->freq;
>> 		} else {
>> 			attr->sample_period = opts->default_interval;
>> 		}
>> 	}
>
> yes, I think thats right.. we should use || instead of &&
>
> It will allow to change period for event types with predefined
> attr->sample_period like tracepoints.

Right.  As I read the code, it works "if (!attr->sample_period)" case only.

>
> However, I tried with tracepoints and even with this fix
> and following command line:
>
>   # perf record -e syscalls:sys_enter_read -c 2 ls
>
> you'll still get samples with period 1. The reason is in
> kernel code:
>
> static void perf_swevent_event(struct perf_event *event, u64 nr,
>                                struct perf_sample_data *data,
>                                struct pt_regs *regs)
> {
> ...
>         if ((event->attr.sample_type & PERF_SAMPLE_PERIOD) && !event->attr.freq) {
>                 data->period = nr;
>                 return perf_swevent_overflow(event, 1, data, regs);
>
> bacause above condition is true for tracepoints.
>
> It looks like a bug, but I'm not sure how handy it'd be
> set period other than 1 for tracepoints thought.. ;)

Agreed.  But at least we should support whatever user wants IMHO..

>
> Maybe it's not that big issue in comparison of screwing
> up other software events processing.

:)

Thanks,
Namhyung

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

* [tip:perf/core] perf tools: Put proper period for for samples without PERIOD sample_type
  2014-02-03 11:44 [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type Jiri Olsa
                   ` (2 preceding siblings ...)
  2014-02-05  1:27 ` [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type Namhyung Kim
@ 2014-02-22 17:56 ` tip-bot for Jiri Olsa
  3 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Jiri Olsa @ 2014-02-22 17:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, andi, a.p.zijlstra,
	namhyung, jolsa, adrian.hunter, dsahern, tglx, cjashfor, mingo

Commit-ID:  bc5290869d0a7f7abbde76ac95a7f7b6f5d7bb7b
Gitweb:     http://git.kernel.org/tip/bc5290869d0a7f7abbde76ac95a7f7b6f5d7bb7b
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Mon, 3 Feb 2014 12:44:41 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 18 Feb 2014 09:34:46 -0300

perf tools: Put proper period for for samples without PERIOD sample_type

We use PERF_SAMPLE_PERIOD sample type only for frequency
setup -F (default) option. The -c does not need store period,
because it's always the same.

In -c case the report code uses '1' as  period. Fixing
it to perf_event_attr::sample_period.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1391427883-13443-1-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 55407c5..c6f8ce9 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1220,7 +1220,7 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 	memset(data, 0, sizeof(*data));
 	data->cpu = data->pid = data->tid = -1;
 	data->stream_id = data->id = data->time = -1ULL;
-	data->period = 1;
+	data->period = evsel->attr.sample_period;
 	data->weight = 0;
 
 	if (event->header.type != PERF_RECORD_SAMPLE) {

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

* [tip:perf/core] perf tools: Add call-graph option support into .perfconfig
  2014-02-03 11:44 ` [PATCH 2/3] perf tools: Add call-graph option support into .perfconfig Jiri Olsa
@ 2014-02-22 17:56   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Jiri Olsa @ 2014-02-22 17:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, andi, a.p.zijlstra,
	namhyung, jolsa, adrian.hunter, dsahern, tglx, cjashfor, mingo

Commit-ID:  eb853e80324fa87faf7ae7e1a763ad643f908f2d
Gitweb:     http://git.kernel.org/tip/eb853e80324fa87faf7ae7e1a763ad643f908f2d
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Mon, 3 Feb 2014 12:44:42 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 18 Feb 2014 09:34:47 -0300

perf tools: Add call-graph option support into .perfconfig

Adding call-graph option support into .perfconfig file, so it's now
possible use call-graph option like:

  [top]
        call-graph = fp

  [record]
        call-graph = dwarf,8192

Above options ONLY setup the unwind method. To enable perf record/top to
actually use it the command line option -g/-G must be specified.

The --call-graph option overloads .perfconfig setup.

Assuming above configuration:

  $ perf record -g ls
  - enables dwarf unwind with user stack size dump 8192 bytes

  $ perf top -G
  - enables frame pointer unwind

  $ perf record --call-graph=fp ls
  - enables frame pointer unwind

  $ perf top --call-graph=dwarf,4096 ls
  - enables dwarf unwind with user stack size dump 4096 bytes

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1391427883-13443-2-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 16 ++++++++++++++++
 tools/perf/builtin-top.c    | 12 ++++++++++++
 tools/perf/perf.h           |  1 +
 tools/perf/util/evsel.c     |  2 +-
 4 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index af47531..be9e8bc 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -749,6 +749,8 @@ int record_parse_callchain_opt(const struct option *opt,
 	struct record_opts *opts = opt->value;
 	int ret;
 
+	opts->call_graph_enabled = !unset;
+
 	/* --no-call-graph */
 	if (unset) {
 		opts->call_graph = CALLCHAIN_NONE;
@@ -769,6 +771,8 @@ int record_callchain_opt(const struct option *opt,
 {
 	struct record_opts *opts = opt->value;
 
+	opts->call_graph_enabled = !unset;
+
 	if (opts->call_graph == CALLCHAIN_NONE)
 		opts->call_graph = CALLCHAIN_FP;
 
@@ -776,6 +780,16 @@ int record_callchain_opt(const struct option *opt,
 	return 0;
 }
 
+static int perf_record_config(const char *var, const char *value, void *cb)
+{
+	struct record *rec = cb;
+
+	if (!strcmp(var, "record.call-graph"))
+		return record_parse_callchain(value, &rec->opts);
+
+	return perf_default_config(var, value, cb);
+}
+
 static const char * const record_usage[] = {
 	"perf record [<options>] [<command>]",
 	"perf record [<options>] -- <command> [<options>]",
@@ -907,6 +921,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (rec->evlist == NULL)
 		return -ENOMEM;
 
+	perf_config(perf_record_config, rec);
+
 	argc = parse_options(argc, argv, record_options, record_usage,
 			    PARSE_OPT_STOP_AT_NON_OPTION);
 	if (!argc && target__none(&rec->opts.target))
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 76cd510..ed99ec4 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -991,6 +991,16 @@ parse_callchain_opt(const struct option *opt, const char *arg, int unset)
 	return record_parse_callchain_opt(opt, arg, unset);
 }
 
+static int perf_top_config(const char *var, const char *value, void *cb)
+{
+	struct perf_top *top = cb;
+
+	if (!strcmp(var, "top.call-graph"))
+		return record_parse_callchain(value, &top->record_opts);
+
+	return perf_default_config(var, value, cb);
+}
+
 static int
 parse_percent_limit(const struct option *opt, const char *arg,
 		    int unset __maybe_unused)
@@ -1115,6 +1125,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (top.evlist == NULL)
 		return -ENOMEM;
 
+	perf_config(perf_top_config, &top);
+
 	argc = parse_options(argc, argv, options, top_usage, 0);
 	if (argc)
 		usage_with_options(top_usage, options);
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index e84fa26..2078f33 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -257,6 +257,7 @@ enum perf_call_graph_mode {
 struct record_opts {
 	struct target target;
 	int	     call_graph;
+	bool         call_graph_enabled;
 	bool	     group;
 	bool	     inherit_stat;
 	bool	     no_buffering;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index c6f8ce9..8201abe 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -595,7 +595,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 		attr->mmap_data = track;
 	}
 
-	if (opts->call_graph) {
+	if (opts->call_graph_enabled) {
 		perf_evsel__set_sample_bit(evsel, CALLCHAIN);
 
 		if (opts->call_graph == CALLCHAIN_DWARF) {

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

* [tip:perf/core] perf record: Add readable output for callchain debug
  2014-02-03 11:44 ` [PATCH 3/3] perf tools: Add readable output for callchain debug Jiri Olsa
  2014-02-03 15:09   ` Arnaldo Carvalho de Melo
@ 2014-02-22 17:56   ` tip-bot for Jiri Olsa
  1 sibling, 0 replies; 11+ messages in thread
From: tip-bot for Jiri Olsa @ 2014-02-22 17:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, andi, a.p.zijlstra,
	namhyung, jolsa, adrian.hunter, dsahern, tglx, cjashfor, mingo

Commit-ID:  a601fdff1af20ea0208e918f5e97a247a3c37a40
Gitweb:     http://git.kernel.org/tip/a601fdff1af20ea0208e918f5e97a247a3c37a40
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Mon, 3 Feb 2014 12:44:43 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 18 Feb 2014 09:34:47 -0300

perf record: Add readable output for callchain debug

Adding people readable output for callchain debug, to get following '-v'
output:

  $ perf record -v -g ls
  callchain: type DWARF
  callchain: stack dump size 4096
  ...

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1391427883-13443-3-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 4 +++-
 tools/perf/perf.h           | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index be9e8bc..7b8f0e6 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -735,7 +735,9 @@ int record_parse_callchain(const char *arg, struct record_opts *opts)
 
 static void callchain_debug(struct record_opts *opts)
 {
-	pr_debug("callchain: type %d\n", opts->call_graph);
+	static const char *str[CALLCHAIN_MAX] = { "NONE", "FP", "DWARF" };
+
+	pr_debug("callchain: type %s\n", str[opts->call_graph]);
 
 	if (opts->call_graph == CALLCHAIN_DWARF)
 		pr_debug("callchain: stack dump size %d\n",
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 2078f33..6898ad0 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -251,7 +251,8 @@ void pthread__unblock_sigwinch(void);
 enum perf_call_graph_mode {
 	CALLCHAIN_NONE,
 	CALLCHAIN_FP,
-	CALLCHAIN_DWARF
+	CALLCHAIN_DWARF,
+	CALLCHAIN_MAX
 };
 
 struct record_opts {

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

end of thread, other threads:[~2014-02-22 17:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-03 11:44 [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type Jiri Olsa
2014-02-03 11:44 ` [PATCH 2/3] perf tools: Add call-graph option support into .perfconfig Jiri Olsa
2014-02-22 17:56   ` [tip:perf/core] " tip-bot for Jiri Olsa
2014-02-03 11:44 ` [PATCH 3/3] perf tools: Add readable output for callchain debug Jiri Olsa
2014-02-03 15:09   ` Arnaldo Carvalho de Melo
2014-02-03 16:39     ` Jiri Olsa
2014-02-22 17:56   ` [tip:perf/core] perf record: " tip-bot for Jiri Olsa
2014-02-05  1:27 ` [PATCH 1/3] perf tools: Put proper period for for samples without PERIOD sample_type Namhyung Kim
2014-02-05 14:33   ` Jiri Olsa
2014-02-06  6:24     ` Namhyung Kim
2014-02-22 17:56 ` [tip:perf/core] " tip-bot for 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).