Skip to contentSkip to conversation

perf: PEBS/period freerunning fixes

PATCH In review Jiri Olsa 5 participants 25 messages Feb 1, 2018 – Feb 17, 2018

Subsystems (2)
lore
Share & tools
lore

Jiri Olsa

author reviewer
#0 lore raw

cover perf: PEBS/period freerunning fixes

hi,
Stephan reported that we don't support period for enabling large PEBS data, which there's no reason for. Sending fix for that plus related perf tool period fixes Stephan reported.

thanks,
jirka

---
Jiri Olsa (3):
perf tools: Fix period/freq terms setup
perf record: Fix period option handling x86/events/intel/ds: Add PERF_SAMPLE_PERIOD into PEBS_FREERUNNING_FLAGS

 arch/x86/events/perf_event.h |  3 ++-
 tools/perf/builtin-record.c  |  3 ++-
 tools/perf/perf.h            |  1 +
 tools/perf/util/evsel.c      | 13 ++++++++++---
 4 files changed, 15 insertions(+), 5 deletions(-)
author reviewer
#1 lore raw

PATCH 1/3 perf tools: Fix period/freq terms setup

Stephane reported that we don't set properly PERIOD sample type for events with period term defined.

Before:

  $ perf record -e cpu/cpu-cycles,period=1000/u ls
  $ perf evlist -v
  cpu/cpu-cycles,period=1000/u: ... sample_type: IP|TID|TIME|PERIOD, ...

After:

  $ perf record -e cpu/cpu-cycles,period=1000/u ls
  $ perf evlist -v
  cpu/cpu-cycles,period=1000/u: ... sample_type: IP|TID|TIME, ...

Setting PERIOD sample type based on period term setup.

---
 tools/perf/util/evsel.c | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/util/evsel.c +2 -0
