* [PATCH v2 1/4] perf tests: Make "test data symbol" more robust on Neoverse N1
2024-04-10 10:34 [PATCH v2 0/4] perf test: "object code reading" test fixes James Clark
@ 2024-04-10 10:34 ` James Clark
2024-04-10 10:34 ` [PATCH v2 2/4] perf tests: Apply attributes to all events in object code reading test James Clark
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: James Clark @ 2024-04-10 10:34 UTC (permalink / raw
To: linux-perf-users, irogers, namhyung
Cc: James Clark, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Adrian Hunter, Liang, Kan, Athira Rajeev, Spoorthy S,
Leo Yan, linux-kernel
To prevent anyone from seeing a test failure appear as a regression and
thinking that it was caused by their code change, insert some noise into
the loop which makes it immune to sampling bias issues (errata 1694299).
The "test data symbol" test can fail with any unrelated change that
shifts the loop into an unfortunate position in the Perf binary which is
almost impossible to debug as the root cause of the test failure.
Ultimately it's caused by the referenced errata.
Fixes: 60abedb8aa90 ("perf test: Introduce script for data symbol testing")
Signed-off-by: James Clark <james.clark@arm.com>
---
tools/perf/tests/workloads/datasym.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/tools/perf/tests/workloads/datasym.c b/tools/perf/tests/workloads/datasym.c
index ddd40bc63448..8e08fc75a973 100644
--- a/tools/perf/tests/workloads/datasym.c
+++ b/tools/perf/tests/workloads/datasym.c
@@ -16,6 +16,22 @@ static int datasym(int argc __maybe_unused, const char **argv __maybe_unused)
{
for (;;) {
buf1.data1++;
+ if (buf1.data1 == 123) {
+ /*
+ * Add some 'noise' in the loop to work around errata
+ * 1694299 on Arm N1.
+ *
+ * Bias exists in SPE sampling which can cause the load
+ * and store instructions to be skipped entirely. This
+ * comes and goes randomly depending on the offset the
+ * linker places the datasym loop at in the Perf binary.
+ * With an extra branch in the middle of the loop that
+ * isn't always taken, the instruction stream is no
+ * longer a continuous repeating pattern that interacts
+ * badly with the bias.
+ */
+ buf1.data1++;
+ }
buf1.data2 += buf1.data1;
}
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 2/4] perf tests: Apply attributes to all events in object code reading test
2024-04-10 10:34 [PATCH v2 0/4] perf test: "object code reading" test fixes James Clark
2024-04-10 10:34 ` [PATCH v2 1/4] perf tests: Make "test data symbol" more robust on Neoverse N1 James Clark
@ 2024-04-10 10:34 ` James Clark
2024-04-10 10:34 ` [PATCH v2 3/4] perf map: Remove kernel map before updating start and end addresses James Clark
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: James Clark @ 2024-04-10 10:34 UTC (permalink / raw
To: linux-perf-users, irogers, namhyung
Cc: James Clark, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Adrian Hunter, Liang, Kan, Athira Rajeev, Spoorthy S,
Leo Yan, linux-kernel
PERF_PMU_CAP_EXTENDED_HW_TYPE results in multiple events being opened on
heterogeneous systems. Currently this test only sets its required
attributes on the first event. Not disabling enable_on_exec on the other
events causes the test to fail because the forked objdump processes are
sampled. No tracking event is opened so Perf only knows about its own
mappings causing the objdump samples to give the following error:
$ perf test -vvv "object code reading"
Reading object code for memory address: 0xffff9aaa55ec
thread__find_map failed
---- end(-1) ----
24: Object code reading : FAILED!
Fixes: 251aa040244a ("perf parse-events: Wildcard most "numeric" events")
Signed-off-by: James Clark <james.clark@arm.com>
---
tools/perf/tests/code-reading.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 7a3a7bbbec71..29d2f3ee4e10 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -637,11 +637,11 @@ static int do_test_code_reading(bool try_kcore)
evlist__config(evlist, &opts, NULL);
- evsel = evlist__first(evlist);
-
- evsel->core.attr.comm = 1;
- evsel->core.attr.disabled = 1;
- evsel->core.attr.enable_on_exec = 0;
+ evlist__for_each_entry(evlist, evsel) {
+ evsel->core.attr.comm = 1;
+ evsel->core.attr.disabled = 1;
+ evsel->core.attr.enable_on_exec = 0;
+ }
ret = evlist__open(evlist);
if (ret < 0) {
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 3/4] perf map: Remove kernel map before updating start and end addresses
2024-04-10 10:34 [PATCH v2 0/4] perf test: "object code reading" test fixes James Clark
2024-04-10 10:34 ` [PATCH v2 1/4] perf tests: Make "test data symbol" more robust on Neoverse N1 James Clark
2024-04-10 10:34 ` [PATCH v2 2/4] perf tests: Apply attributes to all events in object code reading test James Clark
@ 2024-04-10 10:34 ` James Clark
2024-04-10 10:34 ` [PATCH v2 4/4] perf tests: Remove dependency on lscpu James Clark
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: James Clark @ 2024-04-10 10:34 UTC (permalink / raw
To: linux-perf-users, irogers, namhyung
Cc: James Clark, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Adrian Hunter, Liang, Kan, Athira Rajeev, Spoorthy S,
Leo Yan, linux-kernel
In a debug build there is validation that mmap lists are sorted when
taking a lock. In machine__update_kernel_mmap() the start and end
addresses are updated resulting in an unsorted list before the map is
removed from the list. When the map is removed, the lock is taken which
triggers the validation and the failure:
$ perf test "object code reading"
--- start ---
perf: util/maps.c:88: check_invariants: Assertion `map__start(prev) <= map__start(map)' failed.
Aborted
Fix it by updating the addresses after removal, but before insertion.
The bug depends on the ordering and type of debug info on the system and
doesn't reproduce everywhere.
Fixes: 659ad3492b91 ("perf maps: Switch from rbtree to lazily sorted array for addresses")
Signed-off-by: James Clark <james.clark@arm.com>
---
tools/perf/util/machine.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 5eb9044bc223..a26c8bea58d0 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1549,8 +1549,8 @@ static int machine__update_kernel_mmap(struct machine *machine,
updated = map__get(orig);
machine->vmlinux_map = updated;
- machine__set_kernel_mmap(machine, start, end);
maps__remove(machine__kernel_maps(machine), orig);
+ machine__set_kernel_mmap(machine, start, end);
err = maps__insert(machine__kernel_maps(machine), updated);
map__put(orig);
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 4/4] perf tests: Remove dependency on lscpu
2024-04-10 10:34 [PATCH v2 0/4] perf test: "object code reading" test fixes James Clark
` (2 preceding siblings ...)
2024-04-10 10:34 ` [PATCH v2 3/4] perf map: Remove kernel map before updating start and end addresses James Clark
@ 2024-04-10 10:34 ` James Clark
2024-04-10 16:17 ` [PATCH v2 0/4] perf test: "object code reading" test fixes Ian Rogers
2024-04-10 18:49 ` Namhyung Kim
5 siblings, 0 replies; 8+ messages in thread
From: James Clark @ 2024-04-10 10:34 UTC (permalink / raw
To: linux-perf-users, irogers, namhyung
Cc: James Clark, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Adrian Hunter, Liang, Kan, Athira Rajeev, Spoorthy S,
Leo Yan, linux-kernel
This check can be done with uname which is more portable. At the same
time re-arrange it into a standard if statement so that it's more
readable.
Signed-off-by: James Clark <james.clark@arm.com>
---
tools/perf/tests/shell/test_arm_callgraph_fp.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/perf/tests/shell/test_arm_callgraph_fp.sh b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
index 83b53591b1ea..61898e256616 100755
--- a/tools/perf/tests/shell/test_arm_callgraph_fp.sh
+++ b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
@@ -6,7 +6,9 @@ shelldir=$(dirname "$0")
# shellcheck source=lib/perf_has_symbol.sh
. "${shelldir}"/lib/perf_has_symbol.sh
-lscpu | grep -q "aarch64" || exit 2
+if [ "$(uname -m)" != "aarch64" ]; then
+ exit 2
+fi
if perf version --build-options | grep HAVE_DWARF_UNWIND_SUPPORT | grep -q OFF
then
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 0/4] perf test: "object code reading" test fixes
2024-04-10 10:34 [PATCH v2 0/4] perf test: "object code reading" test fixes James Clark
` (3 preceding siblings ...)
2024-04-10 10:34 ` [PATCH v2 4/4] perf tests: Remove dependency on lscpu James Clark
@ 2024-04-10 16:17 ` Ian Rogers
2024-04-10 21:00 ` Arnaldo Carvalho de Melo
2024-04-10 18:49 ` Namhyung Kim
5 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2024-04-10 16:17 UTC (permalink / raw
To: James Clark
Cc: linux-perf-users, namhyung, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Adrian Hunter, Liang, Kan, Athira Rajeev, Spoorthy S,
Leo Yan, linux-kernel
On Wed, Apr 10, 2024 at 3:35 AM James Clark <james.clark@arm.com> wrote:
>
> A few fixes around the object code reading test. The first patch
> appears to be unrelated, but in this case the data symbol test is
> broken on Arm N1 by the second commit.
>
> Changes since V1:
> * Put data symbol test fix first so that bisecting still works on N1
> * Instead of skipping "test data symbol" on N1, add some noise into
> the loop.
> * Add a commit to replace the only usage of lscpu in the tests with
> uname
>
> James Clark (4):
> perf tests: Make "test data symbol" more robust on Neoverse N1
> perf tests: Apply attributes to all events in object code reading test
> perf map: Remove kernel map before updating start and end addresses
> perf tests: Remove dependency on lscpu
Reviewed-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> tools/perf/tests/code-reading.c | 10 +++++-----
> tools/perf/tests/shell/test_arm_callgraph_fp.sh | 4 +++-
> tools/perf/tests/workloads/datasym.c | 16 ++++++++++++++++
> tools/perf/util/machine.c | 2 +-
> 4 files changed, 25 insertions(+), 7 deletions(-)
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 0/4] perf test: "object code reading" test fixes
2024-04-10 16:17 ` [PATCH v2 0/4] perf test: "object code reading" test fixes Ian Rogers
@ 2024-04-10 21:00 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-04-10 21:00 UTC (permalink / raw
To: James Clark, Ian Rogers
Cc: linux-perf-users, namhyung, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Liang, Kan, Athira Rajeev, Spoorthy S, Leo Yan, linux-kernel
On Wed, Apr 10, 2024 at 09:17:04AM -0700, Ian Rogers wrote:
> On Wed, Apr 10, 2024 at 3:35 AM James Clark <james.clark@arm.com> wrote:
> >
> > A few fixes around the object code reading test. The first patch
> > appears to be unrelated, but in this case the data symbol test is
> > broken on Arm N1 by the second commit.
> >
> > Changes since V1:
> > * Put data symbol test fix first so that bisecting still works on N1
> > * Instead of skipping "test data symbol" on N1, add some noise into
> > the loop.
> > * Add a commit to replace the only usage of lscpu in the tests with
> > uname
> >
> > James Clark (4):
> > perf tests: Make "test data symbol" more robust on Neoverse N1
> > perf tests: Apply attributes to all events in object code reading test
> > perf map: Remove kernel map before updating start and end addresses
> > perf tests: Remove dependency on lscpu
>
> Reviewed-by: Ian Rogers <irogers@google.com>
Thanks, applied to perf-tools-next,
- Arnaldo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/4] perf test: "object code reading" test fixes
2024-04-10 10:34 [PATCH v2 0/4] perf test: "object code reading" test fixes James Clark
` (4 preceding siblings ...)
2024-04-10 16:17 ` [PATCH v2 0/4] perf test: "object code reading" test fixes Ian Rogers
@ 2024-04-10 18:49 ` Namhyung Kim
5 siblings, 0 replies; 8+ messages in thread
From: Namhyung Kim @ 2024-04-10 18:49 UTC (permalink / raw
To: James Clark
Cc: linux-perf-users, irogers, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Adrian Hunter, Liang, Kan, Athira Rajeev, Spoorthy S,
Leo Yan, linux-kernel
Hello,
On Wed, Apr 10, 2024 at 3:35 AM James Clark <james.clark@arm.com> wrote:
>
> A few fixes around the object code reading test. The first patch
> appears to be unrelated, but in this case the data symbol test is
> broken on Arm N1 by the second commit.
>
> Changes since V1:
> * Put data symbol test fix first so that bisecting still works on N1
> * Instead of skipping "test data symbol" on N1, add some noise into
> the loop.
> * Add a commit to replace the only usage of lscpu in the tests with
> uname
>
> James Clark (4):
> perf tests: Make "test data symbol" more robust on Neoverse N1
> perf tests: Apply attributes to all events in object code reading test
> perf map: Remove kernel map before updating start and end addresses
> perf tests: Remove dependency on lscpu
Acked-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 8+ messages in thread