Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1217598 > unrolled thread

[PATCH] perf stat: fix per-pkg event reporting bug

Started byStephane Eranian <eranian@google.com>
First post2015-09-02 15:20 +0200
Last post2015-09-03 23:40 +0200
Articles 14 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [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

#1217598 — [PATCH] perf stat: fix per-pkg event reporting bug

FromStephane Eranian <eranian@google.com>
Date2015-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]


#1217844

FromAndi Kleen <ak@linux.intel.com>
Date2015-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]


#1218138

FromJiri Olsa <jolsa@redhat.com>
Date2015-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]


#1218137

FromJiri Olsa <jolsa@redhat.com>
Date2015-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]


#1218187

FromStephane Eranian <eranian@google.com>
Date2015-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]


#1218190

FromJiri Olsa <jolsa@redhat.com>
Date2015-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]


#1218191

FromStephane Eranian <eranian@google.com>
Date2015-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]


#1218206

FromJiri Olsa <jolsa@redhat.com>
Date2015-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]


#1218210

FromStephane Eranian <eranian@google.com>
Date2015-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]


#1218220

FromJiri Olsa <jolsa@redhat.com>
Date2015-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]


#1218443

FromArnaldo Carvalho de Melo <acme@redhat.com>
Date2015-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]


#1218449

FromStephane Eranian <eranian@google.com>
Date2015-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]


#1218565

FromArnaldo Carvalho de Melo <acme@redhat.com>
Date2015-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]


#1218589

FromStephane Eranian <eranian@google.com>
Date2015-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