index 66fa45198a11..f2f2eaafde6d 100644
@@ -745,12 +745,14 @@ static void apply_config_terms(struct perf_evsel *evsel,
 			if (!(term->weak && opts->user_interval != ULLONG_MAX)) {
 				attr->sample_period = term->val.period;
 				attr->freq = 0;
+				perf_evsel__reset_sample_bit(evsel, PERIOD);
 			}
 			break;
 		case PERF_EVSEL__CONFIG_TERM_FREQ:
 			if (!(term->weak && opts->user_freq != UINT_MAX)) {
 				attr->sample_freq = term->val.freq;
 				attr->freq = 1;
+				perf_evsel__set_sample_bit(evsel, PERIOD);
 			}
 			break;
 		case PERF_EVSEL__CONFIG_TERM_TIME:
Signature · 1 line
2.13.6
#2 lore raw

Re: [PATCH 1/3] perf tools: Fix period/freq terms setup

Jiri,

Quoted 8 lines · Jiri Olsa

Stephane reported that we don't set properly PERIOD sample type for events with period term defined.

Before:

  $ perf record -e cpu/cpu-cycles,period=1000/u ls
  $ perf evlist -v
  cpu/cpu-cycles,period=1000/u: ... sample_type: IP|TID|TIME|PERIOD, ...
Quoted 7 lines

After:

  $ perf record -e cpu/cpu-cycles,period=1000/u ls
  $ perf evlist -v
  cpu/cpu-cycles,period=1000/u: ... sample_type: IP|TID|TIME, ...

Setting PERIOD sample type based on period term setup.

there is still one problem remaining here. It has to do with the handling of cycles:pp or :p or :ppp. Suppose I want to set a period for it while I am also sampling on other events: Something like:

$ perf record -e
cycles:pp,instructions,cpu/event=0xd0,umaks=0x81,period=100000/ .....

I want to set the period for cycles:pp, but not for instructions. I cannot use -c because
it would also force a period on instructions. I could use the raw hw raw event code for cycles:pp.
But that does not work because recent kernels prevent use of hw filters on events programmed
for PEBS, e.g., cpu/event=0xc2,umask=0x1,cmask=16,inv/pp is rejected. PEBS does not support filters.
It works in the case of cycles:pp simply by the nature on the underlying event and the stalls.

To get precise cycles, the only event syntax you can use is cycles:pp, but then you cannot specify
an event-specific period. This needs to be fixed as well.

I'd like to be able to say:

$ perf record -e
cycles:pp:period=10000001,cpu/event=0xd0,umaks=0x81,period=100000/

Or something equivalent.

Otherwise, I tested what you have written so far and it works.

Thanks.

Quoted 29 lines
---
 tools/perf/util/evsel.c | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/util/evsel.c +2 -1
index 66fa45198a11..f2f2eaafde6d 100644
@@ -745,12 +745,14 @@ static void apply_config_terms(struct perf_evsel *evsel,
                        if (!(term->weak && opts->user_interval != ULLONG_MAX)) {
                                attr->sample_period = term->val.period;
                                attr->freq = 0;
+                               perf_evsel__reset_sample_bit(evsel, PERIOD);
                        }
                        break;
                case PERF_EVSEL__CONFIG_TERM_FREQ:
                        if (!(term->weak && opts->user_freq != UINT_MAX)) {
                                attr->sample_freq = term->val.freq;
                                attr->freq = 1;
+                               perf_evsel__set_sample_bit(evsel, PERIOD);
                        }
                        break;
                case PERF_EVSEL__CONFIG_TERM_TIME:
--

2.13.6

#3 lore raw

Re: [PATCH 1/3] perf tools: Fix period/freq terms setup

Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:

Quoted 40 lines

Jiri,

Quoted 8 lines · Jiri Olsa

Stephane reported that we don't set properly PERIOD sample type for events with period term defined.

Before:

  $ perf record -e cpu/cpu-cycles,period=1000/u ls
  $ perf evlist -v
  cpu/cpu-cycles,period=1000/u: ... sample_type: IP|TID|TIME|PERIOD, ...
Quoted 7 lines

After:

  $ perf record -e cpu/cpu-cycles,period=1000/u ls
  $ perf evlist -v
  cpu/cpu-cycles,period=1000/u: ... sample_type: IP|TID|TIME, ...

Setting PERIOD sample type based on period term setup.

there is still one problem remaining here. It has to do with the handling of cycles:pp or :p or :ppp. Suppose I want to set a period for it while I am also sampling on other events: Something like:

$ perf record -e
cycles:pp,instructions,cpu/event=0xd0,umaks=0x81,period=100000/ .....

I want to set the period for cycles:pp, but not for instructions. I cannot use -c because
it would also force a period on instructions. I could use the raw hw raw event code for cycles:pp.
But that does not work because recent kernels prevent use of hw filters on events programmed
for PEBS, e.g., cpu/event=0xc2,umask=0x1,cmask=16,inv/pp is rejected. PEBS does not support filters.
It works in the case of cycles:pp simply by the nature on the underlying event and the stalls.

To get precise cycles, the only event syntax you can use is cycles:pp, but then you cannot specify
an event-specific period. This needs to be fixed as well.

Quoted 8 lines

I'd like to be able to say:

$ perf record -e
cycles:pp:period=10000001,cpu/event=0xd0,umaks=0x81,period=100000/

Or something equivalent.

Otherwise, I tested what you have written so far and it works.

So I take that as a Tested-by: Stephane and will apply the patches, Jiri can continue working on these other aspects, right?

- Arnaldo

Quoted 32 lines

Thanks.

Quoted 29 lines
---
 tools/perf/util/evsel.c | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/util/evsel.c +2 -1
index 66fa45198a11..f2f2eaafde6d 100644
@@ -745,12 +745,14 @@ static void apply_config_terms(struct perf_evsel *evsel,
                        if (!(term->weak && opts->user_interval != ULLONG_MAX)) {
                                attr->sample_period = term->val.period;
                                attr->freq = 0;
+                               perf_evsel__reset_sample_bit(evsel, PERIOD);
                        }
                        break;
                case PERF_EVSEL__CONFIG_TERM_FREQ:
                        if (!(term->weak && opts->user_freq != UINT_MAX)) {
                                attr->sample_freq = term->val.freq;
                                attr->freq = 1;
+                               perf_evsel__set_sample_bit(evsel, PERIOD);
                        }
                        break;
                case PERF_EVSEL__CONFIG_TERM_TIME:
--

2.13.6

#4 lore raw

Re: [PATCH 1/3] perf tools: Fix period/freq terms setup

Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 5 lines

Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:

Quoted 1 line

Otherwise, I tested what you have written so far and it works.

So I take that as a Tested-by: Stephane and will apply the patches, Jiri can continue working on these other aspects, right?

I also added this for the casual reader to get up to speed more quickly, please check that it makes sense.

Committer note:

When we use -c or a period=N term in the event definition, then we don't need to ask the kernel, via perf_event_attr.sample_type |= PERF_SAMPLE_PERIOD, to put the event period in each sample, as we know it already, it is in perf_event_attr.sample_period.

- Arnaldo

#5 lore raw

Re: [PATCH 1/3] perf tools: Fix period/freq terms setup

Quoted 10 lines · Arnaldo Carvalho de Melo

Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 5 lines

Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:

Quoted 1 line

Otherwise, I tested what you have written so far and it works.

So I take that as a Tested-by: Stephane and will apply the patches, Jiri can continue working on these other aspects, right?

I also added this for the casual reader to get up to speed more quickly, please check that it makes sense.

Quoted 7 lines

Committer note:

When we use -c or a period=N term in the event definition, then we don't need to ask the kernel, via perf_event_attr.sample_type |= PERF_SAMPLE_PERIOD, to put the event period in each sample, as we know it already, it is in perf_event_attr.sample_period.

Not quite. It depends on how each event is setup. I can mix & match period and frequency. The PERF_SAMPLE_PERIOD can be dropped only if all the events use a fixed period either via period=N or -c.

I hope that perf report can deal with config mixing period and fixed mode correctly.

#6 lore raw

Re: [PATCH 1/3] perf tools: Fix period/freq terms setup

Quoted 14 lines · Stephane Eranian
Quoted 12 lines · Arnaldo Carvalho de Melo

Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 5 lines

Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:

Quoted 1 line

Otherwise, I tested what you have written so far and it works.

So I take that as a Tested-by: Stephane and will apply the patches, Jiri can continue working on these other aspects, right?

I also added this for the casual reader to get up to speed more quickly, please check that it makes sense.

Committer note:

Quoted 8 lines
Quoted 5 lines

When we use -c or a period=N term in the event definition, then we don't need to ask the kernel, via perf_event_attr.sample_type |= PERF_SAMPLE_PERIOD, to put the event period in each sample, as we know it already, it is in perf_event_attr.sample_period.

Not quite. It depends on how each event is setup. I can mix & match period and frequency. The PERF_SAMPLE_PERIOD can be dropped only if all the events use a fixed period either via period=N or -c.

I think you can have both period and freq based event in one session if that's your concern..? what would be the problem?

jirka

Quoted 2 lines

I hope that perf report can deal with config mixing period and fixed mode correctly.

#7 lore raw

Re: [PATCH 1/3] perf tools: Fix period/freq terms setup

Quoted 20 lines · Jiri Olsa
Quoted 19 lines · Stephane Eranian
Quoted 17 lines · Arnaldo Carvalho de Melo

Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 5 lines

Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:

Quoted 1 line

Otherwise, I tested what you have written so far and it works.

So I take that as a Tested-by: Stephane and will apply the patches, Jiri can continue working on these other aspects, right?

I also added this for the casual reader to get up to speed more quickly, please check that it makes sense.

Committer note:

When we use -c or a period=N term in the event definition, then we don't need to ask the kernel, via perf_event_attr.sample_type |= PERF_SAMPLE_PERIOD, to put the event period in each sample, as we know it already, it is in perf_event_attr.sample_period.

Quoted 7 lines
Quoted 3 lines

Not quite. It depends on how each event is setup. I can mix & match period and frequency. The PERF_SAMPLE_PERIOD can be dropped only if all the events use a fixed period either via period=N or -c.

I think you can have both period and freq based event in one session if that's your concern..? what would be the problem?

My understanding was that perf only support configs where all events have the same attr.sample_type. With frequency mode, you'd want the period recorded in some cases.

Quoted 4 lines

jirka

Quoted 2 lines

I hope that perf report can deal with config mixing period and fixed mode correctly.

maintainer Tested-by Stephane Eranian Acked-by Peter Zijlstra
#8 lore raw

Re: [PATCH 1/3] perf tools: Fix period/freq terms setup

Em Mon, Feb 05, 2018 at 12:58:16PM -0800, Stephane Eranian escreveu:

Quoted 25 lines · Jiri Olsa
Quoted 23 lines · Stephane Eranian
Quoted 22 lines
Quoted 10 lines · Arnaldo Carvalho de Melo

Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 5 lines

Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:

Quoted 1 line

Otherwise, I tested what you have written so far and it works.

So I take that as a Tested-by: Stephane and will apply the patches, Jiri can continue working on these other aspects, right?

I also added this for the casual reader to get up to speed more quickly, please check that it makes sense.

Quoted 7 lines

Committer note:

When we use -c or a period=N term in the event definition, then we don't need to ask the kernel, via perf_event_attr.sample_type |= PERF_SAMPLE_PERIOD, to put the event period in each sample, as we know it already, it is in perf_event_attr.sample_period.

Not quite. It depends on how each event is setup. I can mix & match period and frequency. The PERF_SAMPLE_PERIOD can be dropped only if all the events use a fixed period either via period=N or -c.

Quoted 6 lines
Quoted 2 lines

I think you can have both period and freq based event in one session if that's your concern..? what would be the problem?

My understanding was that perf only support configs where all events have the same attr.sample_type. With frequency mode, you'd want the period recorded in some cases.

[root@jouet ~]# perf record -e cycles/period=2/,instructions/freq=100/ ^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 1.895 MB perf.data (122 samples) ]

[root@jouet ~]# perf report
[root@jouet ~]# perf evlist -v
cycles/period=2/: size: 112, { sample_period, sample_freq }: 2, sample_type: IP|TID|TIME|CPU|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1 instructions/freq=100/: size: 112, config: 0x1, { sample_period, sample_freq }: 100, sample_type: IP|TID|TIME|CPU|PERIOD|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, freq: 1, sample_id_all: 1, exclude_guest: 1 [root@jouet ~]#

commit ff3d527cebc1fa3707c617bfe9e74f53fcfb0955

Author: Adrian Hunter <[email protected]>
Date:   Tue Aug 27 11:23:07 2013 +0300

perf: make events stream always parsable

The event stream is not always parsable because the format of a sample is dependent on the sample_type of the selected event. When there is more than one selected event and the sample_types are not the same then parsing becomes problematic. A sample can be matched to its selected event using the ID that is allocated when the event is opened. Unfortunately, to get the ID from the sample means first parsing it.

This patch adds a new sample format bit PERF_SAMPLE_IDENTIFER that puts the ID at a fixed position so that the ID can be retrieved without parsing the sample. For sample events, that is the first position immediately after the header. For non-sample events, that is the last position.

In this respect parsing samples requires that the sample_type and ID values are recorded. For example, perf tools records struct perf_event_attr and the IDs within the perf.data file. Those must be read first before it is possible to parse samples found later in the perf.data file.

#9 lore raw

Re: [PATCH 1/3] perf tools: Fix period/freq terms setup

Quoted 37 lines · Arnaldo Carvalho de Melo

Em Mon, Feb 05, 2018 at 12:58:16PM -0800, Stephane Eranian escreveu:

Quoted 25 lines · Jiri Olsa
Quoted 23 lines · Stephane Eranian
Quoted 22 lines
Quoted 10 lines · Arnaldo Carvalho de Melo

Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 5 lines

Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:

Quoted 1 line

Otherwise, I tested what you have written so far and it works.

So I take that as a Tested-by: Stephane and will apply the patches, Jiri can continue working on these other aspects, right?

I also added this for the casual reader to get up to speed more quickly, please check that it makes sense.

Quoted 7 lines

Committer note:

When we use -c or a period=N term in the event definition, then we don't need to ask the kernel, via perf_event_attr.sample_type |= PERF_SAMPLE_PERIOD, to put the event period in each sample, as we know it already, it is in perf_event_attr.sample_period.

Not quite. It depends on how each event is setup. I can mix & match period and frequency. The PERF_SAMPLE_PERIOD can be dropped only if all the events use a fixed period either via period=N or -c.

Quoted 6 lines
Quoted 2 lines

I think you can have both period and freq based event in one session if that's your concern..? what would be the problem?

My understanding was that perf only support configs where all events have the same attr.sample_type. With frequency mode, you'd want the period recorded in some cases.

[root@jouet ~]# perf record -e cycles/period=2/,instructions/freq=100/ ^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 1.895 MB perf.data (122 samples) ]

Quoted 6 lines

[root@jouet ~]# perf report
[root@jouet ~]# perf evlist -v
cycles/period=2/: size: 112, { sample_period, sample_freq }: 2, sample_type: IP|TID|TIME|CPU|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1 instructions/freq=100/: size: 112, config: 0x1, { sample_period, sample_freq }: 100, sample_type: IP|TID|TIME|CPU|PERIOD|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, freq: 1, sample_id_all: 1, exclude_guest: 1 [root@jouet ~]#

Looks like this is working then, great!

Now, related to profiling and reporting. There is still an issue I keep running into
with grouping. I want to sample on N events, where N > number of hw counters. Yet I want the same output as perf report --group, i.e., side-by-side profiles as
opposed to showing me one event profile at a time (which is not very useful).

You should not require events to belong to the same group to support this. Many other tools support such output (e.g., VTUNE, Gooda). It is still very valuable even
though events may not have been measured at the same time.

Let me use a simple (and silly but portable) example. Today if I do on Intel x86:

 $ perf record -e branches,branches,branches,branches,branches my_test

And I do:

$ perf report --group
It will show me 5 distinct profiles.

I would like perf to show me a single profile where the 5 events are side-by-side.

Similar to what I get if I do instead:

$ perf record -e '{branches,branches,branches,branches}' my_test
$ perf report --group

But here, I would have to ensure all events fits in a group to allow the reporting
I want. So that would limit me to 4 events.

I think perf report --group should work regardless of how the events were grouped.
Is there already a way to work around this?

Thanks.

Quoted 29 lines

commit ff3d527cebc1fa3707c617bfe9e74f53fcfb0955

Author: Adrian Hunter <[email protected]>
Date:   Tue Aug 27 11:23:07 2013 +0300

perf: make events stream always parsable

The event stream is not always parsable because the format of a sample is dependent on the sample_type of the selected event. When there is more than one selected event and the sample_types are not the same then parsing becomes problematic. A sample can be matched to its selected event using the ID that is allocated when the event is opened. Unfortunately, to get the ID from the sample means first parsing it.

This patch adds a new sample format bit PERF_SAMPLE_IDENTIFER that puts the ID at a fixed position so that the ID can be retrieved without parsing the sample. For sample events, that is the first position immediately after the header. For non-sample events, that is the last position.

In this respect parsing samples requires that the sample_type and ID values are recorded. For example, perf tools records struct perf_event_attr and the IDs within the perf.data file. Those must be read first before it is possible to parse samples found later in the perf.data file.

#10 lore raw

Re: [PATCH 1/3] perf tools: Fix period/freq terms setup

On Mon, Feb 05, 2018 at 06:51:05PM -0800, Stephane Eranian wrote:

SNIP

Quoted 32 lines
Quoted 1 line

Looks like this is working then, great!

Now, related to profiling and reporting. There is still an issue I keep running into
with grouping. I want to sample on N events, where N > number of hw counters. Yet I want the same output as perf report --group, i.e., side-by-side profiles as
opposed to showing me one event profile at a time (which is not very useful).

You should not require events to belong to the same group to support this. Many other tools support such output (e.g., VTUNE, Gooda). It is still very valuable even
though events may not have been measured at the same time.

Let me use a simple (and silly but portable) example. Today if I do on Intel x86:

 $ perf record -e branches,branches,branches,branches,branches my_test

And I do:

$ perf report --group
It will show me 5 distinct profiles.

I would like perf to show me a single profile where the 5 events are side-by-side.

Similar to what I get if I do instead:

$ perf record -e '{branches,branches,branches,branches}' my_test
$ perf report --group
Quoted 7 lines

But here, I would have to ensure all events fits in a group to allow the reporting
I want. So that would limit me to 4 events.

I think perf report --group should work regardless of how the events were grouped.
Is there already a way to work around this?

no workaround.. please try attached patch, it seems to work for what you described

thanks,
jirka

---

tools/perf/builtin-report.c +5 -1
index 4ad5dc649716..35a013992092 100644
@@ -937,6 +937,7 @@ int cmd_report(int argc, const char **argv)
 		"perf report [<options>]",
 		NULL
 	};
+	bool group_set = false;
 	struct report report = {
 		.tool = {
 			.sample		 = process_sample_event,
@@ -1056,7 +1057,7 @@ int cmd_report(int argc, const char **argv)
 		   "Specify disassembler style (e.g. -M intel for intel syntax)"),
 	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
 		    "Show a column with the sum of periods"),
-	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
+	OPT_BOOLEAN_SET(0, "group", &symbol_conf.event_group, &group_set,
 		    "Show event group information together"),
 	OPT_CALLBACK_NOOPT('b', "branch-stack", &branch_mode, "",
 		    "use branch records for per branch histogram filling",
@@ -1173,6 +1174,9 @@ int cmd_report(int argc, const char **argv)
 	has_br_stack = perf_header__has_feat(&session->header,
 					     HEADER_BRANCH_STACK);
 
+	if (group_set && !session->evlist->nr_groups)
+		perf_evlist__set_leader(session->evlist);
+
 	if (itrace_synth_opts.last_branch)
 		has_br_stack = true;
 
#11 lore raw

Re: [PATCH 1/3] perf tools: Fix period/freq terms setup

Quoted 40 lines · Jiri Olsa

On Mon, Feb 05, 2018 at 06:51:05PM -0800, Stephane Eranian wrote:

SNIP

Quoted 36 lines
Quoted 1 line

Looks like this is working then, great!

Now, related to profiling and reporting. There is still an issue I keep running into
with grouping. I want to sample on N events, where N > number of hw counters. Yet I want the same output as perf report --group, i.e., side-by-side profiles as
opposed to showing me one event profile at a time (which is not very useful).

You should not require events to belong to the same group to support this. Many other tools support such output (e.g., VTUNE, Gooda). It is still very valuable even
though events may not have been measured at the same time.

Let me use a simple (and silly but portable) example. Today if I do on Intel x86:

 $ perf record -e branches,branches,branches,branches,branches my_test

And I do:

$ perf report --group
It will show me 5 distinct profiles.

I would like perf to show me a single profile where the 5 events are side-by-side.

Similar to what I get if I do instead:

$ perf record -e '{branches,branches,branches,branches}' my_test
$ perf report --group

But here, I would have to ensure all events fits in a group to allow the reporting
I want. So that would limit me to 4 events.

Quoted 7 lines
Quoted 3 lines

I think perf report --group should work regardless of how the events were grouped.
Is there already a way to work around this?

no workaround.. please try attached patch, it seems to work for what you described

Works for me. That's great!
Thanks.

Quoted 36 lines

thanks,
jirka

---

tools/perf/builtin-report.c +5 -1
index 4ad5dc649716..35a013992092 100644
@@ -937,6 +937,7 @@ int cmd_report(int argc, const char **argv)
                "perf report [<options>]",
                NULL
        };
+       bool group_set = false;
        struct report report = {
                .tool = {
                        .sample          = process_sample_event,
@@ -1056,7 +1057,7 @@ int cmd_report(int argc, const char **argv)
                   "Specify disassembler style (e.g. -M intel for intel syntax)"),
        OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
                    "Show a column with the sum of periods"),
-       OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
+       OPT_BOOLEAN_SET(0, "group", &symbol_conf.event_group, &group_set,
                    "Show event group information together"),
        OPT_CALLBACK_NOOPT('b', "branch-stack", &branch_mode, "",
                    "use branch records for per branch histogram filling",
@@ -1173,6 +1174,9 @@ int cmd_report(int argc, const char **argv)
        has_br_stack = perf_header__has_feat(&session->header,
                                             HEADER_BRANCH_STACK);
 
+       if (group_set && !session->evlist->nr_groups)
+               perf_evlist__set_leader(session->evlist);
+
        if (itrace_synth_opts.last_branch)
                has_br_stack = true;
 
#12 lore raw

PATCH 1/1 perf report: Add support to display group output for non group events

On Wed, Feb 07, 2018 at 10:52:35AM -0800, Stephane Eranian wrote:

SNIP

Quoted 12 lines
Quoted 12 lines
Quoted 11 lines

Similar to what I get if I do instead:

$ perf record -e '{branches,branches,branches,branches}' my_test
$ perf report --group

But here, I would have to ensure all events fits in a group to allow the reporting
I want. So that would limit me to 4 events.

I think perf report --group should work regardless of how the events were grouped.
Is there already a way to work around this?

Quoted 7 lines
Quoted 3 lines

no workaround.. please try attached patch, it seems to work for what you described

Works for me. That's great!
Thanks.

thanks, full patch attached

jirka

---
Add support to display group output for if non grouped events are detected and user forces --group option. Now for non-group events recorded like:

  $ perf record -e 'cycles,instructions' ls

you can still get group output by using --group option in report:

  $ perf report --group --stdio
  ...
  #         Overhead  Command  Shared Object     Symbol
  # ................  .......  ................  ......................
  #
      17.67%   0.00%  ls       libc-2.25.so      [.] _IO_do_write@@GLIB
      15.59%  25.94%  ls       ls                [.] calculate_columns
      15.41%  31.35%  ls       libc-2.25.so      [.] __strcoll_l
  ...
---
 tools/perf/Documentation/perf-report.txt | 3 ++-
 tools/perf/builtin-report.c              | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)
tools/perf/Documentation/perf-report.txt +2 -1
index 907e505b6309..a76b871f78a6 100644
@@ -354,7 +354,8 @@ OPTIONS
         Path to objdump binary.
 
 --group::
-	Show event group information together.
+	Show event group information together. It forces group output also
+	if there are no groups defined in data file.
 
 --demangle::
 	Demangle symbol names to human readable form. It's enabled by default,
tools/perf/builtin-report.c +5 -1
index 8ef71669e7a0..1eedb1815c4c 100644
@@ -938,6 +938,7 @@ int cmd_report(int argc, const char **argv)
 		"perf report [<options>]",
 		NULL
 	};
+	bool group_set = false;
 	struct report report = {
 		.tool = {
 			.sample		 = process_sample_event,
@@ -1057,7 +1058,7 @@ int cmd_report(int argc, const char **argv)
 		   "Specify disassembler style (e.g. -M intel for intel syntax)"),
 	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
 		    "Show a column with the sum of periods"),
-	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
+	OPT_BOOLEAN_SET(0, "group", &symbol_conf.event_group, &group_set,
 		    "Show event group information together"),
 	OPT_CALLBACK_NOOPT('b', "branch-stack", &branch_mode, "",
 		    "use branch records for per branch histogram filling",
@@ -1174,6 +1175,9 @@ int cmd_report(int argc, const char **argv)
 	has_br_stack = perf_header__has_feat(&session->header,
 					     HEADER_BRANCH_STACK);
 
+	if (group_set && !session->evlist->nr_groups)
+		perf_evlist__set_leader(session->evlist);
+
 	if (itrace_synth_opts.last_branch)
 		has_br_stack = true;
 
Signature · 1 line
2.13.6
#13 lore raw

Re: [PATCH] perf report: Add support to display group output for non group events

Em Fri, Feb 09, 2018 at 10:27:34AM +0100, Jiri Olsa escreveu:

Quoted 19 lines · Stephane Eranian

SNIP

Quoted 15 lines
Quoted 15 lines
Quoted 11 lines

Similar to what I get if I do instead:

$ perf record -e '{branches,branches,branches,branches}' my_test
$ perf report --group

But here, I would have to ensure all events fits in a group to allow the reporting
I want. So that would limit me to 4 events.

I think perf report --group should work regardless of how the events were grouped.
Is there already a way to work around this?

no workaround.. please try attached patch, it seems to work for what you described

Quoted 8 lines
Quoted 4 lines

Works for me. That's great!
Thanks.

thanks, full patch attached

jirka

Humm, its a nice hack, but it would be even better if it didn't showed it as if it was really a group:

Samples: 20 of event 'anon group { cycles, instructions }', Event count (approx.): 4712980

It would be better to instead add another condition to the evlist that would trigger the view with all the examples...

I'm applying it anyway, as it is useful, but would be nice to have the same output except for that header, that should read instead:

Samples: 20 of non grouped events: cycles, instructions, Event count (approx.): 4712980

- Arnaldo

Quoted 76 lines

---
Add support to display group output for if non grouped events are detected and user forces --group option. Now for non-group events recorded like:

  $ perf record -e 'cycles,instructions' ls

you can still get group output by using --group option in report:

  $ perf report --group --stdio
  ...
  #         Overhead  Command  Shared Object     Symbol
  # ................  .......  ................  ......................
  #
      17.67%   0.00%  ls       libc-2.25.so      [.] _IO_do_write@@GLIB
      15.59%  25.94%  ls       ls                [.] calculate_columns
      15.41%  31.35%  ls       libc-2.25.so      [.] __strcoll_l
  ...
---
 tools/perf/Documentation/perf-report.txt | 3 ++-
 tools/perf/builtin-report.c              | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)
tools/perf/Documentation/perf-report.txt +2 -1
index 907e505b6309..a76b871f78a6 100644
@@ -354,7 +354,8 @@ OPTIONS
         Path to objdump binary.
 
 --group::
-	Show event group information together.
+	Show event group information together. It forces group output also
+	if there are no groups defined in data file.
 
 --demangle::
 	Demangle symbol names to human readable form. It's enabled by default,
tools/perf/builtin-report.c +5 -1
index 8ef71669e7a0..1eedb1815c4c 100644
@@ -938,6 +938,7 @@ int cmd_report(int argc, const char **argv)
 		"perf report [<options>]",
 		NULL
 	};
