Re: [PATCH v2 38/49] perf python: Port arm-cs-trace-disasm to perf module

[Ian Rogers]

On Wed, Sep 23, 2026 at 2:00 AM Leo Yan <leo.yan@xxxxxxx> wrote:
>
> On Sun, Sep 20, 2026 at 10:06:56PM -0700, Ian Rogers wrote:
>
> [...]
>
> > ---
> >  tools/perf/python/arm-cs-trace-disasm.py      | 356 ++++++++++++++++++
>
> A new script is added under tools/perf/python/, but I don't see the
> legacy script under tools/perf/scripts/python/ being removed.
>
> Should we maintain two copies of the script for the same purpose?
>
> > --- a/tools/perf/tests/shell/coresight/test_arm_coresight_disasm.sh
> > +++ b/tools/perf/tests/shell/coresight/test_arm_coresight_disasm.sh
>
> > @@ -44,8 +56,8 @@ branch_search='[[:space:]](bl|b(\.(eq|ne|cs|cc|mi|pl|vs|vc|hi|ls|ge|lt|gt|le|al)
> >  if [ "$(id -u)" == 0 ] && [ -e /proc/kcore ]; then
> >       echo "Testing kernel disassembly"
> >       perf record -o ${perfdata} -e cs_etm//k --kcore -Se -m,64K -- touch $file > /dev/null 2>&1
> > -     perf script -i ${perfdata} --itrace=b -s python:${script_path} -- \
> > -             -d --stop-sample=2 -k ${perfdata}/kcore_dir/kcore 2> /dev/null > ${file}
> > +     $PYTHON ${script_path} -i ${perfdata} --itrace=b -d --stop-sample=2 \
> > +             -k ${perfdata}/kcore_dir/kcore 2> /dev/null > ${file}
> >       grep -q -E ${branch_search} ${file}
> >       echo "Found kernel branches"
> >  else
> > @@ -56,8 +68,8 @@ fi
> >  ## Test user ##
> >  echo "Testing userspace disassembly"
> >  perf record -o ${perfdata} -e cs_etm//u -Se -m,64K -- touch $file > /dev/null 2>&1
> > -perf script -i ${perfdata} --itrace=b -s python:${script_path} -- \
> > -     -d --stop-sample=2 2> /dev/null > ${file}
> > +perf inject --itrace=b -i ${perfdata} -o ${perfdata2}
> > +$PYTHON ${script_path} -i ${perfdata2} -d --stop-sample=2 2> /dev/null > ${file}
>
> The perf script command is replaced with perf inject and a Python
> command. I learned from the above that we can pass the itrace option to
> the Python script.
>
> This is fine with me. I'm just curious why we don't use a single Python
> command here.

A single Python command that would handle both the itrace and file
processing? We could. Having the script command also perform an
inject-like rewrite of the data seemed less obvious to me than just
having perf inject do the work and then running the script. It does
mean a change for people using the script, and two commands could be
less convenient than one. We could expand the Python module "session"
code to include the ability to rewrite itrace data specifically for
this script. I'm hoping that we can have more composable perf_tools in
the code, similar to the ASLR code. It would be nice to add itrace
support to the Python code with composing tools in mind. I was just
putting off worrying about this for now.

One effect of not using the inject command with itrace was that it had
accidentally broken for intel-pt over time as we lacked testing
coverage.


> >  grep -q -E ${branch_search} ${file}
> >  echo "Found userspace branches"
> >
> > diff --git a/tools/perf/tests/shell/lib/setup_python.sh b/tools/perf/tests/shell/lib/setup_python.sh
> > index 2173215a0517..220d9663f81a 100644
> > --- a/tools/perf/tests/shell/lib/setup_python.sh
> > +++ b/tools/perf/tests/shell/lib/setup_python.sh
> > @@ -18,6 +18,10 @@ fi
> >  # Set PYTHONPATH to find the in-tree built perf.so first, avoiding system-wide perf.so
> >  if [ -n "$PERF_EXEC_PATH" ] && [ -d "$PERF_EXEC_PATH/python" ]; then
> >    PYTHONPATH_DIR="$PERF_EXEC_PATH/python"
> > +elif [ -n "${BASH_SOURCE[0]}" ] && [ -d "$(dirname "${BASH_SOURCE[0]}")/../../../python" ]; then
> > +  PYTHONPATH_DIR="$(dirname "${BASH_SOURCE[0]}")/../../../python"
> > +elif [ -d "$(dirname "$0")/../../../python" ]; then
> > +  PYTHONPATH_DIR="$(dirname "$0")/../../../python"
>
> Not sure if this is specific to arm-cs-trace-disasm.py. Would this be a
> common change?

Agreed. Will fix in v3.

Thanks,
Ian

> Thanks,
> Leo