From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754805AbbICMZx (ORCPT ); Thu, 3 Sep 2015 08:25:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46804 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752636AbbICMZu (ORCPT ); Thu, 3 Sep 2015 08:25:50 -0400 Date: Thu, 3 Sep 2015 14:25:44 +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: <20150903122544.GH13114@krava.redhat.com> References: <1441199871-7384-1-git-send-email-eranian@google.com> <20150903100134.GD13114@krava.redhat.com> <20150903120133.GF13114@krava.redhat.com> <20150903121310.GG13114@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:16:41AM -0700, Stephane Eranian wrote: > On Thu, Sep 3, 2015 at 5:13 AM, Jiri Olsa 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 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' > > > 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