+	bool group_set = false;
 	struct report report = {
 		.tool = {
 			.sample		 = process_sample_event,
@@ -1057,7 +1058,7 @@ int cmd_report(int argc, const char **argv)
 		   "Specify disassembler style (e.g. -M intel for intel syntax)"),
 	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
 		    "Show a column with the sum of periods"),
-	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
+	OPT_BOOLEAN_SET(0, "group", &symbol_conf.event_group, &group_set,
 		    "Show event group information together"),
 	OPT_CALLBACK_NOOPT('b', "branch-stack", &branch_mode, "",
 		    "use branch records for per branch histogram filling",
@@ -1174,6 +1175,9 @@ int cmd_report(int argc, const char **argv)
 	has_br_stack = perf_header__has_feat(&session->header,
 					     HEADER_BRANCH_STACK);
 
+	if (group_set && !session->evlist->nr_groups)
+		perf_evlist__set_leader(session->evlist);
+
 	if (itrace_synth_opts.last_branch)
 		has_br_stack = true;
 
Signature · 1 line
2.13.6
#14 lore raw

Re: [PATCH] perf report: Add support to display group output for non group events

Em Fri, Feb 09, 2018 at 03:37:11PM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 34 lines

Em Fri, Feb 09, 2018 at 10:27:34AM +0100, Jiri Olsa escreveu:

Quoted 19 lines · Stephane Eranian

SNIP

Quoted 15 lines
Quoted 15 lines
Quoted 11 lines

Similar to what I get if I do instead:

$ perf record -e '{branches,branches,branches,branches}' my_test
$ perf report --group

But here, I would have to ensure all events fits in a group to allow the reporting
I want. So that would limit me to 4 events.

I think perf report --group should work regardless of how the events were grouped.
Is there already a way to work around this?

no workaround.. please try attached patch, it seems to work for what you described

Quoted 8 lines
Quoted 4 lines

Works for me. That's great!
Thanks.

thanks, full patch attached

jirka

Humm, its a nice hack, but it would be even better if it didn't showed it as if it was really a group:

Samples: 20 of event 'anon group { cycles, instructions }', Event count (approx.): 4712980

Quoted 7 lines

It would be better to instead add another condition to the evlist that would trigger the view with all the examples...

I'm applying it anyway, as it is useful, but would be nice to have the same output except for that header, that should read instead:

Samples: 20 of non grouped events: cycles, instructions, Event count (approx.): 4712980

Till then, that documentation hunk should help understanding this issue. :-)

