* [PATCH 0/3] perf: PEBS/period freerunning fixes @ 2018-02-01 8:38 Jiri Olsa 2018-02-01 8:38 ` [PATCH 1/3] perf tools: Fix period/freq terms setup Jiri Olsa ` (2 more replies) 0 siblings, 3 replies; 25+ messages in thread From: Jiri Olsa @ 2018-02-01 8:38 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra, Stephane Eranian 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(-) ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-01 8:38 [PATCH 0/3] perf: PEBS/period freerunning fixes Jiri Olsa @ 2018-02-01 8:38 ` Jiri Olsa 2018-02-02 18:45 ` Stephane Eranian 2018-02-05 21:35 ` [tip:perf/urgent] perf evsel: " tip-bot for Jiri Olsa 2018-02-01 8:38 ` [PATCH 2/3] perf record: Fix period option handling Jiri Olsa 2018-02-01 8:38 ` [PATCH 3/3] x86/events/intel/ds: Add PERF_SAMPLE_PERIOD into PEBS_FREERUNNING_FLAGS Jiri Olsa 2 siblings, 2 replies; 25+ messages in thread From: Jiri Olsa @ 2018-02-01 8:38 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra, Stephane Eranian 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. Reported-by: Stephane Eranian <eranian@google.com> Link: http://lkml.kernel.org/n/tip-anrtntkwfto5rqulegfwitn5@git.kernel.org Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- 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 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-01 8:38 ` [PATCH 1/3] perf tools: Fix period/freq terms setup Jiri Olsa @ 2018-02-02 18:45 ` Stephane Eranian 2018-02-02 20:28 ` Arnaldo Carvalho de Melo 2018-02-03 15:30 ` [PATCH 1/3] perf tools: Fix period/freq terms setup Jiri Olsa 2018-02-05 21:35 ` [tip:perf/urgent] perf evsel: " tip-bot for Jiri Olsa 1 sibling, 2 replies; 25+ messages in thread From: Stephane Eranian @ 2018-02-02 18:45 UTC (permalink / raw) To: Jiri Olsa Cc: Arnaldo Carvalho de Melo, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra Jiri, On Thu, Feb 1, 2018 at 12:38 AM, Jiri Olsa <jolsa@kernel.org> 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=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. > Reported-by: Stephane Eranian <eranian@google.com> > Link: http://lkml.kernel.org/n/tip-anrtntkwfto5rqulegfwitn5@git.kernel.org > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > 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 > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-02 18:45 ` Stephane Eranian @ 2018-02-02 20:28 ` Arnaldo Carvalho de Melo 2018-02-02 20:40 ` Arnaldo Carvalho de Melo 2018-02-03 15:30 ` [PATCH 1/3] perf tools: Fix period/freq terms setup Jiri Olsa 1 sibling, 1 reply; 25+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-02-02 20:28 UTC (permalink / raw) To: Stephane Eranian Cc: Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra 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 <jolsa@kernel.org> 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=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. 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 <eranian@google.com> > > Link: http://lkml.kernel.org/n/tip-anrtntkwfto5rqulegfwitn5@git.kernel.org > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > 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 > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-02 20:28 ` Arnaldo Carvalho de Melo @ 2018-02-02 20:40 ` Arnaldo Carvalho de Melo 2018-02-02 21:04 ` Stephane Eranian 0 siblings, 1 reply; 25+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-02-02 20:40 UTC (permalink / raw) To: Stephane Eranian Cc: Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-02 20:40 ` Arnaldo Carvalho de Melo @ 2018-02-02 21:04 ` Stephane Eranian 2018-02-05 15:17 ` Jiri Olsa 0 siblings, 1 reply; 25+ messages in thread From: Stephane Eranian @ 2018-02-02 21:04 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra On Fri, Feb 2, 2018 at 12:40 PM, Arnaldo Carvalho de Melo <acme@kernel.org> 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. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-02 21:04 ` Stephane Eranian @ 2018-02-05 15:17 ` Jiri Olsa 2018-02-05 20:58 ` Stephane Eranian 0 siblings, 1 reply; 25+ messages in thread From: Jiri Olsa @ 2018-02-05 15:17 UTC (permalink / raw) To: Stephane Eranian Cc: Arnaldo Carvalho de Melo, Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra 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 > <acme@kernel.org> 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. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-05 15:17 ` Jiri Olsa @ 2018-02-05 20:58 ` Stephane Eranian 2018-02-05 21:13 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 25+ messages in thread From: Stephane Eranian @ 2018-02-05 20:58 UTC (permalink / raw) To: Jiri Olsa Cc: Arnaldo Carvalho de Melo, Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra On Mon, Feb 5, 2018 at 7:17 AM, Jiri Olsa <jolsa@redhat.com> 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 >> <acme@kernel.org> 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. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-05 20:58 ` Stephane Eranian @ 2018-02-05 21:13 ` Arnaldo Carvalho de Melo 2018-02-06 2:51 ` Stephane Eranian 0 siblings, 1 reply; 25+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-02-05 21:13 UTC (permalink / raw) To: Stephane Eranian Cc: Jiri Olsa, Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra Em Mon, Feb 05, 2018 at 12:58:16PM -0800, Stephane Eranian escreveu: > On Mon, Feb 5, 2018 at 7:17 AM, Jiri Olsa <jolsa@redhat.com> 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 > >> <acme@kernel.org> 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 <adrian.hunter@intel.com> 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 <adrian.hunter@intel.com> Tested-by: Stephane Eranian <eranian@google.com> Acked-by: Peter Zijlstra <peterz@infradead.org> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-05 21:13 ` Arnaldo Carvalho de Melo @ 2018-02-06 2:51 ` Stephane Eranian 2018-02-06 9:35 ` Jiri Olsa 0 siblings, 1 reply; 25+ messages in thread From: Stephane Eranian @ 2018-02-06 2:51 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Jiri Olsa, Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra On Mon, Feb 5, 2018 at 1:13 PM, Arnaldo Carvalho de Melo <acme@kernel.org> 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 <jolsa@redhat.com> 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 >> >> <acme@kernel.org> 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 <adrian.hunter@intel.com> > 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 <adrian.hunter@intel.com> > Tested-by: Stephane Eranian <eranian@google.com> > Acked-by: Peter Zijlstra <peterz@infradead.org> > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-06 2:51 ` Stephane Eranian @ 2018-02-06 9:35 ` Jiri Olsa 2018-02-07 18:52 ` Stephane Eranian 0 siblings, 1 reply; 25+ messages in thread From: Jiri Olsa @ 2018-02-06 9:35 UTC (permalink / raw) To: Stephane Eranian Cc: Arnaldo Carvalho de Melo, Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra 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 [<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; ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-06 9:35 ` Jiri Olsa @ 2018-02-07 18:52 ` Stephane Eranian 2018-02-09 9:27 ` [PATCH] perf report: Add support to display group output for non group events Jiri Olsa 0 siblings, 1 reply; 25+ messages in thread From: Stephane Eranian @ 2018-02-07 18:52 UTC (permalink / raw) To: Jiri Olsa Cc: Arnaldo Carvalho de Melo, Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra On Tue, Feb 6, 2018 at 1:35 AM, Jiri Olsa <jolsa@redhat.com> 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 <eranian@google.com> > 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 [<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; > ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] perf report: Add support to display group output for non group events 2018-02-07 18:52 ` Stephane Eranian @ 2018-02-09 9:27 ` Jiri Olsa 2018-02-09 18:37 ` Arnaldo Carvalho de Melo 2018-02-17 11:23 ` [tip:perf/core] " tip-bot for Jiri Olsa 0 siblings, 2 replies; 25+ messages in thread From: Jiri Olsa @ 2018-02-09 9:27 UTC (permalink / raw) To: Stephane Eranian Cc: Arnaldo Carvalho de Melo, Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra On Wed, Feb 07, 2018 at 10:52:35AM -0800, Stephane Eranian wrote: SNIP > >> 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 <eranian@google.com> 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 ... Requested-and-Tested-by: Stephane Eranian <eranian@google.com> Link: http://lkml.kernel.org/n/tip-m1ffikw8c3a55b3uaxrmk5w3@git.kernel.org Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/perf/Documentation/perf-report.txt | 3 ++- tools/perf/builtin-report.c | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt index 907e505b6309..a76b871f78a6 100644 --- a/tools/perf/Documentation/perf-report.txt +++ b/tools/perf/Documentation/perf-report.txt @@ -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, diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 8ef71669e7a0..1eedb1815c4c 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -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; -- 2.13.6 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH] perf report: Add support to display group output for non group events 2018-02-09 9:27 ` [PATCH] perf report: Add support to display group output for non group events Jiri Olsa @ 2018-02-09 18:37 ` Arnaldo Carvalho de Melo 2018-02-09 18:43 ` Arnaldo Carvalho de Melo 2018-02-09 19:10 ` Jiri Olsa 2018-02-17 11:23 ` [tip:perf/core] " tip-bot for Jiri Olsa 1 sibling, 2 replies; 25+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-02-09 18:37 UTC (permalink / raw) To: Jiri Olsa Cc: Stephane Eranian, Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra Em Fri, Feb 09, 2018 at 10:27:34AM +0100, Jiri Olsa escreveu: > On Wed, Feb 07, 2018 at 10:52:35AM -0800, Stephane Eranian wrote: > > SNIP > > > >> 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 <eranian@google.com> > > 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 > > --- > 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 > ... > > Requested-and-Tested-by: Stephane Eranian <eranian@google.com> > Link: http://lkml.kernel.org/n/tip-m1ffikw8c3a55b3uaxrmk5w3@git.kernel.org > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/perf/Documentation/perf-report.txt | 3 ++- > tools/perf/builtin-report.c | 6 +++++- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt > index 907e505b6309..a76b871f78a6 100644 > --- a/tools/perf/Documentation/perf-report.txt > +++ b/tools/perf/Documentation/perf-report.txt > @@ -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, > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c > index 8ef71669e7a0..1eedb1815c4c 100644 > --- a/tools/perf/builtin-report.c > +++ b/tools/perf/builtin-report.c > @@ -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; > > -- > 2.13.6 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] perf report: Add support to display group output for non group events 2018-02-09 18:37 ` Arnaldo Carvalho de Melo @ 2018-02-09 18:43 ` Arnaldo Carvalho de Melo 2018-02-09 19:10 ` Jiri Olsa 1 sibling, 0 replies; 25+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-02-09 18:43 UTC (permalink / raw) To: Jiri Olsa Cc: Stephane Eranian, Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra Em Fri, Feb 09, 2018 at 03:37:11PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Fri, Feb 09, 2018 at 10:27:34AM +0100, Jiri Olsa escreveu: > > On Wed, Feb 07, 2018 at 10:52:35AM -0800, Stephane Eranian wrote: > > > > SNIP > > > > > >> 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 <eranian@google.com> > > > > 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 Till then, that documentation hunk should help understanding this issue. :-) - Arnaldo ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] perf report: Add support to display group output for non group events 2018-02-09 18:37 ` Arnaldo Carvalho de Melo 2018-02-09 18:43 ` Arnaldo Carvalho de Melo @ 2018-02-09 19:10 ` Jiri Olsa 2018-02-09 19:12 ` Arnaldo Carvalho de Melo 1 sibling, 1 reply; 25+ messages in thread From: Jiri Olsa @ 2018-02-09 19:10 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Stephane Eranian, Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra On Fri, Feb 09, 2018 at 03:37:11PM -0300, Arnaldo Carvalho de Melo wrote: > Em Fri, Feb 09, 2018 at 10:27:34AM +0100, Jiri Olsa escreveu: > > On Wed, Feb 07, 2018 at 10:52:35AM -0800, Stephane Eranian wrote: > > > > SNIP > > > > > >> 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 <eranian@google.com> > > > > 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 right, I'll try to post something like that thanks, jirka ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] perf report: Add support to display group output for non group events 2018-02-09 19:10 ` Jiri Olsa @ 2018-02-09 19:12 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 25+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-02-09 19:12 UTC (permalink / raw) To: Jiri Olsa Cc: Stephane Eranian, Jiri Olsa, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra Em Fri, Feb 09, 2018 at 08:10:57PM +0100, Jiri Olsa escreveu: > On Fri, Feb 09, 2018 at 03:37:11PM -0300, Arnaldo Carvalho de Melo wrote: > > 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... > > 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [tip:perf/core] perf report: Add support to display group output for non group events 2018-02-09 9:27 ` [PATCH] perf report: Add support to display group output for non group events Jiri Olsa 2018-02-09 18:37 ` Arnaldo Carvalho de Melo @ 2018-02-17 11:23 ` tip-bot for Jiri Olsa 1 sibling, 0 replies; 25+ messages in thread From: tip-bot for Jiri Olsa @ 2018-02-17 11:23 UTC (permalink / raw) To: linux-tip-commits Cc: namhyung, jolsa, linux-kernel, mingo, peterz, ak, dsahern, eranian, alexander.shishkin, acme, tglx, hpa, jolsa Commit-ID: ad52b8cb4886f572b147b02f4c59a648bbf05f9c Gitweb: https://git.kernel.org/tip/ad52b8cb4886f572b147b02f4c59a648bbf05f9c Author: Jiri Olsa <jolsa@redhat.com> AuthorDate: Fri, 9 Feb 2018 10:27:34 +0100 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> 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... Requested-by: Stephane Eranian <eranian@google.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com> Tested-by: Stephane Eranian <eranian@google.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Andi Kleen <ak@linux.intel.com> Cc: David Ahern <dsahern@gmail.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lkml.kernel.org/r/20180209092734.GB20449@krava Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/Documentation/perf-report.txt | 3 ++- tools/perf/builtin-report.c | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt index 907e505..a76b871 100644 --- a/tools/perf/Documentation/perf-report.txt +++ b/tools/perf/Documentation/perf-report.txt @@ -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, diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 8ef7166..1eedb18 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -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; ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-02 18:45 ` Stephane Eranian 2018-02-02 20:28 ` Arnaldo Carvalho de Melo @ 2018-02-03 15:30 ` Jiri Olsa 2018-02-04 0:19 ` Stephane Eranian 1 sibling, 1 reply; 25+ messages in thread From: Jiri Olsa @ 2018-02-03 15:30 UTC (permalink / raw) To: Stephane Eranian Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra 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 <jolsa@kernel.org> 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=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. you can use p modifier behind like: cpu/.../pp > > 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] perf tools: Fix period/freq terms setup 2018-02-03 15:30 ` [PATCH 1/3] perf tools: Fix period/freq terms setup Jiri Olsa @ 2018-02-04 0:19 ` Stephane Eranian 0 siblings, 0 replies; 25+ messages in thread From: Stephane Eranian @ 2018-02-04 0:19 UTC (permalink / raw) To: Jiri Olsa Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra On Sat, Feb 3, 2018 at 7:30 AM, Jiri Olsa <jolsa@redhat.com> 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 <jolsa@kernel.org> 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=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. > > you can use p modifier behind like: cpu/.../pp > >> >> 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) ] > 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=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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [tip:perf/urgent] perf evsel: Fix period/freq terms setup 2018-02-01 8:38 ` [PATCH 1/3] perf tools: Fix period/freq terms setup Jiri Olsa 2018-02-02 18:45 ` Stephane Eranian @ 2018-02-05 21:35 ` tip-bot for Jiri Olsa 1 sibling, 0 replies; 25+ messages in thread From: tip-bot for Jiri Olsa @ 2018-02-05 21:35 UTC (permalink / raw) To: linux-tip-commits Cc: mingo, namhyung, linux-kernel, alexander.shishkin, peterz, dsahern, hpa, jolsa, eranian, acme, ak, tglx Commit-ID: 49c0ae80eb32426fa133246200628e529067c595 Gitweb: https://git.kernel.org/tip/49c0ae80eb32426fa133246200628e529067c595 Author: Jiri Olsa <jolsa@kernel.org> AuthorDate: Thu, 1 Feb 2018 09:38:10 +0100 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> 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. Reported-by: Stephane Eranian <eranian@google.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org> Tested-by: Stephane Eranian <eranian@google.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Andi Kleen <ak@linux.intel.com> Cc: David Ahern <dsahern@gmail.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lkml.kernel.org/r/20180201083812.11359-2-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- 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 66fa451..f2f2eaa 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: ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/3] perf record: Fix period option handling 2018-02-01 8:38 [PATCH 0/3] perf: PEBS/period freerunning fixes Jiri Olsa 2018-02-01 8:38 ` [PATCH 1/3] perf tools: Fix period/freq terms setup Jiri Olsa @ 2018-02-01 8:38 ` Jiri Olsa 2018-02-05 21:36 ` [tip:perf/urgent] " tip-bot for Jiri Olsa 2018-02-01 8:38 ` [PATCH 3/3] x86/events/intel/ds: Add PERF_SAMPLE_PERIOD into PEBS_FREERUNNING_FLAGS Jiri Olsa 2 siblings, 1 reply; 25+ messages in thread From: Jiri Olsa @ 2018-02-01 8:38 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra, Stephane Eranian 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. Reported-by: Stephane Eranian <eranian@google.com> Link: http://lkml.kernel.org/n/tip-b89ggszz9x26hm7ztor29b0s@git.kernel.org Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/perf/builtin-record.c | 3 ++- tools/perf/perf.h | 1 + tools/perf/util/evsel.c | 11 ++++++++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index f251e824edac..907267206973 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -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, diff --git a/tools/perf/perf.h b/tools/perf/perf.h index 2357f4ccc9c7..cfe46236a5e5 100644 --- a/tools/perf/perf.h +++ b/tools/perf/perf.h @@ -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; diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index f2f2eaafde6d..ff359c9ece2e 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -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) -- 2.13.6 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [tip:perf/urgent] perf record: Fix period option handling 2018-02-01 8:38 ` [PATCH 2/3] perf record: Fix period option handling Jiri Olsa @ 2018-02-05 21:36 ` tip-bot for Jiri Olsa 0 siblings, 0 replies; 25+ messages in thread From: tip-bot for Jiri Olsa @ 2018-02-05 21:36 UTC (permalink / raw) To: linux-tip-commits Cc: jolsa, namhyung, linux-kernel, hpa, peterz, tglx, acme, mingo, alexander.shishkin, dsahern, eranian, ak Commit-ID: f290aa1ffa45ed7e37599840878b4dae68269ee1 Gitweb: https://git.kernel.org/tip/f290aa1ffa45ed7e37599840878b4dae68269ee1 Author: Jiri Olsa <jolsa@kernel.org> AuthorDate: Thu, 1 Feb 2018 09:38:11 +0100 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> 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 ~]# Reported-by: Stephane Eranian <eranian@google.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com> Tested-by: Stephane Eranian <eranian@google.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Andi Kleen <ak@linux.intel.com> Cc: David Ahern <dsahern@gmail.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lkml.kernel.org/r/20180201083812.11359-3-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/builtin-record.c | 3 ++- tools/perf/perf.h | 1 + tools/perf/util/evsel.c | 11 ++++++++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 65681a1..bf4ca74 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -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, diff --git a/tools/perf/perf.h b/tools/perf/perf.h index 2357f4cc..cfe4623 100644 --- a/tools/perf/perf.h +++ b/tools/perf/perf.h @@ -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; diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index f2f2eaa..ff359c9 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -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) ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/3] x86/events/intel/ds: Add PERF_SAMPLE_PERIOD into PEBS_FREERUNNING_FLAGS 2018-02-01 8:38 [PATCH 0/3] perf: PEBS/period freerunning fixes Jiri Olsa 2018-02-01 8:38 ` [PATCH 1/3] perf tools: Fix period/freq terms setup Jiri Olsa 2018-02-01 8:38 ` [PATCH 2/3] perf record: Fix period option handling Jiri Olsa @ 2018-02-01 8:38 ` Jiri Olsa 2018-02-05 21:36 ` [tip:perf/urgent] " tip-bot for Jiri Olsa 2 siblings, 1 reply; 25+ messages in thread From: Jiri Olsa @ 2018-02-01 8:38 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: lkml, Ingo Molnar, Namhyung Kim, David Ahern, Andi Kleen, Alexander Shishkin, Peter Zijlstra, Stephane Eranian 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. Reported-by: Stephane Eranian <eranian@google.com> Link: http://lkml.kernel.org/n/tip-hx5yqua1c9u7iewm3h8frq5h@git.kernel.org Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- arch/x86/events/perf_event.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h index 8e4ea143ed96..78f91ec1056e 100644 --- a/arch/x86/events/perf_event.h +++ b/arch/x86/events/perf_event.h @@ -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 | \ -- 2.13.6 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [tip:perf/urgent] x86/events/intel/ds: Add PERF_SAMPLE_PERIOD into PEBS_FREERUNNING_FLAGS 2018-02-01 8:38 ` [PATCH 3/3] x86/events/intel/ds: Add PERF_SAMPLE_PERIOD into PEBS_FREERUNNING_FLAGS Jiri Olsa @ 2018-02-05 21:36 ` tip-bot for Jiri Olsa 0 siblings, 0 replies; 25+ messages in thread From: tip-bot for Jiri Olsa @ 2018-02-05 21:36 UTC (permalink / raw) To: linux-tip-commits Cc: jolsa, kan.liang, linux-kernel, hpa, mingo, tglx, ak, alexander.shishkin, peterz, dsahern, acme, eranian, namhyung Commit-ID: 11974914e8e6d44387b4b23fa6dbd40c94baeb8d Gitweb: https://git.kernel.org/tip/11974914e8e6d44387b4b23fa6dbd40c94baeb8d Author: Jiri Olsa <jolsa@kernel.org> AuthorDate: Thu, 1 Feb 2018 09:38:12 +0100 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> 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 Reported-by: Stephane Eranian <eranian@google.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org> Tested-by: Kan Liang <kan.liang@intel.com> Tested-by: Stephane Eranian <eranian@google.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Andi Kleen <ak@linux.intel.com> Cc: David Ahern <dsahern@gmail.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lkml.kernel.org/r/20180201083812.11359-4-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- arch/x86/events/perf_event.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h index 8e4ea143..78f91ec 100644 --- a/arch/x86/events/perf_event.h +++ b/arch/x86/events/perf_event.h @@ -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 | \ ^ permalink raw reply related [flat|nested] 25+ messages in thread
end of thread, other threads:[~2018-02-17 11:33 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-01 8:38 [PATCH 0/3] perf: PEBS/period freerunning fixes Jiri Olsa 2018-02-01 8:38 ` [PATCH 1/3] perf tools: Fix period/freq terms setup Jiri Olsa 2018-02-02 18:45 ` Stephane Eranian 2018-02-02 20:28 ` Arnaldo Carvalho de Melo 2018-02-02 20:40 ` Arnaldo Carvalho de Melo 2018-02-02 21:04 ` Stephane Eranian 2018-02-05 15:17 ` Jiri Olsa 2018-02-05 20:58 ` Stephane Eranian 2018-02-05 21:13 ` Arnaldo Carvalho de Melo 2018-02-06 2:51 ` Stephane Eranian 2018-02-06 9:35 ` Jiri Olsa 2018-02-07 18:52 ` Stephane Eranian 2018-02-09 9:27 ` [PATCH] perf report: Add support to display group output for non group events Jiri Olsa 2018-02-09 18:37 ` Arnaldo Carvalho de Melo 2018-02-09 18:43 ` Arnaldo Carvalho de Melo 2018-02-09 19:10 ` Jiri Olsa 2018-02-09 19:12 ` Arnaldo Carvalho de Melo 2018-02-17 11:23 ` [tip:perf/core] " tip-bot for Jiri Olsa 2018-02-03 15:30 ` [PATCH 1/3] perf tools: Fix period/freq terms setup Jiri Olsa 2018-02-04 0:19 ` Stephane Eranian 2018-02-05 21:35 ` [tip:perf/urgent] perf evsel: " tip-bot for Jiri Olsa 2018-02-01 8:38 ` [PATCH 2/3] perf record: Fix period option handling Jiri Olsa 2018-02-05 21:36 ` [tip:perf/urgent] " tip-bot for Jiri Olsa 2018-02-01 8:38 ` [PATCH 3/3] x86/events/intel/ds: Add PERF_SAMPLE_PERIOD into PEBS_FREERUNNING_FLAGS Jiri Olsa 2018-02-05 21:36 ` [tip:perf/urgent] " tip-bot for Jiri Olsa
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.