Samples without an L3 miss are discarded and counter is reset with
random value (between 1-15 for fetch pmu and 1-127 for op pmu) when
IBS L3 miss filtering is enabled. This causes a sampling period skew
but there is no way to reconstruct aggregated sampling period. So
print a warning at perf record if user sets l3missonly=1.
Ex:
# perf record -c 10000 -C 0 -e ibs_op/l3missonly=1/
WARNING: Hw internally resets sampling period when L3 Miss Filtering is enabled
and tagged operation does not cause L3 Miss. This causes sampling period skew.
Signed-off-by: Ravi Bangoria <[email protected]>
Acked-by: Ian Rogers <[email protected]>
---
tools/perf/arch/x86/util/evsel.c | 49 ++++++++++++++++++++++++++++++++
tools/perf/util/evsel.c | 7 +++++
tools/perf/util/evsel.h | 1 +
3 files changed, 57 insertions(+)
diff --git a/tools/perf/arch/x86/util/evsel.c b/tools/perf/arch/x86/util/evsel.c
index 88306183d629..fceb904902ab 100644
--- a/tools/perf/arch/x86/util/evsel.c
+++ b/tools/perf/arch/x86/util/evsel.c
@@ -5,6 +5,7 @@
#include "util/env.h"
#include "util/pmu.h"
#include "linux/string.h"
+#include "util/debug.h"
void arch_evsel__set_sample_weight(struct evsel *evsel)
{
@@ -60,3 +61,51 @@ bool arch_evsel__must_be_in_group(const struct evsel *evsel)
(!strcasecmp(evsel->name, "slots") ||
strcasestr(evsel->name, "topdown"));
}
+
+static void ibs_l3miss_warn(void)
+{
+ pr_warning(
+"WARNING: Hw internally resets sampling period when L3 Miss Filtering is enabled\n"
+"and tagged operation does not cause L3 Miss. This causes sampling period skew.\n");
+}
+
+void arch__post_evsel_config(struct evsel *evsel, struct perf_event_attr *attr)
+{
+ struct perf_pmu *evsel_pmu, *ibs_fetch_pmu, *ibs_op_pmu;
+ static int warned_once;
+ /* 0: Uninitialized, 1: Yes, -1: No */
+ static int is_amd;
+
+ if (warned_once || is_amd == -1)
+ return;
+
+ if (!is_amd) {
+ struct perf_env *env = evsel__env(evsel);
+
+ if (!perf_env__cpuid(env) || !env->cpuid ||
+ !strstarts(env->cpuid, "AuthenticAMD")) {
+ is_amd = -1;
+ return;
+ }
+ is_amd = 1;
+ }
+
+ evsel_pmu = evsel__find_pmu(evsel);
+ if (!evsel_pmu)
+ return;
+
+ ibs_fetch_pmu = perf_pmu__find("ibs_fetch");
+ ibs_op_pmu = perf_pmu__find("ibs_op");
+
+ if (ibs_fetch_pmu && ibs_fetch_pmu->type == evsel_pmu->type) {
+ if (attr->config & (1ULL << 59)) {
+ ibs_l3miss_warn();
+ warned_once = 1;
+ }
+ } else if (ibs_op_pmu && ibs_op_pmu->type == evsel_pmu->type) {
+ if (attr->config & (1ULL << 16)) {
+ ibs_l3miss_warn();
+ warned_once = 1;
+ }
+ }
+}
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index ce499c5da8d7..8fea51a9cd90 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1091,6 +1091,11 @@ void __weak arch_evsel__fixup_new_cycles(struct perf_event_attr *attr __maybe_un
{
}
+void __weak arch__post_evsel_config(struct evsel *evsel __maybe_unused,
+ struct perf_event_attr *attr __maybe_unused)
+{
+}
+
static void evsel__set_default_freq_period(struct record_opts *opts,
struct perf_event_attr *attr)
{
@@ -1366,6 +1371,8 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
*/
if (evsel__is_dummy_event(evsel))
evsel__reset_sample_bit(evsel, BRANCH_STACK);
+
+ arch__post_evsel_config(evsel, attr);
}
int evsel__set_filter(struct evsel *evsel, const char *filter)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 73ea48e94079..92bed8e2f7d8 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -297,6 +297,7 @@ void evsel__set_sample_id(struct evsel *evsel, bool use_sample_identifier);
void arch_evsel__set_sample_weight(struct evsel *evsel);
void arch_evsel__fixup_new_cycles(struct perf_event_attr *attr);
+void arch__post_evsel_config(struct evsel *evsel, struct perf_event_attr *attr);
int evsel__set_filter(struct evsel *evsel, const char *filter);
int evsel__append_tp_filter(struct evsel *evsel, const char *filter);
--
2.31.1
Hi Ravi,
On Tue, May 31, 2022 at 8:27 PM Ravi Bangoria <[email protected]> wrote:
>
> Samples without an L3 miss are discarded and counter is reset with
> random value (between 1-15 for fetch pmu and 1-127 for op pmu) when
> IBS L3 miss filtering is enabled. This causes a sampling period skew
> but there is no way to reconstruct aggregated sampling period. So
> print a warning at perf record if user sets l3missonly=1.
>
> Ex:
> # perf record -c 10000 -C 0 -e ibs_op/l3missonly=1/
> WARNING: Hw internally resets sampling period when L3 Miss Filtering is enabled
> and tagged operation does not cause L3 Miss. This causes sampling period skew.
>
> Signed-off-by: Ravi Bangoria <[email protected]>
> Acked-by: Ian Rogers <[email protected]>
> ---
> tools/perf/arch/x86/util/evsel.c | 49 ++++++++++++++++++++++++++++++++
> tools/perf/util/evsel.c | 7 +++++
> tools/perf/util/evsel.h | 1 +
> 3 files changed, 57 insertions(+)
>
> diff --git a/tools/perf/arch/x86/util/evsel.c b/tools/perf/arch/x86/util/evsel.c
> index 88306183d629..fceb904902ab 100644
> --- a/tools/perf/arch/x86/util/evsel.c
> +++ b/tools/perf/arch/x86/util/evsel.c
> @@ -5,6 +5,7 @@
> #include "util/env.h"
> #include "util/pmu.h"
> #include "linux/string.h"
> +#include "util/debug.h"
>
> void arch_evsel__set_sample_weight(struct evsel *evsel)
> {
> @@ -60,3 +61,51 @@ bool arch_evsel__must_be_in_group(const struct evsel *evsel)
> (!strcasecmp(evsel->name, "slots") ||
> strcasestr(evsel->name, "topdown"));
> }
> +
> +static void ibs_l3miss_warn(void)
> +{
> + pr_warning(
> +"WARNING: Hw internally resets sampling period when L3 Miss Filtering is enabled\n"
> +"and tagged operation does not cause L3 Miss. This causes sampling period skew.\n");
> +}
> +
> +void arch__post_evsel_config(struct evsel *evsel, struct perf_event_attr *attr)
> +{
> + struct perf_pmu *evsel_pmu, *ibs_fetch_pmu, *ibs_op_pmu;
> + static int warned_once;
> + /* 0: Uninitialized, 1: Yes, -1: No */
> + static int is_amd;
> +
> + if (warned_once || is_amd == -1)
> + return;
> +
> + if (!is_amd) {
> + struct perf_env *env = evsel__env(evsel);
> +
> + if (!perf_env__cpuid(env) || !env->cpuid ||
> + !strstarts(env->cpuid, "AuthenticAMD")) {
> + is_amd = -1;
> + return;
> + }
> + is_amd = 1;
> + }
> +
> + evsel_pmu = evsel__find_pmu(evsel);
> + if (!evsel_pmu)
> + return;
> +
> + ibs_fetch_pmu = perf_pmu__find("ibs_fetch");
> + ibs_op_pmu = perf_pmu__find("ibs_op");
> +
> + if (ibs_fetch_pmu && ibs_fetch_pmu->type == evsel_pmu->type) {
> + if (attr->config & (1ULL << 59)) {
It'd be nice if we used a macro or something instead of the
magic number.
> + ibs_l3miss_warn();
> + warned_once = 1;
> + }
> + } else if (ibs_op_pmu && ibs_op_pmu->type == evsel_pmu->type) {
> + if (attr->config & (1ULL << 16)) {
Ditto.
Thanks,
Namhyung
> + ibs_l3miss_warn();
> + warned_once = 1;
> + }
> + }
> +}
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index ce499c5da8d7..8fea51a9cd90 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1091,6 +1091,11 @@ void __weak arch_evsel__fixup_new_cycles(struct perf_event_attr *attr __maybe_un
> {
> }
>
> +void __weak arch__post_evsel_config(struct evsel *evsel __maybe_unused,
> + struct perf_event_attr *attr __maybe_unused)
> +{
> +}
> +
> static void evsel__set_default_freq_period(struct record_opts *opts,
> struct perf_event_attr *attr)
> {
> @@ -1366,6 +1371,8 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
> */
> if (evsel__is_dummy_event(evsel))
> evsel__reset_sample_bit(evsel, BRANCH_STACK);
> +
> + arch__post_evsel_config(evsel, attr);
> }
>
> int evsel__set_filter(struct evsel *evsel, const char *filter)
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 73ea48e94079..92bed8e2f7d8 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -297,6 +297,7 @@ void evsel__set_sample_id(struct evsel *evsel, bool use_sample_identifier);
>
> void arch_evsel__set_sample_weight(struct evsel *evsel);
> void arch_evsel__fixup_new_cycles(struct perf_event_attr *attr);
> +void arch__post_evsel_config(struct evsel *evsel, struct perf_event_attr *attr);
>
> int evsel__set_filter(struct evsel *evsel, const char *filter);
> int evsel__append_tp_filter(struct evsel *evsel, const char *filter);
> --
> 2.31.1
>
On 03-Jun-22 10:42 AM, Ravi Bangoria wrote:
>>> + if (ibs_fetch_pmu && ibs_fetch_pmu->type == evsel_pmu->type) {
>>> + if (attr->config & (1ULL << 59)) {
>>
>> It'd be nice if we used a macro or something instead of the
>> magic number.
>>
>>> + ibs_l3miss_warn();
>>> + warned_once = 1;
>>> + }
>>> + } else if (ibs_op_pmu && ibs_op_pmu->type == evsel_pmu->type) {
>>> + if (attr->config & (1ULL << 16)) {
>>
>> Ditto.
>
> Thanks for the review, Namhyung.
>
> Arnaldo, Would you be able to squash below trivial patch into original
> patch? Please let me know if you want me to respin the series instead.
I'm planning to respin with asprintf() change. Sorry for the noise.
Thanks,
Ravi
>> + if (ibs_fetch_pmu && ibs_fetch_pmu->type == evsel_pmu->type) {
>> + if (attr->config & (1ULL << 59)) {
>
> It'd be nice if we used a macro or something instead of the
> magic number.
>
>> + ibs_l3miss_warn();
>> + warned_once = 1;
>> + }
>> + } else if (ibs_op_pmu && ibs_op_pmu->type == evsel_pmu->type) {
>> + if (attr->config & (1ULL << 16)) {
>
> Ditto.
Thanks for the review, Namhyung.
Arnaldo, Would you be able to squash below trivial patch into original
patch? Please let me know if you want me to respin the series instead.
---><---
From 352228ee010b0782410e233233377d9484ea51bd Mon Sep 17 00:00:00 2001
From: Ravi Bangoria <[email protected]>
Date: Fri, 3 Jun 2022 10:19:01 +0530
Subject: [PATCH] perf record ibs: Define macros for IBS L3MissOnly bits
Instead of magic numbers use macros for IBS L3MissOnly bits.
Signed-off-by: Ravi Bangoria <[email protected]>
---
tools/perf/arch/x86/util/evsel.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/perf/arch/x86/util/evsel.c b/tools/perf/arch/x86/util/evsel.c
index fceb904902ab..53763e583804 100644
--- a/tools/perf/arch/x86/util/evsel.c
+++ b/tools/perf/arch/x86/util/evsel.c
@@ -7,6 +7,9 @@
#include "linux/string.h"
#include "util/debug.h"
+#define IBS_FETCH_L3MISSONLY (1ULL << 59)
+#define IBS_OP_L3MISSONLY (1ULL << 16)
+
void arch_evsel__set_sample_weight(struct evsel *evsel)
{
evsel__set_sample_bit(evsel, WEIGHT_STRUCT);
@@ -98,12 +101,12 @@ void arch__post_evsel_config(struct evsel *evsel, struct perf_event_attr *attr)
ibs_op_pmu = perf_pmu__find("ibs_op");
if (ibs_fetch_pmu && ibs_fetch_pmu->type == evsel_pmu->type) {
- if (attr->config & (1ULL << 59)) {
+ if (attr->config & IBS_FETCH_L3MISSONLY) {
ibs_l3miss_warn();
warned_once = 1;
}
} else if (ibs_op_pmu && ibs_op_pmu->type == evsel_pmu->type) {
- if (attr->config & (1ULL << 16)) {
+ if (attr->config & IBS_OP_L3MISSONLY) {
ibs_l3miss_warn();
warned_once = 1;
}
--
2.31.1
Em Fri, Jun 03, 2022 at 10:58:13AM +0530, Ravi Bangoria escreveu:
> On 03-Jun-22 10:42 AM, Ravi Bangoria wrote:
> >>> + if (ibs_fetch_pmu && ibs_fetch_pmu->type == evsel_pmu->type) {
> >>> + if (attr->config & (1ULL << 59)) {
> >>
> >> It'd be nice if we used a macro or something instead of the
> >> magic number.
> >>
> >>> + ibs_l3miss_warn();
> >>> + warned_once = 1;
> >>> + }
> >>> + } else if (ibs_op_pmu && ibs_op_pmu->type == evsel_pmu->type) {
> >>> + if (attr->config & (1ULL << 16)) {
> >>
> >> Ditto.
> >
> > Thanks for the review, Namhyung.
> >
> > Arnaldo, Would you be able to squash below trivial patch into original
> > patch? Please let me know if you want me to respin the series instead.
>
> I'm planning to respin with asprintf() change. Sorry for the noise.
Ok, will wait for the respin then.
- Arnaldo