From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753732AbbICMNT (ORCPT ); Thu, 3 Sep 2015 08:13:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43922 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752004AbbICMNS (ORCPT ); Thu, 3 Sep 2015 08:13:18 -0400 Date: Thu, 3 Sep 2015 14:13:10 +0200 From: Jiri Olsa To: Stephane Eranian Cc: LKML , Arnaldo Carvalho de Melo , Peter Zijlstra , "mingo@elte.hu" , "ak@linux.intel.com" , Namhyung Kim , "Liang, Kan" , David Ahern , Adrian Hunter Subject: Re: [PATCH] perf stat: fix per-pkg event reporting bug Message-ID: <20150903121310.GG13114@krava.redhat.com> References: <1441199871-7384-1-git-send-email-eranian@google.com> <20150903100134.GD13114@krava.redhat.com> <20150903120133.GF13114@krava.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Sep 03, 2015 at 05:05:32AM -0700, Stephane Eranian wrote: > On Thu, Sep 3, 2015 at 5:01 AM, Jiri Olsa 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 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; }