Re: [PATCH v1 0/7] perf: Support multiple system call tables in the build

Ian Rogers
2025-02-03T18:28:54Z

On Sat, Feb 1, 2025 at 12:51 AM Ian Rogers <irogers@google.com> wrote:
>
> On Fri, Jan 31, 2025 at 11:15 PM Ian Rogers <irogers@google.com> wrote:
> >
> > This work builds on the clean up of system call tables and removal of
> > libaudit by Charlie Jenkins <charlie@rivosinc.com>.
> >
> > The system call table in perf trace is used to map system call numbers
> > to names and vice versa. Prior to these changes, a single table
> > matching the perf binary's build was present. The table would be
> > incorrect if tracing say a 32-bit binary from a 64-bit version of
> > perf, the names and numbers wouldn't match.
> >
> > Change the build so that a single system call file is built and the
> > potentially multiple tables are identifiable from the ELF machine type
> > of the process being examined. To determine the ELF machine type, the
> > executable's header is read from /proc/pid/exe with fallbacks to using
> > the perf's binary type when unknown.
> >
> > Remove some runtime types used by the system call tables and make
> > equivalents generated at build time.
> >
> > Ian Rogers (7):
> > perf syscalltble: Remove syscall_table.h
> > perf trace: Reorganize syscalls
> > perf syscalltbl: Remove struct syscalltbl
> > perf thread: Add support for reading the e_machine type for a thread
> > perf trace beauty: Add syscalltbl.sh generating all system call tables
> > perf syscalltbl: Use lookup table containing multiple architectures
> > perf build: Remove Makefile.syscalls
>
> If you are looking for the improvement this series achieves, patch 6
> has sample before and after output:
> https://lore.kernel.org/lkml/20250201071455.718247-7-irogers@google.com/

I think there is a follow up to these patches to clean up the notion
of "arch" and its "name" member:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/disasm.h?h=perf-tools-next#n20
The name on x86-64 has the value "x86" and isn't known to be i386 or
x86-64, causing ambiguity to work around:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/disasm.c?h=perf-tools-next#n1898
It feels likely that rather than recording (in env) a string based on
uname and bashing it around:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/env.c?h=perf-tools-next#n440
It would be better to record the e_machine values for the set of
threads in the record, or just a set of potential e_machine values. We
can get rid of name in arch, use e_machine from some dso helper, and
then deal with this as appropriate without needing to carry around
is_64bit to deal with ambiguity.

A change like this would need its own feature flag, and helpers
between the old name and e_machine so all notions of the old style
name could be removed, etc. It may be convenient to duplicate arch
tables for i386/x86-64 rather than have disasm's arch deal with a set
of e_machines.

Thanks,
Ian