- Arnaldo

#15 lore raw

Re: [PATCH] perf report: Add support to display group output for non group events

In reply to Arnaldo Carvalho de Melo
Quoted 34 lines · Arnaldo Carvalho de Melo

Em Fri, Feb 09, 2018 at 10:27:34AM +0100, Jiri Olsa escreveu:

Quoted 19 lines · Stephane Eranian

SNIP

Quoted 15 lines
Quoted 15 lines
Quoted 11 lines

Similar to what I get if I do instead:

$ perf record -e '{branches,branches,branches,branches}' my_test
$ perf report --group

But here, I would have to ensure all events fits in a group to allow the reporting
I want. So that would limit me to 4 events.

I think perf report --group should work regardless of how the events were grouped.
Is there already a way to work around this?

no workaround.. please try attached patch, it seems to work for what you described

Quoted 8 lines
Quoted 4 lines

Works for me. That's great!
Thanks.

thanks, full patch attached

jirka

Humm, its a nice hack, but it would be even better if it didn't showed it as if it was really a group:

Samples: 20 of event 'anon group { cycles, instructions }', Event count (approx.): 4712980

Quoted 7 lines

It would be better to instead add another condition to the evlist that would trigger the view with all the examples...

I'm applying it anyway, as it is useful, but would be nice to have the same output except for that header, that should read instead:

