Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1217598 > unrolled thread
| Started by | Stephane Eranian <eranian@google.com> |
|---|---|
| First post | 2015-09-02 15:20 +0200 |
| Last post | 2015-09-03 23:40 +0200 |
| Articles | 14 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] perf stat: fix per-pkg event reporting bug Stephane Eranian <eranian@google.com> - 2015-09-02 15:20 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Andi Kleen <ak@linux.intel.com> - 2015-09-02 22:30 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Jiri Olsa <jolsa@redhat.com> - 2015-09-03 12:10 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Jiri Olsa <jolsa@redhat.com> - 2015-09-03 12:10 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Stephane Eranian <eranian@google.com> - 2015-09-03 13:50 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Jiri Olsa <jolsa@redhat.com> - 2015-09-03 14:10 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Stephane Eranian <eranian@google.com> - 2015-09-03 14:10 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Jiri Olsa <jolsa@redhat.com> - 2015-09-03 14:20 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Stephane Eranian <eranian@google.com> - 2015-09-03 14:20 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Jiri Olsa <jolsa@redhat.com> - 2015-09-03 14:30 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Arnaldo Carvalho de Melo <acme@redhat.com> - 2015-09-03 19:00 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Stephane Eranian <eranian@google.com> - 2015-09-03 19:20 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Arnaldo Carvalho de Melo <acme@redhat.com> - 2015-09-03 22:50 +0200
Re: [PATCH] perf stat: fix per-pkg event reporting bug Stephane Eranian <eranian@google.com> - 2015-09-03 23:40 +0200
| From | Stephane Eranian <eranian@google.com> |
|---|---|
| Date | 2015-09-02 15:20 +0200 |
| Subject | [PATCH] perf stat: fix per-pkg event reporting bug |
| Message-ID | <q4g7g-683-21@gated-at.bofh.it> |
Per-pkg events need to be captured once per processor
socket. The code in check_per_pkg() ensures only one
value per processor package is used. However there is
a problem with this function in case the first CPU of
the package does not measure anything for the per-pkg event,
but other CPUs do.
Consider the following
$ create cgroup FOO; echo $$ >FOO/tasks; taskset -c 1 noploop &
$ perf stat -a -I 1000 -e intel_cqm/llc_occupancy/ -G FOO sleep 100
1.00000 <not counted> Bytes intel_cqm/llc_occupancy/ FOO
The reason for this is that CPU0 in the cgrop has nothing running on it.
Yet check_per_plg() will mark socket0 as processed and no other event
value will be considered for the socket.
This patch fixes the problem by having check_per_pkg() only consider
events which actually ran.
Patch is relative to tip.git.
Signed-off-by: Stephane Eranian <eranian@google.com>
---
tools/perf/util/stat.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
index 415c359..f1d8359 100644
--- a/tools/perf/util/stat.c
+++ b/tools/perf/util/stat.c
@@ -196,7 +196,8 @@ static void zero_per_pkg(struct perf_evsel *counter)
memset(counter->per_pkg_mask, 0, MAX_NR_CPUS);
}
-static int check_per_pkg(struct perf_evsel *counter, int cpu, bool *skip)
+static int check_per_pkg(struct perf_evsel *counter,
+ struct perf_counts_values *vals, int cpu, bool *skip)
{
unsigned long *mask = counter->per_pkg_mask;
struct cpu_map *cpus = perf_evsel__cpus(counter);
@@ -218,6 +219,17 @@ static int check_per_pkg(struct perf_evsel *counter, int cpu, bool *skip)
counter->per_pkg_mask = mask;
}
+ /*
+ * we do not consider an event that has not run as a good
+ * instance to mark a package as used (skip=1). Otherwise
+ * we may run into a situation where the first CPU in a package
+ * is not running anything, yet the second is, and this function
+ * would mark the package as used after the first CPU and would
+ * not read the values from the second CPU.
+ */
+ if (!(vals->run && vals->ena))
+ return 0;
+
s = cpu_map__get_socket(cpus, cpu);
if (s < 0)
return -1;
@@ -235,7 +247,7 @@ process_counter_values(struct perf_stat_config *config, struct perf_evsel *evsel
static struct perf_counts_values zero;
bool skip = false;
- if (check_per_pkg(evsel, cpu, &skip)) {
+ if (check_per_pkg(evsel, aggr, cpu, &skip)) {
pr_err("failed to read per-pkg counter\n");
return -1;
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Andi Kleen <ak@linux.intel.com> |
|---|---|
| Date | 2015-09-02 22:30 +0200 |
| Message-ID | <q4mPo-7io-17@gated-at.bofh.it> |
| In reply to | #1217598 |
On Wed, Sep 02, 2015 at 03:17:51PM +0200, Stephane Eranian wrote: > Per-pkg events need to be captured once per processor > socket. The code in check_per_pkg() ensures only one > value per processor package is used. However there is > a problem with this function in case the first CPU of > the package does not measure anything for the per-pkg event, > but other CPUs do. I've seen a similar(?) bug with -C and --per-core combined. Some logical threads are not correctly accounted there either. Do you think that's another instance of such a bug? -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2015-09-03 12:10 +0200 |
| Message-ID | <q4zCW-r9-29@gated-at.bofh.it> |
| In reply to | #1217844 |
On Wed, Sep 02, 2015 at 01:26:57PM -0700, Andi Kleen wrote: > On Wed, Sep 02, 2015 at 03:17:51PM +0200, Stephane Eranian wrote: > > Per-pkg events need to be captured once per processor > > socket. The code in check_per_pkg() ensures only one > > value per processor package is used. However there is > > a problem with this function in case the first CPU of > > the package does not measure anything for the per-pkg event, > > but other CPUs do. > > I've seen a similar(?) bug with -C and --per-core combined. > Some logical threads are not correctly accounted there either. > Do you think that's another instance of such a bug? AFAICS this is related strictly to per-pkg events, is this what you were doing? jirka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2015-09-03 12:10 +0200 |
| Message-ID | <q4zCW-r9-17@gated-at.bofh.it> |
| In reply to | #1217598 |
On Wed, Sep 02, 2015 at 03:17:51PM +0200, Stephane Eranian wrote:
SNIP
> + /*
> + * we do not consider an event that has not run as a good
> + * instance to mark a package as used (skip=1). Otherwise
> + * we may run into a situation where the first CPU in a package
> + * is not running anything, yet the second is, and this function
> + * would mark the package as used after the first CPU and would
> + * not read the values from the second CPU.
> + */
> + if (!(vals->run && vals->ena))
> + return 0;
> +
> s = cpu_map__get_socket(cpus, cpu);
> if (s < 0)
> return -1;
> @@ -235,7 +247,7 @@ process_counter_values(struct perf_stat_config *config, struct perf_evsel *evsel
> static struct perf_counts_values zero;
> bool skip = false;
>
> - if (check_per_pkg(evsel, cpu, &skip)) {
> + if (check_per_pkg(evsel, aggr, cpu, &skip)) {
should we pass 'count' instead o 'aggr' ?
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Stephane Eranian <eranian@google.com> |
|---|---|
| Date | 2015-09-03 13:50 +0200 |
| Message-ID | <q4BbI-2uW-7@gated-at.bofh.it> |
| In reply to | #1218137 |
On Thu, Sep 3, 2015 at 3:01 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Wed, Sep 02, 2015 at 03:17:51PM +0200, Stephane Eranian wrote:
>
> SNIP
>
>> + /*
>> + * we do not consider an event that has not run as a good
>> + * instance to mark a package as used (skip=1). Otherwise
>> + * we may run into a situation where the first CPU in a package
>> + * is not running anything, yet the second is, and this function
>> + * would mark the package as used after the first CPU and would
>> + * not read the values from the second CPU.
>> + */
>> + if (!(vals->run && vals->ena))
>> + return 0;
>> +
>> s = cpu_map__get_socket(cpus, cpu);
>> if (s < 0)
>> return -1;
>> @@ -235,7 +247,7 @@ process_counter_values(struct perf_stat_config *config, struct perf_evsel *evsel
>> static struct perf_counts_values zero;
>> bool skip = false;
>>
>> - if (check_per_pkg(evsel, cpu, &skip)) {
>> + if (check_per_pkg(evsel, aggr, cpu, &skip)) {
>
> should we pass 'count' instead o 'aggr' ?
>
the reason I passed counts_values is in case this function needs to be
called from other places which do
not use aggr mode.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2015-09-03 14:10 +0200 |
| Message-ID | <q4Bv3-36H-5@gated-at.bofh.it> |
| In reply to | #1218187 |
On Thu, Sep 03, 2015 at 04:48:52AM -0700, Stephane Eranian wrote:
> On Thu, Sep 3, 2015 at 3:01 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> > On Wed, Sep 02, 2015 at 03:17:51PM +0200, Stephane Eranian wrote:
> >
> > SNIP
> >
> >> + /*
> >> + * we do not consider an event that has not run as a good
> >> + * instance to mark a package as used (skip=1). Otherwise
> >> + * we may run into a situation where the first CPU in a package
> >> + * is not running anything, yet the second is, and this function
> >> + * would mark the package as used after the first CPU and would
> >> + * not read the values from the second CPU.
> >> + */
> >> + if (!(vals->run && vals->ena))
> >> + return 0;
> >> +
> >> s = cpu_map__get_socket(cpus, cpu);
> >> if (s < 0)
> >> return -1;
> >> @@ -235,7 +247,7 @@ process_counter_values(struct perf_stat_config *config, struct perf_evsel *evsel
> >> static struct perf_counts_values zero;
> >> bool skip = false;
> >>
> >> - if (check_per_pkg(evsel, cpu, &skip)) {
> >> + if (check_per_pkg(evsel, aggr, cpu, &skip)) {
> >
> > should we pass 'count' instead o 'aggr' ?
> >
> the reason I passed counts_values is in case this function needs to be
> called from other places which do
> not use aggr mode.
sure, but 'aggr' is being computed within process_counter_values
process_counter_values gets 'count' argument with values read
for given cpu/thread for further processing, and it seems to
me that 'count' values should be passed to check_per_pkg
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Stephane Eranian <eranian@google.com> |
|---|---|
| Date | 2015-09-03 14:10 +0200 |
| Message-ID | <q4Bv3-36H-7@gated-at.bofh.it> |
| In reply to | #1218190 |
On Thu, Sep 3, 2015 at 5:01 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Thu, Sep 03, 2015 at 04:48:52AM -0700, Stephane Eranian wrote:
>> On Thu, Sep 3, 2015 at 3:01 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>> > On Wed, Sep 02, 2015 at 03:17:51PM +0200, Stephane Eranian wrote:
>> >
>> > SNIP
>> >
>> >> + /*
>> >> + * we do not consider an event that has not run as a good
>> >> + * instance to mark a package as used (skip=1). Otherwise
>> >> + * we may run into a situation where the first CPU in a package
>> >> + * is not running anything, yet the second is, and this function
>> >> + * would mark the package as used after the first CPU and would
>> >> + * not read the values from the second CPU.
>> >> + */
>> >> + if (!(vals->run && vals->ena))
>> >> + return 0;
>> >> +
>> >> s = cpu_map__get_socket(cpus, cpu);
>> >> if (s < 0)
>> >> return -1;
>> >> @@ -235,7 +247,7 @@ process_counter_values(struct perf_stat_config *config, struct perf_evsel *evsel
>> >> static struct perf_counts_values zero;
>> >> bool skip = false;
>> >>
>> >> - if (check_per_pkg(evsel, cpu, &skip)) {
>> >> + if (check_per_pkg(evsel, aggr, cpu, &skip)) {
>> >
>> > should we pass 'count' instead o 'aggr' ?
>> >
>> the reason I passed counts_values is in case this function needs to be
>> called from other places which do
>> not use aggr mode.
>
> sure, but 'aggr' is being computed within process_counter_values
>
> process_counter_values gets 'count' argument with values read
> for given cpu/thread for further processing, and it seems to
> me that 'count' values should be passed to check_per_pkg
>
You do not want to aggregate values, you want to look at the individual events
for each CPU because you need to look at their run/ena fields.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2015-09-03 14:20 +0200 |
| Message-ID | <q4BEL-3hX-31@gated-at.bofh.it> |
| In reply to | #1218191 |
On Thu, Sep 03, 2015 at 05:05:32AM -0700, Stephane Eranian wrote:
> On Thu, Sep 3, 2015 at 5:01 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> > On Thu, Sep 03, 2015 at 04:48:52AM -0700, Stephane Eranian wrote:
> >> On Thu, Sep 3, 2015 at 3:01 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> >> > On Wed, Sep 02, 2015 at 03:17:51PM +0200, Stephane Eranian wrote:
> >> >
> >> > SNIP
> >> >
> >> >> + /*
> >> >> + * we do not consider an event that has not run as a good
> >> >> + * instance to mark a package as used (skip=1). Otherwise
> >> >> + * we may run into a situation where the first CPU in a package
> >> >> + * is not running anything, yet the second is, and this function
> >> >> + * would mark the package as used after the first CPU and would
> >> >> + * not read the values from the second CPU.
> >> >> + */
> >> >> + if (!(vals->run && vals->ena))
> >> >> + return 0;
> >> >> +
> >> >> s = cpu_map__get_socket(cpus, cpu);
> >> >> if (s < 0)
> >> >> return -1;
> >> >> @@ -235,7 +247,7 @@ process_counter_values(struct perf_stat_config *config, struct perf_evsel *evsel
> >> >> static struct perf_counts_values zero;
> >> >> bool skip = false;
> >> >>
> >> >> - if (check_per_pkg(evsel, cpu, &skip)) {
> >> >> + if (check_per_pkg(evsel, aggr, cpu, &skip)) {
> >> >
> >> > should we pass 'count' instead o 'aggr' ?
> >> >
> >> the reason I passed counts_values is in case this function needs to be
> >> called from other places which do
> >> not use aggr mode.
> >
> > sure, but 'aggr' is being computed within process_counter_values
> >
> > process_counter_values gets 'count' argument with values read
> > for given cpu/thread for further processing, and it seems to
> > me that 'count' values should be passed to check_per_pkg
> >
> You do not want to aggregate values, you want to look at the individual events
> for each CPU because you need to look at their run/ena fields.
yes, but for 'count' not 'aggr'
jirka
---
diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
index f1d83599217b..2d065d065b67 100644
--- a/tools/perf/util/stat.c
+++ b/tools/perf/util/stat.c
@@ -247,7 +247,7 @@ process_counter_values(struct perf_stat_config *config, struct perf_evsel *evsel
static struct perf_counts_values zero;
bool skip = false;
- if (check_per_pkg(evsel, aggr, cpu, &skip)) {
+ if (check_per_pkg(evsel, count, cpu, &skip)) {
pr_err("failed to read per-pkg counter\n");
return -1;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Stephane Eranian <eranian@google.com> |
|---|---|
| Date | 2015-09-03 14:20 +0200 |
| Message-ID | <q4BEL-3hX-41@gated-at.bofh.it> |
| In reply to | #1218206 |
On Thu, Sep 3, 2015 at 5:13 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Thu, Sep 03, 2015 at 05:05:32AM -0700, Stephane Eranian wrote:
>> On Thu, Sep 3, 2015 at 5:01 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>> > On Thu, Sep 03, 2015 at 04:48:52AM -0700, Stephane Eranian wrote:
>> >> On Thu, Sep 3, 2015 at 3:01 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>> >> > On Wed, Sep 02, 2015 at 03:17:51PM +0200, Stephane Eranian wrote:
>> >> >
>> >> > SNIP
>> >> >
>> >> >> + /*
>> >> >> + * we do not consider an event that has not run as a good
>> >> >> + * instance to mark a package as used (skip=1). Otherwise
>> >> >> + * we may run into a situation where the first CPU in a package
>> >> >> + * is not running anything, yet the second is, and this function
>> >> >> + * would mark the package as used after the first CPU and would
>> >> >> + * not read the values from the second CPU.
>> >> >> + */
>> >> >> + if (!(vals->run && vals->ena))
>> >> >> + return 0;
>> >> >> +
>> >> >> s = cpu_map__get_socket(cpus, cpu);
>> >> >> if (s < 0)
>> >> >> return -1;
>> >> >> @@ -235,7 +247,7 @@ process_counter_values(struct perf_stat_config *config, struct perf_evsel *evsel
>> >> >> static struct perf_counts_values zero;
>> >> >> bool skip = false;
>> >> >>
>> >> >> - if (check_per_pkg(evsel, cpu, &skip)) {
>> >> >> + if (check_per_pkg(evsel, aggr, cpu, &skip)) {
>> >> >
>> >> > should we pass 'count' instead o 'aggr' ?
>> >> >
>> >> the reason I passed counts_values is in case this function needs to be
>> >> called from other places which do
>> >> not use aggr mode.
>> >
>> > sure, but 'aggr' is being computed within process_counter_values
>> >
>> > process_counter_values gets 'count' argument with values read
>> > for given cpu/thread for further processing, and it seems to
>> > me that 'count' values should be passed to check_per_pkg
>> >
>> You do not want to aggregate values, you want to look at the individual events
>> for each CPU because you need to look at their run/ena fields.
>
> yes, but for 'count' not 'aggr'
>
Ah, yes, sorry, has to be count and not aggr. Sent the wrong version.
Can you fix it? Or do you want me to resubmit?
> jirka
>
>
> ---
> diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
> index f1d83599217b..2d065d065b67 100644
> --- a/tools/perf/util/stat.c
> +++ b/tools/perf/util/stat.c
> @@ -247,7 +247,7 @@ process_counter_values(struct perf_stat_config *config, struct perf_evsel *evsel
> static struct perf_counts_values zero;
> bool skip = false;
>
> - if (check_per_pkg(evsel, aggr, cpu, &skip)) {
> + if (check_per_pkg(evsel, count, cpu, &skip)) {
> pr_err("failed to read per-pkg counter\n");
> return -1;
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2015-09-03 14:30 +0200 |
| Message-ID | <q4BOq-3tf-27@gated-at.bofh.it> |
| In reply to | #1218210 |
On Thu, Sep 03, 2015 at 05:16:41AM -0700, Stephane Eranian wrote:
> On Thu, Sep 3, 2015 at 5:13 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> > On Thu, Sep 03, 2015 at 05:05:32AM -0700, Stephane Eranian wrote:
> >> On Thu, Sep 3, 2015 at 5:01 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> >> > On Thu, Sep 03, 2015 at 04:48:52AM -0700, Stephane Eranian wrote:
> >> >> On Thu, Sep 3, 2015 at 3:01 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> >> >> > On Wed, Sep 02, 2015 at 03:17:51PM +0200, Stephane Eranian wrote:
> >> >> >
> >> >> > SNIP
> >> >> >
> >> >> >> + /*
> >> >> >> + * we do not consider an event that has not run as a good
> >> >> >> + * instance to mark a package as used (skip=1). Otherwise
> >> >> >> + * we may run into a situation where the first CPU in a package
> >> >> >> + * is not running anything, yet the second is, and this function
> >> >> >> + * would mark the package as used after the first CPU and would
> >> >> >> + * not read the values from the second CPU.
> >> >> >> + */
> >> >> >> + if (!(vals->run && vals->ena))
> >> >> >> + return 0;
> >> >> >> +
> >> >> >> s = cpu_map__get_socket(cpus, cpu);
> >> >> >> if (s < 0)
> >> >> >> return -1;
> >> >> >> @@ -235,7 +247,7 @@ process_counter_values(struct perf_stat_config *config, struct perf_evsel *evsel
> >> >> >> static struct perf_counts_values zero;
> >> >> >> bool skip = false;
> >> >> >>
> >> >> >> - if (check_per_pkg(evsel, cpu, &skip)) {
> >> >> >> + if (check_per_pkg(evsel, aggr, cpu, &skip)) {
> >> >> >
> >> >> > should we pass 'count' instead o 'aggr' ?
> >> >> >
> >> >> the reason I passed counts_values is in case this function needs to be
> >> >> called from other places which do
> >> >> not use aggr mode.
> >> >
> >> > sure, but 'aggr' is being computed within process_counter_values
> >> >
> >> > process_counter_values gets 'count' argument with values read
> >> > for given cpu/thread for further processing, and it seems to
> >> > me that 'count' values should be passed to check_per_pkg
> >> >
> >> You do not want to aggregate values, you want to look at the individual events
> >> for each CPU because you need to look at their run/ena fields.
> >
> > yes, but for 'count' not 'aggr'
> >
> Ah, yes, sorry, has to be count and not aggr. Sent the wrong version.
> Can you fix it? Or do you want me to resubmit?
well, Arnaldo will queue it.. leaving up to him ;-)
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@redhat.com> |
|---|---|
| Date | 2015-09-03 19:00 +0200 |
| Message-ID | <q4G1I-Xg-15@gated-at.bofh.it> |
| In reply to | #1218220 |
Em Thu, Sep 03, 2015 at 02:25:44PM +0200, Jiri Olsa escreveu: > On Thu, Sep 03, 2015 at 05:16:41AM -0700, Stephane Eranian wrote: > > On Thu, Sep 3, 2015 at 5:13 AM, Jiri Olsa <jolsa@redhat.com> wrote: > > > yes, but for 'count' not 'aggr' > > Ah, yes, sorry, has to be count and not aggr. Sent the wrong version. > > Can you fix it? Or do you want me to resubmit? > > well, Arnaldo will queue it.. leaving up to him ;-) Please resubmit, with a [PATCH v2 ...] in it, and with a v2 right before your Signed-off-by: stating what you changed, that helps when I see multiple patches, i.e. you document what was changed and I don't have to follow that many threads :-) - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Stephane Eranian <eranian@google.com> |
|---|---|
| Date | 2015-09-03 19:20 +0200 |
| Message-ID | <q4Gl4-1yZ-13@gated-at.bofh.it> |
| In reply to | #1218443 |
On Thu, Sep 3, 2015 at 9:53 AM, Arnaldo Carvalho de Melo <acme@redhat.com> wrote: > Em Thu, Sep 03, 2015 at 02:25:44PM +0200, Jiri Olsa escreveu: >> On Thu, Sep 03, 2015 at 05:16:41AM -0700, Stephane Eranian wrote: >> > On Thu, Sep 3, 2015 at 5:13 AM, Jiri Olsa <jolsa@redhat.com> wrote: >> > > yes, but for 'count' not 'aggr' > >> > Ah, yes, sorry, has to be count and not aggr. Sent the wrong version. >> > Can you fix it? Or do you want me to resubmit? >> >> well, Arnaldo will queue it.. leaving up to him ;-) > > Please resubmit, with a [PATCH v2 ...] in it, and with a v2 right > before your Signed-off-by: stating what you changed, that helps when I > see multiple patches, i.e. you document what was changed and I don't > have to follow that many threads :-) > I already sent the V2, but I forgot to state the small change. Do you want a V3? Or you add: fix perf_counter_value argument to check_per_pkg. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@redhat.com> |
|---|---|
| Date | 2015-09-03 22:50 +0200 |
| Message-ID | <q4JCj-69B-15@gated-at.bofh.it> |
| In reply to | #1218449 |
Em Thu, Sep 03, 2015 at 10:12:12AM -0700, Stephane Eranian escreveu: > On Thu, Sep 3, 2015 at 9:53 AM, Arnaldo Carvalho de Melo > <acme@redhat.com> wrote: > > Em Thu, Sep 03, 2015 at 02:25:44PM +0200, Jiri Olsa escreveu: > >> On Thu, Sep 03, 2015 at 05:16:41AM -0700, Stephane Eranian wrote: > >> > On Thu, Sep 3, 2015 at 5:13 AM, Jiri Olsa <jolsa@redhat.com> wrote: > >> > > yes, but for 'count' not 'aggr' > > > >> > Ah, yes, sorry, has to be count and not aggr. Sent the wrong version. > >> > Can you fix it? Or do you want me to resubmit? > >> > >> well, Arnaldo will queue it.. leaving up to him ;-) > > > > Please resubmit, with a [PATCH v2 ...] in it, and with a v2 right > > before your Signed-off-by: stating what you changed, that helps when I > > see multiple patches, i.e. you document what was changed and I don't > > have to follow that many threads :-) > > > I already sent the V2, but I forgot to state the small change. > Do you want a V3? Or you add: fix perf_counter_value argument to check_per_pkg. > Thanks. Ok, I can do it... - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Stephane Eranian <eranian@google.com> |
|---|---|
| Date | 2015-09-03 23:40 +0200 |
| Message-ID | <q4KoH-7iO-25@gated-at.bofh.it> |
| In reply to | #1218565 |
On Thu, Sep 3, 2015 at 1:42 PM, Arnaldo Carvalho de Melo <acme@redhat.com> wrote: > Em Thu, Sep 03, 2015 at 10:12:12AM -0700, Stephane Eranian escreveu: >> On Thu, Sep 3, 2015 at 9:53 AM, Arnaldo Carvalho de Melo >> <acme@redhat.com> wrote: >> > Em Thu, Sep 03, 2015 at 02:25:44PM +0200, Jiri Olsa escreveu: >> >> On Thu, Sep 03, 2015 at 05:16:41AM -0700, Stephane Eranian wrote: >> >> > On Thu, Sep 3, 2015 at 5:13 AM, Jiri Olsa <jolsa@redhat.com> wrote: >> >> > > yes, but for 'count' not 'aggr' >> > >> >> > Ah, yes, sorry, has to be count and not aggr. Sent the wrong version. >> >> > Can you fix it? Or do you want me to resubmit? >> >> >> >> well, Arnaldo will queue it.. leaving up to him ;-) >> > >> > Please resubmit, with a [PATCH v2 ...] in it, and with a v2 right >> > before your Signed-off-by: stating what you changed, that helps when I >> > see multiple patches, i.e. you document what was changed and I don't >> > have to follow that many threads :-) >> > >> I already sent the V2, but I forgot to state the small change. >> Do you want a V3? Or you add: fix perf_counter_value argument to check_per_pkg. >> Thanks. > > Ok, I can do it... > Thanks. > - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web