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

[Stephane Eranian]

On Tue, Feb 6, 2018 at 1:35 AM, Jiri Olsa  wrote:
> On Mon, Feb 05, 2018 at 06:51:05PM -0800, Stephane Eranian wrote:
>
> SNIP
>
>> >
>> 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?
>
> no workaround.. please try attached patch, it seems
> to work for what you described
>
Works for me. That's great!
Thanks.

Tested-By: Stephane Eranian 

> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 4ad5dc649716..35a013992092 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -937,6 +937,7 @@ int cmd_report(int argc, const char **argv)
> "perf report []",
> 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", _conf.show_total_period,
> "Show a column with the sum of periods"),
> -   OPT_BOOLEAN(0, "group", _conf.event_group,
> +   OPT_BOOLEAN_SET(0, "group", _conf.event_group, _set,
> "Show event group information together"),
> OPT_CALLBACK_NOOPT('b', "branch-stack", _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(>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;
>

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

[Stephane Eranian]

On Tue, Feb 6, 2018 at 1:35 AM, Jiri Olsa  wrote:
> On Mon, Feb 05, 2018 at 06:51:05PM -0800, Stephane Eranian wrote:
>
> SNIP
>
>> >
>> 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?
>
> no workaround.. please try attached patch, it seems
> to work for what you described
>
Works for me. That's great!
Thanks.

Tested-By: Stephane Eranian 

> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 4ad5dc649716..35a013992092 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -937,6 +937,7 @@ int cmd_report(int argc, const char **argv)
> "perf report []",
> 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", _conf.show_total_period,
> "Show a column with the sum of periods"),
> -   OPT_BOOLEAN(0, "group", _conf.event_group,
> +   OPT_BOOLEAN_SET(0, "group", _conf.event_group, _set,
> "Show event group information together"),
> OPT_CALLBACK_NOOPT('b', "branch-stack", _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(>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;
>

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

[Jiri Olsa]

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

SNIP

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

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

thanks,
jirka


---
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 4ad5dc649716..35a013992092 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -937,6 +937,7 @@ int cmd_report(int argc, const char **argv)
"perf report []",
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", _conf.show_total_period,
"Show a column with the sum of periods"),
-   OPT_BOOLEAN(0, "group", _conf.event_group,
+   OPT_BOOLEAN_SET(0, "group", _conf.event_group, _set,
"Show event group information together"),
OPT_CALLBACK_NOOPT('b', "branch-stack", _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(>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;
 

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

[Jiri Olsa]

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

SNIP

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

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

thanks,
jirka


---
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 4ad5dc649716..35a013992092 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -937,6 +937,7 @@ int cmd_report(int argc, const char **argv)
"perf report []",
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", _conf.show_total_period,
"Show a column with the sum of periods"),
-   OPT_BOOLEAN(0, "group", _conf.event_group,
+   OPT_BOOLEAN_SET(0, "group", _conf.event_group, _set,
"Show event group information together"),
OPT_CALLBACK_NOOPT('b', "branch-stack", _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(>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;
 

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

[Stephane Eranian]

On Mon, Feb 5, 2018 at 1:13 PM, Arnaldo Carvalho de Melo
 wrote:
> Em Mon, Feb 05, 2018 at 12:58:16PM -0800, Stephane Eranian escreveu:
>> On Mon, Feb 5, 2018 at 7:17 AM, Jiri Olsa  wrote:
>> > On Fri, Feb 02, 2018 at 01:04:34PM -0800, Stephane Eranian wrote:
>> >> On Fri, Feb 2, 2018 at 12:40 PM, Arnaldo Carvalho de Melo
>> >>  wrote:
>> >> > Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo 
>> >> > escreveu:
>> >> >> Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
>> >> >> > 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.
>> >> >
>> >> 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.
>
> [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 ~]#
>
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.

> commit ff3d527cebc1fa3707c617bfe9e74f53fcfb0955
> Author: Adrian Hunter 
> 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
> 

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

[Stephane Eranian]

On Mon, Feb 5, 2018 at 1:13 PM, Arnaldo Carvalho de Melo
 wrote:
> Em Mon, Feb 05, 2018 at 12:58:16PM -0800, Stephane Eranian escreveu:
>> On Mon, Feb 5, 2018 at 7:17 AM, Jiri Olsa  wrote:
>> > On Fri, Feb 02, 2018 at 01:04:34PM -0800, Stephane Eranian wrote:
>> >> On Fri, Feb 2, 2018 at 12:40 PM, Arnaldo Carvalho de Melo
>> >>  wrote:
>> >> > Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo 
>> >> > escreveu:
>> >> >> Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
>> >> >> > 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.
>> >> >
>> >> 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.
>
> [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 ~]#
>
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.

> commit ff3d527cebc1fa3707c617bfe9e74f53fcfb0955
> Author: Adrian Hunter 
> 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 

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

[Arnaldo Carvalho de Melo]

Em Mon, Feb 05, 2018 at 12:58:16PM -0800, Stephane Eranian escreveu:
> On Mon, Feb 5, 2018 at 7:17 AM, Jiri Olsa  wrote:
> > On Fri, Feb 02, 2018 at 01:04:34PM -0800, Stephane Eranian wrote:
> >> On Fri, Feb 2, 2018 at 12:40 PM, Arnaldo Carvalho de Melo
> >>  wrote:
> >> > Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo 
> >> > escreveu:
> >> >> Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
> >> >> > 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.
> >> >
> >> 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.

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

Signed-off-by: Adrian Hunter 
Tested-by: Stephane Eranian 
Acked-by: Peter Zijlstra 

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

[Arnaldo Carvalho de Melo]

Em Mon, Feb 05, 2018 at 12:58:16PM -0800, Stephane Eranian escreveu:
> On Mon, Feb 5, 2018 at 7:17 AM, Jiri Olsa  wrote:
> > On Fri, Feb 02, 2018 at 01:04:34PM -0800, Stephane Eranian wrote:
> >> On Fri, Feb 2, 2018 at 12:40 PM, Arnaldo Carvalho de Melo
> >>  wrote:
> >> > Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo 
> >> > escreveu:
> >> >> Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
> >> >> > 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.
> >> >
> >> 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.

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

Signed-off-by: Adrian Hunter 
Tested-by: Stephane Eranian 
Acked-by: Peter Zijlstra 

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

[Stephane Eranian]

On Mon, Feb 5, 2018 at 7:17 AM, Jiri Olsa  wrote:
> On Fri, Feb 02, 2018 at 01:04:34PM -0800, Stephane Eranian wrote:
>> On Fri, Feb 2, 2018 at 12:40 PM, Arnaldo Carvalho de Melo
>>  wrote:
>> > Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo 
>> > escreveu:
>> >> Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
>> >> > 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.
>> >
>> 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.

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

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

[Stephane Eranian]

On Mon, Feb 5, 2018 at 7:17 AM, Jiri Olsa  wrote:
> On Fri, Feb 02, 2018 at 01:04:34PM -0800, Stephane Eranian wrote:
>> On Fri, Feb 2, 2018 at 12:40 PM, Arnaldo Carvalho de Melo
>>  wrote:
>> > Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo 
>> > escreveu:
>> >> Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
>> >> > 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.
>> >
>> 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.

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

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

[Jiri Olsa]

On Fri, Feb 02, 2018 at 01:04:34PM -0800, Stephane Eranian wrote:
> On Fri, Feb 2, 2018 at 12:40 PM, Arnaldo Carvalho de Melo
>  wrote:
> > Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo escreveu:
> >> Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
> >> > 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.
> >
> 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

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

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

[Jiri Olsa]

On Fri, Feb 02, 2018 at 01:04:34PM -0800, Stephane Eranian wrote:
> On Fri, Feb 2, 2018 at 12:40 PM, Arnaldo Carvalho de Melo
>  wrote:
> > Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo escreveu:
> >> Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
> >> > 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.
> >
> 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

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

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

[Stephane Eranian]

On Sat, Feb 3, 2018 at 7:30 AM, Jiri Olsa  wrote:
> On Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian wrote:
>> Jiri,
>>
>> On Thu, Feb 1, 2018 at 12:38 AM, Jiri Olsa  wrote:
>> > 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.
>> >
>> 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=10/ .
>>
>> 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.
>
> you can use p modifier behind like: cpu/.../pp
>
>>
>> I'd like to be able to say:
>>
>> $ perf record -e
>> cycles:pp:period=1001,cpu/event=0xd0,umaks=0x81,period=10/
>>
>> Or something equivalent.
>
> and you can specify terms for hw events like 'cycles'
>
> [jolsa@krava perf]$ sudo ./perf record -e 
> 'cycles/period=1001/pp,cpu/event=0xd0,umask=0x81,period=10/'
> ^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.

> [jolsa@krava perf]$ ./perf evlist -v
> cycles/period=1001/pp: size: 112, { sample_period, sample_freq }: 
> 1001, 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=10/: type: 4, size: 112, config: 0x81d0, 
> { sample_period, sample_freq }: 10, sample_type: IP|TID|TIME|ID|CPU, 
> read_format: ID, disabled: 1, inherit: 1, sample_id_all: 1, exclude_guest: 1
>
> jirka

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

[Stephane Eranian]

On Sat, Feb 3, 2018 at 7:30 AM, Jiri Olsa  wrote:
> On Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian wrote:
>> Jiri,
>>
>> On Thu, Feb 1, 2018 at 12:38 AM, Jiri Olsa  wrote:
>> > 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.
>> >
>> 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=10/ .
>>
>> 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.
>
> you can use p modifier behind like: cpu/.../pp
>
>>
>> I'd like to be able to say:
>>
>> $ perf record -e
>> cycles:pp:period=1001,cpu/event=0xd0,umaks=0x81,period=10/
>>
>> Or something equivalent.
>
> and you can specify terms for hw events like 'cycles'
>
> [jolsa@krava perf]$ sudo ./perf record -e 
> 'cycles/period=1001/pp,cpu/event=0xd0,umask=0x81,period=10/'
> ^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.

> [jolsa@krava perf]$ ./perf evlist -v
> cycles/period=1001/pp: size: 112, { sample_period, sample_freq }: 
> 1001, 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=10/: type: 4, size: 112, config: 0x81d0, 
> { sample_period, sample_freq }: 10, sample_type: IP|TID|TIME|ID|CPU, 
> read_format: ID, disabled: 1, inherit: 1, sample_id_all: 1, exclude_guest: 1
>
> jirka

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

[Jiri Olsa]

On Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian wrote:
> Jiri,
> 
> On Thu, Feb 1, 2018 at 12:38 AM, Jiri Olsa  wrote:
> > 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.
> >
> 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=10/ .
> 
> 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.

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

> 
> I'd like to be able to say:
> 
> $ perf record -e
> cycles:pp:period=1001,cpu/event=0xd0,umaks=0x81,period=10/
> 
> Or something equivalent.

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

[jolsa@krava perf]$ sudo ./perf record -e 
'cycles/period=1001/pp,cpu/event=0xd0,umask=0x81,period=10/'
^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=1001/pp: size: 112, { sample_period, sample_freq }: 1001, 
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=10/: type: 4, size: 112, config: 0x81d0, { 
sample_period, sample_freq }: 10, sample_type: IP|TID|TIME|ID|CPU, 
read_format: ID, disabled: 1, inherit: 1, sample_id_all: 1, exclude_guest: 1

jirka

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

[Jiri Olsa]

On Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian wrote:
> Jiri,
> 
> On Thu, Feb 1, 2018 at 12:38 AM, Jiri Olsa  wrote:
> > 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.
> >
> 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=10/ .
> 
> 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.

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

> 
> I'd like to be able to say:
> 
> $ perf record -e
> cycles:pp:period=1001,cpu/event=0xd0,umaks=0x81,period=10/
> 
> Or something equivalent.

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

[jolsa@krava perf]$ sudo ./perf record -e 
'cycles/period=1001/pp,cpu/event=0xd0,umask=0x81,period=10/'
^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=1001/pp: size: 112, { sample_period, sample_freq }: 1001, 
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=10/: type: 4, size: 112, config: 0x81d0, { 
sample_period, sample_freq }: 10, sample_type: IP|TID|TIME|ID|CPU, 
read_format: ID, disabled: 1, inherit: 1, sample_id_all: 1, exclude_guest: 1

jirka

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

[Stephane Eranian]

On Fri, Feb 2, 2018 at 12:40 PM, Arnaldo Carvalho de Melo
 wrote:
> Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
>> > 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.
>
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.

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

[Stephane Eranian]

On Fri, Feb 2, 2018 at 12:40 PM, Arnaldo Carvalho de Melo
 wrote:
> Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
>> > 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.
>
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.

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

[Arnaldo Carvalho de Melo]

Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
> > 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

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

[Arnaldo Carvalho de Melo]

Em Fri, Feb 02, 2018 at 05:28:49PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
> > 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

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

[Arnaldo Carvalho de Melo]

Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
> Jiri,
> 
> On Thu, Feb 1, 2018 at 12:38 AM, Jiri Olsa  wrote:
> > 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.
> >
> 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=10/ .
> 
> 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=1001,cpu/event=0xd0,umaks=0x81,period=10/
> 
> 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
 
> Thanks.
> 
> 
> > Reported-by: Stephane Eranian 
> > Link: http://lkml.kernel.org/n/tip-anrtntkwfto5rqulegfwi...@git.kernel.org
> > Signed-off-by: Jiri Olsa 
> > ---
> >  tools/perf/util/evsel.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index 66fa45198a11..f2f2eaafde6d 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -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
> >

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

[Arnaldo Carvalho de Melo]

Em Fri, Feb 02, 2018 at 10:45:46AM -0800, Stephane Eranian escreveu:
> Jiri,
> 
> On Thu, Feb 1, 2018 at 12:38 AM, Jiri Olsa  wrote:
> > 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.
> >
> 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=10/ .
> 
> 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=1001,cpu/event=0xd0,umaks=0x81,period=10/
> 
> 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
 
> Thanks.
> 
> 
> > Reported-by: Stephane Eranian 
> > Link: http://lkml.kernel.org/n/tip-anrtntkwfto5rqulegfwi...@git.kernel.org
> > Signed-off-by: Jiri Olsa 
> > ---
> >  tools/perf/util/evsel.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index 66fa45198a11..f2f2eaafde6d 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -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
> >

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

[Stephane Eranian]

Jiri,

On Thu, Feb 1, 2018 at 12:38 AM, Jiri Olsa  wrote:
> 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.
>
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=10/ .

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=1001,cpu/event=0xd0,umaks=0x81,period=10/

Or something equivalent.

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

Thanks.


> Reported-by: Stephane Eranian 
> Link: http://lkml.kernel.org/n/tip-anrtntkwfto5rqulegfwi...@git.kernel.org
> Signed-off-by: Jiri Olsa 
> ---
>  tools/perf/util/evsel.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 66fa45198a11..f2f2eaafde6d 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -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
>

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

[Stephane Eranian]

Jiri,

On Thu, Feb 1, 2018 at 12:38 AM, Jiri Olsa  wrote:
> 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.
>
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=10/ .

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=1001,cpu/event=0xd0,umaks=0x81,period=10/

Or something equivalent.

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

Thanks.


> Reported-by: Stephane Eranian 
> Link: http://lkml.kernel.org/n/tip-anrtntkwfto5rqulegfwi...@git.kernel.org
> Signed-off-by: Jiri Olsa 
> ---
>  tools/perf/util/evsel.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 66fa45198a11..f2f2eaafde6d 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -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
>