Samples: 20 of non grouped events: cycles, instructions, Event count (approx.): 4712980

right, I'll try to post something like that

thanks,
jirka

#16 lore raw

Re: [PATCH] perf report: Add support to display group output for non group events

Em Fri, Feb 09, 2018 at 08:10:57PM +0100, Jiri Olsa escreveu:

Quoted 11 lines · Arnaldo Carvalho de Melo
Quoted 9 lines

Em Fri, Feb 09, 2018 at 10:27:34AM +0100, Jiri Olsa escreveu:

Humm, its a nice hack, but it would be even better if it didn't showed it as if it was really a group:

Samples: 20 of event 'anon group { cycles, instructions }', Event count (approx.): 4712980

It would be better to instead add another condition to the evlist that would trigger the view with all the examples...

Quoted 6 lines
Quoted 4 lines

I'm applying it anyway, as it is useful, but would be nice to have the same output except for that header, that should read instead:

Samples: 20 of non grouped events: cycles, instructions, Event count (approx.): 4712980

right, I'll try to post something like that

I've pushed out with this patch, so please go on from there.

Thanks!

- Arnaldo

#17 lore raw

[tip:perf/core] perf report: Add support to display group output for non group events

In reply to Jiri Olsa
perf report: Add support to display group output for non group events
Commit-ID:  ad52b8cb4886f572b147b02f4c59a648bbf05f9c
Gitweb:     https://git.kernel.org/tip/ad52b8cb4886f572b147b02f4c59a648bbf05f9c
Author:     Jiri Olsa <[email protected]>
AuthorDate: Fri, 9 Feb 2018 10:27:34 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 16 Feb 2018 10:09:24 -0300

