Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934150AbbLPOQL (ORCPT ); Wed, 16 Dec 2015 09:16:11 -0500 Received: from mail-wm0-f45.google.com ([74.125.82.45]:38657 "EHLO mail-wm0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932947AbbLPOQH (ORCPT ); Wed, 16 Dec 2015 09:16:07 -0500 MIME-Version: 1.0 In-Reply-To: <1450227266-2501-3-git-send-email-andi@firstfloor.org> References: <1450227266-2501-1-git-send-email-andi@firstfloor.org> <1450227266-2501-3-git-send-email-andi@firstfloor.org> Date: Wed, 16 Dec 2015 06:16:05 -0800 Message-ID: Subject: Re: [PATCH 02/10] perf, tools, stat: Force --per-core mode for .agg-per-core aliases From: Stephane Eranian To: Andi Kleen Cc: Arnaldo Carvalho de Melo , Peter Zijlstra , Jiri Olsa , Ingo Molnar , LKML , Namhyung Kim , Andi Kleen Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7052 Lines: 182 On Tue, Dec 15, 2015 at 4:54 PM, Andi Kleen wrote: > From: Andi Kleen > > When an event alias is used that the kernel marked as .agg-per-core, force > --per-core mode (and also require -a and forbid cgroups or per thread mode). > This in term means, --topdown forces --per-core mode. > > This is needed for TopDown in SMT mode, because it needs to measure > all threads in a core together and merge the values to compute the correct > percentages of how the pipeline is limited. > > We do this if any alias is agg-per-core. > I would rather call this attribute aggr-per-core. That would be more consistent with how perf calls this. > Add the code to parse the .agg-per-core attributes and propagate > the information to the evsel. Then the main stat code does > the necessary checks and forces per core mode. > > Open issue: in combination with -C ... we get wrong values. I think that's > a existing bug that needs to be debugged/fixed separately. > > Signed-off-by: Andi Kleen > --- > tools/perf/builtin-stat.c | 18 ++++++++++++++++++ > tools/perf/util/evsel.h | 1 + > tools/perf/util/parse-events.c | 1 + > tools/perf/util/pmu.c | 23 +++++++++++++++++++++++ > tools/perf/util/pmu.h | 2 ++ > 5 files changed, 45 insertions(+) > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c > index 4777355..0c7cdda 100644 > --- a/tools/perf/builtin-stat.c > +++ b/tools/perf/builtin-stat.c > @@ -1540,6 +1540,7 @@ static int add_default_attributes(void) > > int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused) > { > + struct perf_evsel *counter; > const char * const stat_usage[] = { > "perf stat [] []", > NULL > @@ -1674,6 +1675,23 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused) > if (add_default_attributes()) > goto out; > > + evlist__for_each (evsel_list, counter) { > + /* Enable per core mode if only a single event requires it. */ > + if (counter->agg_per_core) { > + if (stat_config.aggr_mode != AGGR_GLOBAL && > + stat_config.aggr_mode != AGGR_CORE) { > + pr_err("per core event configuration requires per core mode\n"); > + goto out; > + } > + stat_config.aggr_mode = AGGR_CORE; > + if (nr_cgroups || !target__has_cpu(&target)) { > + pr_err("per core event configuration requires system-wide mode (-a)\n"); > + goto out; > + } > + break; > + } > + } > + > target__validate(&target); > > if (perf_evlist__create_maps(evsel_list, &target) < 0) { > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h > index 5ded1fc..efc7f7c 100644 > --- a/tools/perf/util/evsel.h > +++ b/tools/perf/util/evsel.h > @@ -114,6 +114,7 @@ struct perf_evsel { > bool tracking; > bool per_pkg; > bool precise_max; > + bool agg_per_core; > /* parse modifier helper */ > int exclude_GH; > int nr_members; > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index 6fc8cd7..66d8ebd 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -1027,6 +1027,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data, > evsel->unit = info.unit; > evsel->scale = info.scale; > evsel->per_pkg = info.per_pkg; > + evsel->agg_per_core = info.agg_per_core; > evsel->snapshot = info.snapshot; > } > > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c > index 8a520e9..5a52dac 100644 > --- a/tools/perf/util/pmu.c > +++ b/tools/perf/util/pmu.c > @@ -189,6 +189,23 @@ perf_pmu__parse_per_pkg(struct perf_pmu_alias *alias, char *dir, char *name) > return 0; > } > > +static void > +perf_pmu__parse_agg_per_core(struct perf_pmu_alias *alias, char *dir, char *name) > +{ > + char path[PATH_MAX]; > + FILE *f; > + int flag; > + > + snprintf(path, PATH_MAX, "%s/%s.agg-per-core", dir, name); > + > + f = fopen(path, "r"); > + if (f && fscanf(f, "%d", &flag) == 1) { > + alias->agg_per_core = flag != 0; > + fclose(f); > + } > +} > + > + > static int perf_pmu__parse_snapshot(struct perf_pmu_alias *alias, > char *dir, char *name) > { > @@ -237,6 +254,7 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name, > perf_pmu__parse_scale(alias, dir, name); > perf_pmu__parse_per_pkg(alias, dir, name); > perf_pmu__parse_snapshot(alias, dir, name); > + perf_pmu__parse_agg_per_core(alias, dir, name); > } > > list_add_tail(&alias->list, list); > @@ -271,6 +289,8 @@ static inline bool pmu_alias_info_file(char *name) > return true; > if (len > 9 && !strcmp(name + len - 9, ".snapshot")) > return true; > + if (len > 13 && !strcmp(name + len - 13, ".agg-per-core")) > + return true; > > return false; > } > @@ -847,6 +867,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms, > int ret; > > info->per_pkg = false; > + info->agg_per_core = false; > > /* > * Mark unit and scale as not set > @@ -870,6 +891,8 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms, > > if (alias->per_pkg) > info->per_pkg = true; > + if (alias->agg_per_core) > + info->agg_per_core = true; > > list_del(&term->list); > free(term); > diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h > index 5d7e844..5a43719 100644 > --- a/tools/perf/util/pmu.h > +++ b/tools/perf/util/pmu.h > @@ -32,6 +32,7 @@ struct perf_pmu_info { > double scale; > bool per_pkg; > bool snapshot; > + bool agg_per_core; > }; > > #define UNIT_MAX_LEN 31 /* max length for event unit name */ > @@ -44,6 +45,7 @@ struct perf_pmu_alias { > double scale; > bool per_pkg; > bool snapshot; > + bool agg_per_core; > }; > > struct perf_pmu *perf_pmu__find(const char *name); > -- > 2.4.3 > -- 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/