[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <db7d5375-b5b1-1fff-d612-73873ef2765c@arm.com>
Date: Mon, 14 Aug 2023 14:44:05 +0100
From: James Clark <james.clark@....com>
To: John Garry <john.g.garry@...cle.com>,
linux-perf-users@...r.kernel.org, irogers@...gle.com,
renyu.zj@...ux.alibaba.com
Cc: Will Deacon <will@...nel.org>, Mike Leach <mike.leach@...aro.org>,
Leo Yan <leo.yan@...aro.org>,
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>,
Adrian Hunter <adrian.hunter@...el.com>,
Suzuki K Poulose <suzuki.poulose@....com>,
Kan Liang <kan.liang@...ux.intel.com>,
Kajol Jain <kjain@...ux.ibm.com>,
Nick Forrington <nick.forrington@....com>,
Eduard Zingerman <eddyz87@...il.com>,
Sohom Datta <sohomdatta1@...il.com>,
Rob Herring <robh@...nel.org>, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, coresight@...ts.linaro.org
Subject: Re: [PATCH v5 5/6] perf vendor events arm64: Update stall_slot
workaround for N2 r0p3
On 14/08/2023 14:02, John Garry wrote:
>
>> try:
>> parsed = ast.parse(py, mode='eval')
>> except SyntaxError as e:
>> diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
>> index 7410a165f68b..0985a3cbc6f9 100644
>> --- a/tools/perf/util/expr.c
>> +++ b/tools/perf/util/expr.c
>> @@ -13,6 +13,8 @@
>> #include <util/expr-bison.h>
>> #include <util/expr-flex.h>
>> #include "util/hashmap.h"
>> +#include "util/header.h"
>> +#include "util/pmu.h"
>> #include "smt.h"
>> #include "tsc.h"
>> #include <api/fs/fs.h>
>> @@ -495,3 +497,19 @@ double expr__has_event(const struct
>> expr_parse_ctx *ctx, bool compute_ids, const
>> evlist__delete(tmp);
>> return ret;
>> }
>> +
>> +double expr__strcmp_cpuid_str(const struct expr_parse_ctx *ctx
>> __maybe_unused,
>> + bool compute_ids __maybe_unused, const char *test_id)
>> +{
>> + double ret;
>> + struct perf_pmu *pmu = pmu__find_core_pmu();
>> + char *cpuid = perf_pmu__getcpuid(pmu);
>> +
>> + if (!cpuid)
>> + return NAN;
>> +
>> + ret = !strcmp_cpuid_str(test_id, cpuid);
>
> It seems that strcmp_cpuid_str() is only added in arm64 arch code -
> should there be a weak version for other archs?
I think there is one in tools/perf/util/header.c. I tested the build on
x86 as well as arm so it should be working.
>
>> +
>> + free(cpuid);
>> + return ret;
>> +}
>> diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
>> index 3c1e49b3e35d..c0cec29ddc29 100644
>> --- a/tools/perf/util/expr.h
>> +++ b/tools/perf/util/expr.h
>> @@ -55,5 +55,6 @@ double expr_id_data__value(const struct expr_id_data
>> *data);
>> double expr_id_data__source_count(const struct expr_id_data *data);
>> double expr__get_literal(const char *literal, const struct
>> expr_scanner_ctx *ctx);
>> double expr__has_event(const struct expr_parse_ctx *ctx, bool
>> compute_ids, const char *id);
>> +double expr__strcmp_cpuid_str(const struct expr_parse_ctx *ctx, bool
>> compute_ids, const char *id);
>> #endif
>> diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
>> index dbb117414710..0feef0726c48 100644
>> --- a/tools/perf/util/expr.l
>> +++ b/tools/perf/util/expr.l
>> @@ -114,6 +114,7 @@ if { return IF; }
>> else { return ELSE; }
>> source_count { return SOURCE_COUNT; }
>> has_event { return HAS_EVENT; }
>> +strcmp_cpuid_str { return STRCMP_CPUID_STR; }
>> {literal} { return literal(yyscanner, sctx); }
>> {number} { return value(yyscanner); }
>> {symbol} { return str(yyscanner, ID, sctx->runtime); }
>> diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
>> index 65d54a6f29ad..6c93b358cc2d 100644
>> --- a/tools/perf/util/expr.y
>> +++ b/tools/perf/util/expr.y
>> @@ -39,7 +39,7 @@ int expr_lex(YYSTYPE * yylval_param , void *yyscanner);
>> } ids;
>> }
>> -%token ID NUMBER MIN MAX IF ELSE LITERAL D_RATIO SOURCE_COUNT
>> HAS_EVENT EXPR_ERROR
>> +%token ID NUMBER MIN MAX IF ELSE LITERAL D_RATIO SOURCE_COUNT
>> HAS_EVENT STRCMP_CPUID_STR EXPR_ERROR
>> %left MIN MAX IF
>> %left '|'
>> %left '^'
>> @@ -207,6 +207,12 @@ expr: NUMBER
>> $$.ids = NULL;
>> free($3);
>> }
>> +| STRCMP_CPUID_STR '(' ID ')'
>> +{
>> + $$.val = expr__strcmp_cpuid_str(ctx, compute_ids, $3);
>> + $$.ids = NULL;
>> + free($3);
>> +}
>> | expr '|' expr
>> {
>> if (is_const($1.val) && is_const($3.val)) {
>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>> index b6654b9f55d2..b6948021fe29 100644
>> --- a/tools/perf/util/pmu.c
>> +++ b/tools/perf/util/pmu.c
>> @@ -1779,3 +1779,20 @@ void perf_pmu__delete(struct perf_pmu *pmu)
>> zfree(&pmu->alias_name);
>> free(pmu);
>> }
>> +
>> +struct perf_pmu *pmu__find_core_pmu(void)
>
> Why was this relocated? I don't see anywhere changed which referenced
> pmu__find_core_pmu() in this patch
>
It's in expr__strcmp_cpuid_str(), I added a new call to it.
>> +{
>> + struct perf_pmu *pmu = NULL;
>> +
>> + while ((pmu = perf_pmus__scan_core(pmu))) {
>> + /*
>> + * The cpumap should cover all CPUs. Otherwise, some CPUs may
>> + * not support some events or have different event IDs.
>> + */
>> + if (RC_CHK_ACCESS(pmu->cpus)->nr != cpu__max_cpu().cpu)
>> + return NULL;
>> +
>> + return pmu;
>> + }
>> + return NULL;
>> +}
>> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
>> index 203b92860e3c..580b8d65bd65 100644
>> --- a/tools/perf/util/pmu.h
>> +++ b/tools/perf/util/pmu.h
>> @@ -288,5 +288,6 @@ int perf_pmu__pathname_fd(int dirfd, const char
>> *pmu_name, const char *filename,
>> struct perf_pmu *perf_pmu__lookup(struct list_head *pmus, int dirfd,
>> const char *lookup_name);
>> struct perf_pmu *perf_pmu__create_placeholder_core_pmu(struct
>> list_head *core_pmus);
>> void perf_pmu__delete(struct perf_pmu *pmu);
>> +struct perf_pmu *pmu__find_core_pmu(void);
>> #endif /* __PMU_H */
>
Powered by blists - more mailing lists