perf report: Add support to display group output for non group events

Add support to display group output for if non grouped events are detected and user forces --group option. Now for non-group events recorded like:

  $ perf record -e 'cycles,instructions' ls

you can still get group output by using --group option in report:

  $ perf report --group --stdio
  ...
  #         Overhead  Command  Shared Object     Symbol
  # ................  .......  ................  ......................
  #
      17.67%   0.00%  ls       libc-2.25.so      [.] _IO_do_write@@GLIB
      15.59%  25.94%  ls       ls                [.] calculate_columns
      15.41%  31.35%  ls       libc-2.25.so      [.] __strcoll_l
  ...

Committer note:

We should improve on this by making sure that the first line states that this is not a group, but since the user doesn't have to force group view when really using grouped events (e.g. '{cycles,instructions}'), the user better know what is being done...

---
 tools/perf/Documentation/perf-report.txt | 3 ++-
 tools/perf/builtin-report.c              | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)
tools/perf/Documentation/perf-report.txt +2 -1
index 907e505..a76b871 100644
@@ -354,7 +354,8 @@ OPTIONS
         Path to objdump binary.
 
 --group::
-	Show event group information together.
+	Show event group information together. It forces group output also
+	if there are no groups defined in data file.
 
 --demangle::
 	Demangle symbol names to human readable form. It's enabled by default,
tools/perf/builtin-report.c +5 -1
index 8ef7166..1eedb18 100644
@@ -938,6 +938,7 @@ int cmd_report(int argc, const char **argv)
 		"perf report [<options>]",
 		NULL
 	};
+	bool group_set = false;
 	struct report report = {
 		.tool = {
 			.sample		 = process_sample_event,
@@ -1057,7 +1058,7 @@ int cmd_report(int argc, const char **argv)
 		   "Specify disassembler style (e.g. -M intel for intel syntax)"),
 	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
 		    "Show a column with the sum of periods"),
-	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
+	OPT_BOOLEAN_SET(0, "group", &symbol_conf.event_group, &group_set,
 		    "Show event group information together"),
 	OPT_CALLBACK_NOOPT('b', "branch-stack", &branch_mode, "",
 		    "use branch records for per branch histogram filling",
@@ -1174,6 +1175,9 @@ repeat:
 	has_br_stack = perf_header__has_feat(&session->header,
 					     HEADER_BRANCH_STACK);
 
+	if (group_set && !session->evlist->nr_groups)
+		perf_evlist__set_leader(session->evlist);
+
 	if (itrace_synth_opts.last_branch)
 		has_br_stack = true;
 
#18 lore raw

Re: [PATCH 1/3] perf tools: Fix period/freq terms setup

In reply to Stephane Eranian
Quoted 36 lines · Stephane Eranian

Jiri,

Quoted 8 lines · Jiri Olsa

Stephane reported that we don't set properly PERIOD sample type for events with period term defined.

Before:

  $ perf record -e cpu/cpu-cycles,period=1000/u ls
  $ perf evlist -v
  cpu/cpu-cycles,period=1000/u: ... sample_type: IP|TID|TIME|PERIOD, ...
Quoted 7 lines

After:

  $ perf record -e cpu/cpu-cycles,period=1000/u ls
  $ perf evlist -v
  cpu/cpu-cycles,period=1000/u: ... sample_type: IP|TID|TIME, ...

Setting PERIOD sample type based on period term setup.

there is still one problem remaining here. It has to do with the handling of cycles:pp or :p or :ppp. Suppose I want to set a period for it while I am also sampling on other events: Something like:

$ perf record -e
cycles:pp,instructions,cpu/event=0xd0,umaks=0x81,period=100000/ .....

I want to set the period for cycles:pp, but not for instructions. I cannot use -c because
it would also force a period on instructions. I could use the raw hw raw event code for cycles:pp.
But that does not work because recent kernels prevent use of hw filters on events programmed
for PEBS, e.g., cpu/event=0xc2,umask=0x1,cmask=16,inv/pp is rejected. PEBS does not support filters.
It works in the case of cycles:pp simply by the nature on the underlying event and the stalls.

Quoted 3 lines

To get precise cycles, the only event syntax you can use is cycles:pp, but then you cannot specify
an event-specific period. This needs to be fixed as well.

you can use p modifier behind like: cpu/.../pp

Quoted 7 lines

I'd like to be able to say:

$ perf record -e
cycles:pp:period=10000001,cpu/event=0xd0,umaks=0x81,period=100000/

Or something equivalent.

and you can specify terms for hw events like 'cycles'

[jolsa@krava perf]$ sudo ./perf record -e 'cycles/period=10000001/pp,cpu/event=0xd0,umask=0x81,period=100000/' ^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.579 MB perf.data (722 samples) ]

[jolsa@krava perf]$ ./perf evlist -v
cycles/period=10000001/pp: size: 112, { sample_period, sample_freq }: 10000001, sample_type: IP|TID|TIME|ID|CPU, read_format: ID, disabled: 1, inherit: 1, mmap: 1, comm: 1, task: 1, precise_ip: 2, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1 cpu/event=0xd0,umask=0x81,period=100000/: type: 4, size: 112, config: 0x81d0, { sample_period, sample_freq }: 100000, sample_type: IP|TID|TIME|ID|CPU, read_format: ID, disabled: 1, inherit: 1, sample_id_all: 1, exclude_guest: 1

jirka

#19 lore raw

Re: [PATCH 1/3] perf tools: Fix period/freq terms setup

Quoted 49 lines · Jiri Olsa
Quoted 36 lines · Stephane Eranian

Jiri,

Quoted 8 lines · Jiri Olsa

Stephane reported that we don't set properly PERIOD sample type for events with period term defined.

Before:

  $ perf record -e cpu/cpu-cycles,period=1000/u ls
  $ perf evlist -v
  cpu/cpu-cycles,period=1000/u: ... sample_type: IP|TID|TIME|PERIOD, ...
Quoted 7 lines

