Set of fixes related to the comments here [1]. Mainly cleanups,
additional tests and refactoring since adding the new strcmp_cpuid_str()
metric expression.
I added the string replace function to the perf utils
rather than tools/lib/string.c because it didn't seem
easy to add tests for tools/lib.
[1]: https://lore.kernel.org/linux-arm-kernel/CAP-5=fVnUx0BnJC7X1rrm42OD7Bk=ZsHWNwAZMBYyB7yWhBfhQ@mail.gmail.com/
---
Changes since v1:
* s -> haystack
* find -> needle
James Clark (7):
perf test: Check result of has_event(cycles) test
perf jevents: Remove unused keyword
perf util: Add a function for replacing characters in a string
perf test: Add a test for strcmp_cpuid_str() expression
perf pmu: Move pmu__find_core_pmu() to pmus.c
perf pmus: Simplify perf_pmus__find_core_pmu()
perf pmu: Remove unused function
tools/perf/arch/arm64/util/pmu.c | 20 ++++++-------
tools/perf/pmu-events/metric.py | 3 +-
tools/perf/tests/Build | 1 +
tools/perf/tests/builtin-test.c | 1 +
tools/perf/tests/expr.c | 33 ++++++++++++++++++----
tools/perf/tests/tests.h | 1 +
tools/perf/tests/util.c | 31 +++++++++++++++++++++
tools/perf/util/expr.c | 2 +-
tools/perf/util/pmu.c | 22 ---------------
tools/perf/util/pmu.h | 3 +-
tools/perf/util/pmus.c | 6 ++++
tools/perf/util/string.c | 48 ++++++++++++++++++++++++++++++++
tools/perf/util/string2.h | 1 +
13 files changed, 128 insertions(+), 44 deletions(-)
create mode 100644 tools/perf/tests/util.c
--
2.34.1
Test that the new expression builtin returns a match when the current
escaped CPU ID is given, and that it doesn't match when "0x0" is given.
The CPU ID in test__expr() has to be changed to perf_pmu__getcpuid()
which returns the CPU ID string, rather than the raw CPU ID that
get_cpuid() returns because that can't be used with strcmp_cpuid_str().
It doesn't affect the is_intel test because both versions contain
"Intel".
Reviewed-by: Ian Rogers <[email protected]>
Signed-off-by: James Clark <[email protected]>
---
tools/perf/tests/expr.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index 39be7f3b3a53..78544092ef0c 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -9,6 +9,7 @@
#include <math.h>
#include <stdlib.h>
#include <string.h>
+#include <string2.h>
#include <linux/zalloc.h>
static int test_ids_union(void)
@@ -74,10 +75,13 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u
int ret;
struct expr_parse_ctx *ctx;
bool is_intel = false;
- char buf[128];
+ char strcmp_cpuid_buf[256];
+ struct perf_pmu *pmu = pmu__find_core_pmu();
+ char *cpuid = perf_pmu__getcpuid(pmu);
+ char *escaped_cpuid1, *escaped_cpuid2;
- if (!get_cpuid(buf, sizeof(buf)))
- is_intel = strstr(buf, "Intel") != NULL;
+ TEST_ASSERT_VAL("get_cpuid", cpuid);
+ is_intel = strstr(cpuid, "Intel") != NULL;
TEST_ASSERT_EQUAL("ids_union", test_ids_union(), 0);
@@ -257,9 +261,28 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u
TEST_ASSERT_VAL("source count", hashmap__size(ctx->ids) == 1);
TEST_ASSERT_VAL("source count", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
+
+ /* Test no cpuid match */
+ ret = test(ctx, "strcmp_cpuid_str(0x0)", 0);
+
+ /*
+ * Test cpuid match with current cpuid. Special chars have to be
+ * escaped.
+ */
+ escaped_cpuid1 = strreplace_chars('-', cpuid, "\\-");
+ free(cpuid);
+ escaped_cpuid2 = strreplace_chars(',', escaped_cpuid1, "\\,");
+ free(escaped_cpuid1);
+ escaped_cpuid1 = strreplace_chars('=', escaped_cpuid2, "\\=");
+ free(escaped_cpuid2);
+ scnprintf(strcmp_cpuid_buf, sizeof(strcmp_cpuid_buf),
+ "strcmp_cpuid_str(%s)", escaped_cpuid1);
+ free(escaped_cpuid1);
+ ret |= test(ctx, strcmp_cpuid_buf, 1);
+
/* has_event returns 1 when an event exists. */
expr__add_id_val(ctx, strdup("cycles"), 2);
- ret = test(ctx, "has_event(cycles)", 1);
+ ret |= test(ctx, "has_event(cycles)", 1);
expr__ctx_free(ctx);
--
2.34.1
'cpuid_not_more_than' was the working title of the new
'strcmp_cpuid_str' keyword and was accidentally left in. It was never
used so tidying it up has no effect.
Reviewed-by: Ian Rogers <[email protected]>
Signed-off-by: James Clark <[email protected]>
---
tools/perf/pmu-events/metric.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/perf/pmu-events/metric.py b/tools/perf/pmu-events/metric.py
index 0e9ec65d92ae..1bbb89102901 100644
--- a/tools/perf/pmu-events/metric.py
+++ b/tools/perf/pmu-events/metric.py
@@ -558,8 +558,7 @@ def ParsePerfJson(orig: str) -> Expression:
# Convert accidentally converted scientific notation constants back
py = re.sub(r'([0-9]+)Event\(r"(e[0-9]+)"\)', r'\1\2', py)
# Convert all the known keywords back from events to just the keyword
- keywords = ['if', 'else', 'min', 'max', 'd_ratio', 'source_count', 'has_event', 'strcmp_cpuid_str',
- 'cpuid_not_more_than']
+ keywords = ['if', 'else', 'min', 'max', 'd_ratio', 'source_count', 'has_event', 'strcmp_cpuid_str']
for kw in keywords:
py = re.sub(rf'Event\(r"{kw}"\)', kw, py)
try:
--
2.34.1
Currently the while loop always either exits on the first iteration with
a core PMU, or exits with NULL on heterogeneous systems or when not all
CPUs are online.
Both of the latter behaviors are undesirable for platforms other than
Arm so simplify it to always return the first core PMU, or NULL if none
exist.
This behavior was depended on by the Arm version of
pmu_metrics_table__find(), so the logic has been moved there instead.
Suggested-by: Ian Rogers <[email protected]>
Reviewed-by: Ian Rogers <[email protected]>
Signed-off-by: James Clark <[email protected]>
---
tools/perf/arch/arm64/util/pmu.c | 8 +++++++-
tools/perf/util/pmus.c | 14 +-------------
2 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/tools/perf/arch/arm64/util/pmu.c b/tools/perf/arch/arm64/util/pmu.c
index 3d9330feebd2..3099f5f448ba 100644
--- a/tools/perf/arch/arm64/util/pmu.c
+++ b/tools/perf/arch/arm64/util/pmu.c
@@ -10,8 +10,14 @@
const struct pmu_metrics_table *pmu_metrics_table__find(void)
{
- struct perf_pmu *pmu = perf_pmus__find_core_pmu();
+ struct perf_pmu *pmu;
+
+ /* Metrics aren't currently supported on heterogeneous Arm systems */
+ if (perf_pmus__num_core_pmus() > 1)
+ return NULL;
+ /* Doesn't matter which one here because they'll all be the same */
+ pmu = perf_pmus__find_core_pmu();
if (pmu)
return perf_pmu__find_metrics_table(pmu);
diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
index 032ce57d2b8e..5ae41644ccda 100644
--- a/tools/perf/util/pmus.c
+++ b/tools/perf/util/pmus.c
@@ -596,17 +596,5 @@ struct perf_pmu *evsel__find_pmu(const struct evsel *evsel)
struct perf_pmu *perf_pmus__find_core_pmu(void)
{
- 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;
+ return perf_pmus__scan_core(NULL);
}
--
2.34.1