Re: [PATCH V16 7/7] perf intel-pt: Add a test for pause / resume

From: Ian Rogers
Date: Sun Dec 22 2024 - 11:54:29 EST

Next message: pr-tracker-bot: "Re: [GIT PULL] Devicetree fixes for v6.13, part 1"
Previous message: Marc Kleine-Budde: "Re: [PATCH v6 0/7] can: m_can: Add am62 wakeup support"
In reply to: Adrian Hunter: "Re: [PATCH V16 7/7] perf intel-pt: Add a test for pause / resume"
Next in thread: Adrian Hunter: "Re: [PATCH V16 7/7] perf intel-pt: Add a test for pause / resume"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Sat, Dec 21, 2024 at 11:30 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>
> On 21/12/24 19:10, Ian Rogers wrote:
> > On Sun, Dec 15, 2024 at 11:03 PM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> >>
> >> Add a simple sub-test to the "Miscellaneous Intel PT testing" test to
> >> check pause / resume.
> >>
> >> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> >> Acked-by: Ian Rogers <irogers@xxxxxxxxxx>
> >> Reviewed-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> >> ---
> >> tools/perf/tests/shell/test_intel_pt.sh | 28 +++++++++++++++++++++++++
> >> 1 file changed, 28 insertions(+)
> >>
> >> diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
> >> index e6f0070975f6..f3a9a040bacc 100755
> >> --- a/tools/perf/tests/shell/test_intel_pt.sh
> >> +++ b/tools/perf/tests/shell/test_intel_pt.sh
> >> @@ -644,6 +644,33 @@ test_pipe()
> >> return 0
> >> }
> >>
> >> +test_pause_resume()
> >> +{
> >> + echo "--- Test with pause / resume ---"
> >> + if ! perf_record_no_decode -o "${perfdatafile}" -e intel_pt/aux-action=start-paused/u uname ; then
> >> + echo "SKIP: pause / resume is not supported"
> >> + return 2
> >> + fi
> >> + if ! perf_record_no_bpf -o "${perfdatafile}" \
> >> + -e intel_pt/aux-action=start-paused/u \
> >> + -e instructions/period=50000,aux-action=resume,name=Resume/u \
> >> + -e instructions/period=100000,aux-action=pause,name=Pause/u uname ; then
> >> + echo "perf record with pause / resume failed"
> >> + return 1
> >> + fi
> >> + if ! perf script -i "${perfdatafile}" --itrace=b -Fperiod,event | \
> >> + awk 'BEGIN {paused=1;branches=0}
> >> + /Resume/ {paused=0}
> >> + /branches/ {if (paused) exit 1;branches=1}
> >> + /Pause/ {paused=1}
> >> + END {if (!branches) exit 1}' ; then
> >> + echo "perf record with pause / resume failed"
> >> + return 1
> >> + fi
> >> + echo OK
> >
> > Hi,
> >
> > this new test is now constantly making "Miscellaneous Intel PT testing" fail:
> >
> > ```
> > ...
> > --- Test with pause / resume ---
> > Error:
> > Failure to open event 'intel_pt/aux-action=start-paused/u' on PMU
> > 'intel_pt' which will be removed.
> > The 'aux_action' feature is not supported, update the kernel.
> > Linux
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.003 MB
> > /tmp/perf-test-intel-pt-sh.Hs8jcq0ADc/test-perf.data ]
> > Error:
> > Failure to open event 'intel_pt/aux-action=start-paused/u' on PMU
> > 'intel_pt' which will be removed.
> > The 'aux_action' feature is not supported, update the kernel.
> > Error:
> > Failure to open event 'Resume' on PMU 'cpu' which will be removed.
> > The 'aux_action' feature is not supported, update the kernel.
> > Error:
> > Failure to open event 'Pause' on PMU 'cpu' which will be removed.
> > The 'aux_action' feature is not supported, update the kernel.
> > Linux
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.005 MB
> > /tmp/perf-test-intel-pt-sh.Hs8jcq0ADc/test-perf.data ]
> > perf record with pause / resume failed
> > --- Cleaning up ---
> > ...
> > ```
> >
> > Should the fail be turned into a skip for missing kernel support?
>
> Seems to skip for me with perf-tools-next:
>
> --- Test with pause / resume ---
> Error:
> The 'aux_action' feature is not supported, update the kernel.
> SKIP: pause / resume is not supported
>
> perf version 6.13.rc2.g39c2547579aa

My mistake, I thought I tested a clean client but I was testing with:
https://lore.kernel.org/lkml/20241221192654.94344-1-irogers@xxxxxxxxxx/
The issue there is that intel-pt is opening multiple dummy events in this test:
```
Breakpoint 2, evlist__new () at util/evlist.c:89
89 struct evlist *evlist = zalloc(sizeof(*evlist));
(gdb) n
91 if (evlist != NULL)
(gdb) p evlist->core.nr_entries
$8 = 0
(gdb) p &evlist->core.nr_entries
$9 = (int *) 0x55555618d150
(gdb) watch *$9
Hardware watchpoint 3: *$9
(gdb) c
Continuing.

Hardware watchpoint 3: *$9

Old value = 0
New value = 1
perf_evlist__add (evlist=0x55555618d140, evsel=0x5555561a6910) at evlist.c:112
112 if (evlist->needs_map_propagation)
(gdb) bt
#0 perf_evlist__add (evlist=0x55555618d140, evsel=0x5555561a6910) at
evlist.c:112
#1 0x00005555557c91ba in evlist__add (evlist=0x55555618d140,
entry=0x5555561a6910)
at util/evlist.c:197
#2 0x00005555557c9320 in evlist__splice_list_tail
(evlist=0x55555618d140, list=0x7fffffffbaa8)
at util/evlist.c:218
#3 0x00005555557eb886 in __parse_events (evlist=0x55555618d140,
str=0x7fffffffe5c1 "intel_pt/aux-action=start-paused/u",
pmu_filter=0x0, err=0x7fffffffbb38,
fake_pmu=false, warn_if_reordered=true, fake_tp=false) at
util/parse-events.c:2166
#4 0x00005555557ec4b4 in parse_events_option (opt=0x5555560e5bb0
<__record_options>,
str=0x7fffffffe5c1 "intel_pt/aux-action=start-paused/u", unset=0)
at util/parse-events.c:2352
#5 0x00005555556acede in get_value (p=0x7fffffffbde0,
opt=0x5555560e5bb0 <__record_options>,
flags=1) at parse-options.c:251
#6 0x00005555556ac13f in parse_short_opt (p=0x7fffffffbde0,
options=0x5555560e5bb0 <__record_options>) at parse-options.c:351
#7 0x00005555556aaecb in parse_options_step (ctx=0x7fffffffbde0,
options=0x5555560e5bb0 <__record_options>, usagestr=0x5555560b5bb0
<__record_usage>)
at parse-options.c:539
#8 0x00005555556aa940 in parse_options_subcommand (argc=4, argv=0x7fffffffe310,
options=0x5555560e5bb0 <__record_options>, subcommands=0x0,
usagestr=0x5555560b5bb0 <__record_usage>, flags=2) at parse-options.c:654
#9 0x00005555556ab3d7 in parse_options (argc=4, argv=0x7fffffffe310,
options=0x5555560e5bb0 <__record_options>, usagestr=0x5555560b5bb0
<__record_usage>, flags=2)
at parse-options.c:689
#10 0x00005555555f23dd in cmd_record (argc=4, argv=0x7fffffffe310) at
builtin-record.c:4021
#11 0x000055555569d42f in run_builtin (p=0x5555560f0cb0 <commands+288>, argc=4,
argv=0x7fffffffe310) at perf.c:351
#12 0x000055555569cae3 in handle_internal_command (argc=4,
argv=0x7fffffffe310) at perf.c:404
#13 0x000055555569d2ff in run_argv (argcp=0x7fffffffe15c,
argv=0x7fffffffe150) at perf.c:448
#14 0x000055555569c73f in main (argc=4, argv=0x7fffffffe310) at perf.c:560
(gdb) c
Continuing.

Breakpoint 2, evlist__new () at util/evlist.c:89
89 struct evlist *evlist = zalloc(sizeof(*evlist));
(gdb) bt
#0 evlist__new () at util/evlist.c:89
#1 0x00005555558a8365 in perf_do_probe_api (fn=0x5555558a7e50
<perf_probe_context_switch>,
cpu=..., str=0x5555559ba4fb "cycles:u") at util/perf_api_probe.c:22
#2 0x00005555558a7c94 in perf_probe_api (fn=0x5555558a7e50
<perf_probe_context_switch>)
at util/perf_api_probe.c:74
#3 0x00005555558a7e21 in perf_can_record_switch_events () at
util/perf_api_probe.c:124
#4 0x000055555597273e in intel_pt_recording_options
(itr=0x5555561a9100, evlist=0x55555618d140,
opts=0x5555560e7b58 <record+320>) at arch/x86/util/intel-pt.c:793
#5 0x00005555558be64d in auxtrace_record__options
(itr=0x5555561a9100, evlist=0x55555618d140,
opts=0x5555560e7b58 <record+320>) at util/auxtrace.c:619
#6 0x00005555555f2da3 in cmd_record (argc=1, argv=0x7fffffffe310) at
builtin-record.c:4226
#7 0x000055555569d42f in run_builtin (p=0x5555560f0cb0 <commands+288>, argc=4,
argv=0x7fffffffe310) at perf.c:351
#8 0x000055555569cae3 in handle_internal_command (argc=4,
argv=0x7fffffffe310) at perf.c:404
#9 0x000055555569d2ff in run_argv (argcp=0x7fffffffe15c,
argv=0x7fffffffe150) at perf.c:448
#10 0x000055555569c73f in main (argc=4, argv=0x7fffffffe310) at perf.c:560
(gdb) c
Continuing.

Hardware watchpoint 3: *$9

Old value = 1
New value = 2
perf_evlist__add (evlist=0x55555618d140, evsel=0x5555561a6c10) at evlist.c:112
112 if (evlist->needs_map_propagation)
(gdb) bt
#0 perf_evlist__add (evlist=0x55555618d140, evsel=0x5555561a6c10) at
evlist.c:112
#1 0x00005555557c91ba in evlist__add (evlist=0x55555618d140,
entry=0x5555561a6c10)
at util/evlist.c:197
#2 0x00005555557c980a in evlist__add_aux_dummy
(evlist=0x55555618d140, system_wide=true)
at util/evlist.c:299
#3 0x0000555555974297 in evlist__add_dummy_on_all_cpus (evlist=0x55555618d140)
at arch/x86/util/../../../util/evlist.h:111
#4 0x00005555559727ae in intel_pt_recording_options
(itr=0x5555561a9100, evlist=0x55555618d140,
opts=0x5555560e7b58 <record+320>) at arch/x86/util/intel-pt.c:800
#5 0x00005555558be64d in auxtrace_record__options
(itr=0x5555561a9100, evlist=0x55555618d140,
opts=0x5555560e7b58 <record+320>) at util/auxtrace.c:619
#6 0x00005555555f2da3 in cmd_record (argc=1, argv=0x7fffffffe310) at
builtin-record.c:4226
#7 0x000055555569d42f in run_builtin (p=0x5555560f0cb0 <commands+288>, argc=4,
argv=0x7fffffffe310) at perf.c:351
#8 0x000055555569cae3 in handle_internal_command (argc=4,
argv=0x7fffffffe310) at perf.c:404
#9 0x000055555569d2ff in run_argv (argcp=0x7fffffffe15c,
argv=0x7fffffffe150) at perf.c:448
#10 0x000055555569c73f in main (argc=4, argv=0x7fffffffe310) at perf.c:560
(gdb) c
Continuing.

Hardware watchpoint 3: *$9

Old value = 2
New value = 3
perf_evlist__add (evlist=0x55555618d140, evsel=0x5555561a6ef0) at evlist.c:112
112 if (evlist->needs_map_propagation)
(gdb) bt
#0 perf_evlist__add (evlist=0x55555618d140, evsel=0x5555561a6ef0) at
evlist.c:112
#1 0x00005555557c91ba in evlist__add (evlist=0x55555618d140,
entry=0x5555561a6ef0)
at util/evlist.c:197
#2 0x00005555557c980a in evlist__add_aux_dummy
(evlist=0x55555618d140, system_wide=false)
at util/evlist.c:299
#3 0x00005555559729f8 in intel_pt_recording_options
(itr=0x5555561a9100, evlist=0x55555618d140,
opts=0x5555560e7b58 <record+320>) at arch/x86/util/intel-pt.c:865
#4 0x00005555558be64d in auxtrace_record__options
(itr=0x5555561a9100, evlist=0x55555618d140,
opts=0x5555560e7b58 <record+320>) at util/auxtrace.c:619
#5 0x00005555555f2da3 in cmd_record (argc=1, argv=0x7fffffffe310) at
builtin-record.c:4226
#6 0x000055555569d42f in run_builtin (p=0x5555560f0cb0 <commands+288>, argc=4,
argv=0x7fffffffe310) at perf.c:351
#7 0x000055555569cae3 in handle_internal_command (argc=4,
argv=0x7fffffffe310) at perf.c:404
#8 0x000055555569d2ff in run_argv (argcp=0x7fffffffe15c,
argv=0x7fffffffe150) at perf.c:448
#9 0x000055555569c73f in main (argc=4, argv=0x7fffffffe310) at perf.c:560
(gdb)
```

That is a dummy event is added in 2 places:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/arch/x86/util/intel-pt.c?h=perf-tools-next#n800
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/arch/x86/util/intel-pt.c?h=perf-tools-next#n865

I'll need to update my patch set for things outside of record adding
dummy events, but I'm curious if the intel-pt code could share a
single dummy evsel?

Thanks,
Ian

Next message: pr-tracker-bot: "Re: [GIT PULL] Devicetree fixes for v6.13, part 1"
Previous message: Marc Kleine-Budde: "Re: [PATCH v6 0/7] can: m_can: Add am62 wakeup support"
In reply to: Adrian Hunter: "Re: [PATCH V16 7/7] perf intel-pt: Add a test for pause / resume"
Next in thread: Adrian Hunter: "Re: [PATCH V16 7/7] perf intel-pt: Add a test for pause / resume"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]