After:

  $ perf record -e cpu/cpu-cycles,period=1000/u ls
  $ perf evlist -v
  cpu/cpu-cycles,period=1000/u: ... sample_type: IP|TID|TIME, ...

Setting PERIOD sample type based on period term setup.

there is still one problem remaining here. It has to do with the handling of cycles:pp or :p or :ppp. Suppose I want to set a period for it while I am also sampling on other events: Something like:

$ perf record -e
cycles:pp,instructions,cpu/event=0xd0,umaks=0x81,period=100000/ .....

I want to set the period for cycles:pp, but not for instructions. I cannot use -c because
it would also force a period on instructions. I could use the raw hw raw event code for cycles:pp.
But that does not work because recent kernels prevent use of hw filters on events programmed
for PEBS, e.g., cpu/event=0xc2,umask=0x1,cmask=16,inv/pp is rejected. PEBS does not support filters.
It works in the case of cycles:pp simply by the nature on the underlying event and the stalls.

Quoted 3 lines

To get precise cycles, the only event syntax you can use is cycles:pp, but then you cannot specify
an event-specific period. This needs to be fixed as well.

you can use p modifier behind like: cpu/.../pp

Quoted 6 lines

I'd like to be able to say:

$ perf record -e
cycles:pp:period=10000001,cpu/event=0xd0,umaks=0x81,period=100000/
Quoted 8 lines
Quoted 1 line

Or something equivalent.

and you can specify terms for hw events like 'cycles'

[jolsa@krava perf]$ sudo ./perf record -e 'cycles/period=10000001/pp,cpu/event=0xd0,umask=0x81,period=100000/' ^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.579 MB perf.data (722 samples) ]

Ok, I did not know about this syntax. It looks bizarre because you are using an event name as a PMU instance.
Works for me.
Thanks.

Quoted 5 lines

[jolsa@krava perf]$ ./perf evlist -v
cycles/period=10000001/pp: size: 112, { sample_period, sample_freq }: 10000001, sample_type: IP|TID|TIME|ID|CPU, read_format: ID, disabled: 1, inherit: 1, mmap: 1, comm: 1, task: 1, precise_ip: 2, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1 cpu/event=0xd0,umask=0x81,period=100000/: type: 4, size: 112, config: 0x81d0, { sample_period, sample_freq }: 100000, sample_type: IP|TID|TIME|ID|CPU, read_format: ID, disabled: 1, inherit: 1, sample_id_all: 1, exclude_guest: 1

jirka

#20 lore raw

[tip:perf/urgent] perf evsel: Fix period/freq terms setup

In reply to Jiri Olsa
perf evsel: Fix period/freq terms setup
Commit-ID:  49c0ae80eb32426fa133246200628e529067c595
Gitweb:     https://git.kernel.org/tip/49c0ae80eb32426fa133246200628e529067c595
Author:     Jiri Olsa <[email protected]>
AuthorDate: Thu, 1 Feb 2018 09:38:10 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 5 Feb 2018 12:11:58 -0300

perf evsel: Fix period/freq terms setup

Stephane reported that we don't set properly PERIOD sample type for events with period term defined.

Before:

  $ perf record -e cpu/cpu-cycles,period=1000/u ls
  $ perf evlist -v
  cpu/cpu-cycles,period=1000/u: ... sample_type: IP|TID|TIME|PERIOD, ...

After:

  $ perf record -e cpu/cpu-cycles,period=1000/u ls
  $ perf evlist -v
  cpu/cpu-cycles,period=1000/u: ... sample_type: IP|TID|TIME, ...

Setting PERIOD sample type based on period term setup.

Committer note:

When we use -c or a period=N term in the event definition, then we don't need to ask the kernel, for this event, via perf_event_attr.sample_type |= PERF_SAMPLE_PERIOD, to put the event period in each sample for this event, as we know it already, it is in perf_event_attr.sample_period.

---
 tools/perf/util/evsel.c | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/util/evsel.c +2 -0
