| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 5 May 2022 07:56:36 -0400
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Ian Rogers <irogers@...gle.com>,
Caleb Biggers <caleb.biggers@...el.com>,
Perry Taylor <perry.taylor@...el.com>,
Kshipra Bopardikar <kshipra.bopardikar@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Ravi Bangoria <ravi.bangoria@....com>,
Andi Kleen <ak@...ux.intel.com>,
Haowen Bai <baihaowen@...zu.com>,
Riccardo Mancini <rickyman7@...il.com>,
Kim Phillips <kim.phillips@....com>,
Madhavan Srinivasan <maddy@...ux.ibm.com>,
Shunsuke Nakamura <nakamura.shun@...itsu.com>,
Florian Fischer <florian.fischer@...q.space>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org
Cc: Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH 1/2] perf evlist: Keep topdown counters in weak group
Hi Ian,
Thanks for the patches.
On 5/5/2022 12:38 AM, Ian Rogers wrote:
> On Intel Icelake, topdown events must always be grouped with a slots
> event as leader. When a metric is parsed a weak group is formed and
> retried if perf_event_open fails. The retried events aren't grouped
> breaking the slots leader requirement. This change modifies the weak
> group "reset" behavior so that topdown events aren't broken from the
> group for the retry.
>
> $ perf stat -e '{slots,topdown-bad-spec,topdown-be-bound,topdown-fe-bound,topdown-retiring,branch-instructions,branch-misses,bus-cycles,cache-misses,cache-references,cpu-cycles,instructions,mem-loads,mem-stores,ref-cycles,baclears.any,ARITH.DIVIDER_ACTIVE}:W' -a sleep 1
>
> Performance counter stats for 'system wide':
>
> 47,867,188,483 slots (92.27%)
> <not supported> topdown-bad-spec
> <not supported> topdown-be-bound
> <not supported> topdown-fe-bound
> <not supported> topdown-retiring
> 2,173,346,937 branch-instructions (92.27%)
> 10,540,253 branch-misses # 0.48% of all branches (92.29%)
> 96,291,140 bus-cycles (92.29%)
> 6,214,202 cache-misses # 20.120 % of all cache refs (92.29%)
> 30,886,082 cache-references (76.91%)
> 11,773,726,641 cpu-cycles (84.62%)
> 11,807,585,307 instructions # 1.00 insn per cycle (92.31%)
> 0 mem-loads (92.32%)
> 2,212,928,573 mem-stores (84.69%)
> 10,024,403,118 ref-cycles (92.35%)
> 16,232,978 baclears.any (92.35%)
> 23,832,633 ARITH.DIVIDER_ACTIVE (84.59%)
>
> 0.981070734 seconds time elapsed
>
> After:
>
> $ perf stat -e '{slots,topdown-bad-spec,topdown-be-bound,topdown-fe-bound,topdown-retiring,branch-instructions,branch-misses,bus-cycles,cache-misses,cache-references,cpu-cycles,instructions,mem-loads,mem-stores,ref-cycles,baclears.any,ARITH.DIVIDER_ACTIVE}:W' -a sleep 1
>
> Performance counter stats for 'system wide':
>
> 31040189283 slots (92.27%)
> 8997514811 topdown-bad-spec # 28.2% bad speculation (92.27%)
> 10997536028 topdown-be-bound # 34.5% backend bound (92.27%)
> 4778060526 topdown-fe-bound # 15.0% frontend bound (92.27%)
> 7086628768 topdown-retiring # 22.2% retiring (92.27%)
> 1417611942 branch-instructions (92.26%)
> 5285529 branch-misses # 0.37% of all branches (92.28%)
> 62922469 bus-cycles (92.29%)
> 1440708 cache-misses # 8.292 % of all cache refs (92.30%)
> 17374098 cache-references (76.94%)
> 8040889520 cpu-cycles (84.63%)
> 7709992319 instructions # 0.96 insn per cycle (92.32%)
> 0 mem-loads (92.32%)
> 1515669558 mem-stores (84.68%)
> 6542411177 ref-cycles (92.35%)
> 4154149 baclears.any (92.35%)
> 20556152 ARITH.DIVIDER_ACTIVE (84.59%)
>
> 1.010799593 seconds time elapsed
>
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
> tools/perf/arch/x86/util/evsel.c | 12 ++++++++++++
> tools/perf/util/evlist.c | 16 ++++++++++++++--
> tools/perf/util/evsel.c | 10 ++++++++++
> tools/perf/util/evsel.h | 3 +++
> 4 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/arch/x86/util/evsel.c b/tools/perf/arch/x86/util/evsel.c
> index ac2899a25b7a..40b171de2086 100644
> --- a/tools/perf/arch/x86/util/evsel.c
> +++ b/tools/perf/arch/x86/util/evsel.c
> @@ -3,6 +3,7 @@
> #include <stdlib.h>
> #include "util/evsel.h"
> #include "util/env.h"
> +#include "util/pmu.h"
> #include "linux/string.h"
>
> void arch_evsel__set_sample_weight(struct evsel *evsel)
> @@ -29,3 +30,14 @@ void arch_evsel__fixup_new_cycles(struct perf_event_attr *attr)
>
> free(env.cpuid);
> }
> +
> +bool arch_evsel__must_be_in_group(const struct evsel *evsel)
> +{
> + if ((evsel->pmu_name && strcmp(evsel->pmu_name, "cpu")) ||
> + !pmu_have_event("cpu", "slots"))
> + return false;
> +
The big core of ADL also supports Perf_metrics. At least, we should
check both "cpu" and "cpu_core" here.
Thanks,
Kan
> + return evsel->name &&
> + (!strcasecmp(evsel->name, "slots") ||
> + !strncasecmp(evsel->name, "topdown", 7));
> +}
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 52ea004ba01e..dfa65a383502 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1790,8 +1790,17 @@ struct evsel *evlist__reset_weak_group(struct evlist *evsel_list, struct evsel *
> if (evsel__has_leader(c2, leader)) {
> if (is_open && close)
> perf_evsel__close(&c2->core);
> - evsel__set_leader(c2, c2);
> - c2->core.nr_members = 0;
> + /*
> + * We want to close all members of the group and reopen
> + * them. Some events, like Intel topdown, require being
> + * in a group and so keep these in the group.
> + */
> + if (!evsel__must_be_in_group(c2) && c2 != leader) {
> + evsel__set_leader(c2, c2);
> + c2->core.nr_members = 0;
> + leader->core.nr_members--;
> + }
> +
> /*
> * Set this for all former members of the group
> * to indicate they get reopened.
> @@ -1799,6 +1808,9 @@ struct evsel *evlist__reset_weak_group(struct evlist *evsel_list, struct evsel *
> c2->reset_group = true;
> }
> }
> + /* Reset the leader count if all entries were removed. */
> + if (leader->core.nr_members)
> + leader->core.nr_members = 0;
> return leader;
> }
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index d38722560e80..b7c0c9775673 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -3082,3 +3082,13 @@ int evsel__source_count(const struct evsel *evsel)
> }
> return count;
> }
> +
> +bool __weak arch_evsel__must_be_in_group(const struct evsel *evsel __maybe_unused)
> +{
> + return false;
> +}
> +
> +bool evsel__must_be_in_group(const struct evsel *evsel)
> +{
> + return arch_evsel__must_be_in_group(evsel);
> +}
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 45d674812239..0ed2850b7ebb 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -488,6 +488,9 @@ bool evsel__has_leader(struct evsel *evsel, struct evsel *leader);
> bool evsel__is_leader(struct evsel *evsel);
> void evsel__set_leader(struct evsel *evsel, struct evsel *leader);
> int evsel__source_count(const struct evsel *evsel);
> +bool evsel__must_be_in_group(const struct evsel *evsel);
> +
> +bool arch_evsel__must_be_in_group(const struct evsel *evsel);
>
> /*
> * Macro to swap the bit-field postition and size.
Powered by blists - more mailing lists