index 66fa451..f2f2eaa 100644
@@ -745,12 +745,14 @@ static void apply_config_terms(struct perf_evsel *evsel,
 			if (!(term->weak && opts->user_interval != ULLONG_MAX)) {
 				attr->sample_period = term->val.period;
 				attr->freq = 0;
+				perf_evsel__reset_sample_bit(evsel, PERIOD);
 			}
 			break;
 		case PERF_EVSEL__CONFIG_TERM_FREQ:
 			if (!(term->weak && opts->user_freq != UINT_MAX)) {
 				attr->sample_freq = term->val.freq;
 				attr->freq = 1;
+				perf_evsel__set_sample_bit(evsel, PERIOD);
 			}
 			break;
 		case PERF_EVSEL__CONFIG_TERM_TIME:
author reviewer
#21 lore raw

PATCH 2/3 perf record: Fix period option handling

In reply to Jiri Olsa

Stephan reported we don't unset PERIOD sample type when --no-period is specified. Adding the unset check and reset PERIOD if --no-period is specified.

---
 tools/perf/builtin-record.c |  3 ++-
 tools/perf/perf.h           |  1 +
 tools/perf/util/evsel.c     | 11 ++++++++---
 3 files changed, 11 insertions(+), 4 deletions(-)
tools/perf/builtin-record.c +2 -1
index f251e824edac..907267206973 100644
@@ -1566,7 +1566,8 @@ static struct option __record_options[] = {
 	OPT_BOOLEAN_SET('T', "timestamp", &record.opts.sample_time,
 			&record.opts.sample_time_set,
 			"Record the sample timestamps"),
-	OPT_BOOLEAN('P', "period", &record.opts.period, "Record the sample period"),
+	OPT_BOOLEAN_SET('P', "period", &record.opts.period, &record.opts.period_set,
+			"Record the sample period"),
 	OPT_BOOLEAN('n', "no-samples", &record.opts.no_samples,
 		    "don't sample"),
 	OPT_BOOLEAN_SET('N', "no-buildid-cache", &record.no_buildid_cache,
tools/perf/perf.h +1 -0
index 2357f4ccc9c7..cfe46236a5e5 100644
@@ -50,6 +50,7 @@ struct record_opts {
 	bool	     sample_time_set;
 	bool	     sample_cpu;
 	bool	     period;
+	bool	     period_set;
 	bool	     running_time;
 	bool	     full_auxtrace;
 	bool	     auxtrace_snapshot_mode;
tools/perf/util/evsel.c +8 -3
index f2f2eaafde6d..ff359c9ece2e 100644
@@ -971,9 +971,6 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts,
 	if (target__has_cpu(&opts->target) || opts->sample_cpu)
 		perf_evsel__set_sample_bit(evsel, CPU);
 
-	if (opts->period)
-		perf_evsel__set_sample_bit(evsel, PERIOD);
-
 	/*
 	 * When the user explicitly disabled time don't force it here.
 	 */
@@ -1075,6 +1072,14 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts,
 	apply_config_terms(evsel, opts, track);
 
 	evsel->ignore_missing_thread = opts->ignore_missing_thread;
+
+	/* The --period option takes the precedence. */
+	if (opts->period_set) {
+		if (opts->period)
+			perf_evsel__set_sample_bit(evsel, PERIOD);
+		else
+			perf_evsel__reset_sample_bit(evsel, PERIOD);
+	}
 }
 
 static int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
Signature · 1 line
2.13.6
#22 lore raw

[tip:perf/urgent] perf record: Fix period option handling

perf record: Fix period option handling
Commit-ID:  f290aa1ffa45ed7e37599840878b4dae68269ee1
Gitweb:     https://git.kernel.org/tip/f290aa1ffa45ed7e37599840878b4dae68269ee1
Author:     Jiri Olsa <[email protected]>
AuthorDate: Thu, 1 Feb 2018 09:38:11 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 5 Feb 2018 12:18:28 -0300

perf record: Fix period option handling

Stephan reported we don't unset PERIOD sample type when --no-period is specified. Adding the unset check and reset PERIOD if --no-period is specified.

Committer notes:

Check the sample_type, it shouldn't have PERF_SAMPLE_PERIOD there when --no-period is used.

Before:

  # perf record --no-period sleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.018 MB perf.data (7 samples) ]
  # perf evlist -v
  cycles:ppp: size: 112, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|PERIOD, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: 1, precise_ip: 3, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1
  #

After:

[root@jouet ~]# perf record --no-period sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.019 MB perf.data (17 samples) ]
[root@jouet ~]# perf evlist -v
cycles:ppp: size: 112, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: 1, precise_ip: 3, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1 [root@jouet ~]#

---
 tools/perf/builtin-record.c |  3 ++-
 tools/perf/perf.h           |  1 +
 tools/perf/util/evsel.c     | 11 ++++++++---
 3 files changed, 11 insertions(+), 4 deletions(-)
tools/perf/builtin-record.c +2 -1
index 65681a1..bf4ca74 100644
@@ -1566,7 +1566,8 @@ static struct option __record_options[] = {
 	OPT_BOOLEAN_SET('T', "timestamp", &record.opts.sample_time,
 			&record.opts.sample_time_set,
 			"Record the sample timestamps"),
-	OPT_BOOLEAN('P', "period", &record.opts.period, "Record the sample period"),
+	OPT_BOOLEAN_SET('P', "period", &record.opts.period, &record.opts.period_set,
+			"Record the sample period"),
 	OPT_BOOLEAN('n', "no-samples", &record.opts.no_samples,
 		    "don't sample"),
 	OPT_BOOLEAN_SET('N', "no-buildid-cache", &record.no_buildid_cache,
tools/perf/perf.h +1 -0
index 2357f4cc..cfe4623 100644
@@ -50,6 +50,7 @@ struct record_opts {
 	bool	     sample_time_set;
 	bool	     sample_cpu;
 	bool	     period;
+	bool	     period_set;
 	bool	     running_time;
 	bool	     full_auxtrace;
 	bool	     auxtrace_snapshot_mode;
tools/perf/util/evsel.c +8 -3
index f2f2eaa..ff359c9 100644
@@ -971,9 +971,6 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts,
 	if (target__has_cpu(&opts->target) || opts->sample_cpu)
 		perf_evsel__set_sample_bit(evsel, CPU);
 
-	if (opts->period)
-		perf_evsel__set_sample_bit(evsel, PERIOD);
-
 	/*
 	 * When the user explicitly disabled time don't force it here.
 	 */
@@ -1075,6 +1072,14 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts,
 	apply_config_terms(evsel, opts, track);
 
 	evsel->ignore_missing_thread = opts->ignore_missing_thread;
+
+	/* The --period option takes the precedence. */
+	if (opts->period_set) {
+		if (opts->period)
+			perf_evsel__set_sample_bit(evsel, PERIOD);
+		else
+			perf_evsel__reset_sample_bit(evsel, PERIOD);
+	}
 }
 
 static int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
author reviewer
#23 lore raw

PATCH 3/3 x86/events/intel/ds: Add PERF_SAMPLE_PERIOD into PEBS_FREERUNNING_FLAGS

In reply to Jiri Olsa

Stephan reported that we don't support period for
enabling large PEBS data, which there's no reason
for. Adding PERF_SAMPLE_PERIOD into freerunning
flags.

---
 arch/x86/events/perf_event.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/x86/events/perf_event.h +2 -1
index 8e4ea143ed96..78f91ec1056e 100644
@@ -93,7 +93,8 @@ struct amd_nb {
 	PERF_SAMPLE_ID | PERF_SAMPLE_CPU | PERF_SAMPLE_STREAM_ID | \
 	PERF_SAMPLE_DATA_SRC | PERF_SAMPLE_IDENTIFIER | \
 	PERF_SAMPLE_TRANSACTION | PERF_SAMPLE_PHYS_ADDR | \
-	PERF_SAMPLE_REGS_INTR | PERF_SAMPLE_REGS_USER)
+	PERF_SAMPLE_REGS_INTR | PERF_SAMPLE_REGS_USER | \
+	PERF_SAMPLE_PERIOD)
 
 #define PEBS_REGS \
 	(PERF_REG_X86_AX | \
Signature · 1 line
2.13.6
#24 lore raw

[tip:perf/urgent] x86/events/intel/ds: Add PERF_SAMPLE_PERIOD into PEBS_FREERUNNING_FLAGS

x86/events/intel/ds: Add PERF_SAMPLE_PERIOD into PEBS_FREERUNNING_FLAGS
Commit-ID:  11974914e8e6d44387b4b23fa6dbd40c94baeb8d
Gitweb:     https://git.kernel.org/tip/11974914e8e6d44387b4b23fa6dbd40c94baeb8d
Author:     Jiri Olsa <[email protected]>
AuthorDate: Thu, 1 Feb 2018 09:38:12 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 5 Feb 2018 13:48:44 -0300

x86/events/intel/ds: Add PERF_SAMPLE_PERIOD into PEBS_FREERUNNING_FLAGS

Stephane reported that we don't support period for enabling large PEBS data, which there's no reason for. Adding PERF_SAMPLE_PERIOD into freerunning flags.

Tested it with:

  # perf record -e cycles:P -c 100 --no-timestamp -C 0 --period
---
 arch/x86/events/perf_event.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/x86/events/perf_event.h +2 -1
index 8e4ea143..78f91ec 100644
@@ -93,7 +93,8 @@ struct amd_nb {
 	PERF_SAMPLE_ID | PERF_SAMPLE_CPU | PERF_SAMPLE_STREAM_ID | \
 	PERF_SAMPLE_DATA_SRC | PERF_SAMPLE_IDENTIFIER | \
 	PERF_SAMPLE_TRANSACTION | PERF_SAMPLE_PHYS_ADDR | \
-	PERF_SAMPLE_REGS_INTR | PERF_SAMPLE_REGS_USER)
+	PERF_SAMPLE_REGS_INTR | PERF_SAMPLE_REGS_USER | \
+	PERF_SAMPLE_PERIOD)
 
 #define PEBS_REGS \
 	(PERF_REG_X86_AX | \