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

Charlie Jenkins
2025-02-03T23:02:41Z

On Mon, Feb 03, 2025 at 12:54:59PM -0800, Ian Rogers wrote:
> On Mon, Feb 3, 2025 at 12:06 PM Charlie Jenkins <charlie@rivosinc.com> wrote:
> >
> > On Mon, Feb 03, 2025 at 11:39:01AM -0800, Ian Rogers wrote:
> > > On Mon, Feb 3, 2025 at 11:15 AM Charlie Jenkins <charlie@rivosinc.com> wrote:
> > > >
> > > > On Mon, Feb 03, 2025 at 11:10:49AM -0800, Ian Rogers wrote:
> > > > > On Mon, Feb 3, 2025 at 11:02 AM Charlie Jenkins <charlie@rivosinc.com> wrote:
> > > > > >
> > > > > > On Fri, Jan 31, 2025 at 11:14:48PM -0800, Ian Rogers 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.
> > > > > >
> > > > > > Our approaches are very different but I sent out a patch to do this a
> > > > > > couple of weeks ago [1].
> > > > > >
> > > > > > Did you look at that and decide you didn't like the approach?
> > > > >
> > > > > Missing link?
> > > >
> > > > Whoops here is the link:
> > > >
> > > > https://lore.kernel.org/all/20250114-perf_syscall_arch_runtime-v1-1-5b304e408e11@rivosinc.com/
> > >
> > > I agree it is similar and progress, I think I prefer my changes :-)
> > >
> > > Anything in the arch directories is only going to be built given a
> > > Makefile SRCARCH:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/arch/Build?h=perf-tools-next#n2
> > > which means if we want the system call tables for say ARM on x86-64 we
> > > need to reinvent the build - this isn't a problem today but perhaps
> > > something to consider if we wanted a cross-architecture perf trace
> > > record. I'd like to do away entirely with the arch directory for this
> > > kind of reason. For example, if we run perf on a user space emulator
> > > (say a testing set up) and it sees AMD data fabric PMUs, logic for
> > > those PMUs currently lives in the arch directory for x86 and likely
> > > wasn't even built into the perf tool. We should be able to identify
> > > PMUs and act accordingly based on their names. In the case of the
> > > overloaded "cpu" PMU we can identify it with the CPUID. The less code
> > > in arch and the fewer architecture ifdefs, the better things should
> > > work cross-architecture, in testing scenarios, etc. We also win by
> > > getting code coverage without, for example, having to build and test
> > > on csky or super H.
> > >
> > > Your changes are using the string for the architecture name. I think
> > > that string is something we should get rid of:
> > > https://lore.kernel.org/lkml/CAP-5=fX2BtFzhGLCSqO1QszqfX=HT8RrTdG_5Ttp5Gw4seSstA@mail.gmail.com/
> > > but there's no reason we couldn't clean that up on top of your change.
> >
> > Yes that part is not ideal.
> >
> > >
> > > The build with lots of separate Build/Makefiles is something of a pain
> > > to port/maintain in a bazel version of the build I maintain at Google
> > > (happy to open source if anyone cares). In these changes you just run
> > > a big script and get a big header file out the end, which largely
> > > matches the other perf trace beauty things we build.
> >
> > Fair enough! I wanted to mimic how the kernel was building the syscall
> > tables as closely as possible, it's convenient to have cohesion.
>
> I think it makes sense in the kernel, as the built binary doesn't have
> cross-platform concerns. This is probably also the reason why the perf
> tool has an arch directory. Let me know what you think is the right
> direction for the perf tool syscall table code.

I am hesitant about moving all of the arch-specific syscall generation
flags into a single file. There is currently a really clean separation
between the architectures and it's possible to generate all of the
syscall tables for all of the architecutures based on the paths to the
syscall table. What causes the arch directory to be a pain for Bazel? I
guess I am mostly confused why it makes sense to change the kernel
Makefiles in order to accomidate a rewrite of the build system in Bazel
that isn't planned to be used upstream.

- Charlie

>
> > What is the usecase for the bazel version?
>
> I'm sure there's more info in:
> https://bazel.build/
> but the main thing is having hermetic and reproducible builds. The
> main problem with open sourcing would be dealing with external
> repositories, test set up, etc. Tbh, hermetic builds are a challenge
> as things like lex and yacc will vary the C code they generate based
> on the platform they run on. Bazel has plans to change the world so
> maybe this could be more of a thing in the future.
>
> Fwiw, I did push changes into the perf tree so that man page building
> was reproducible:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/commit/tools/perf/Documentation?h=perf-tools-next&id=d586ac10ce56b2381b8e1d8ed74660c1b2b8ab0d
> The man pages used to reflect the date/time they were built, but now
> reflect the date the of the commit of the file being built:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/Documentation/Makefile?h=perf-tools-next#n255
>
> Thanks,
> Ian
>
> > - Charlie
> >
> > >
> > > Thanks,
> > > Ian
> > >
> > > > >
> > > > > The patches generating a syscall(_32|_64)?.h landed. These changes
> > > > > take your changes and make it so that we just run the script once
> > > > > building a header file for all architectures. On x86 we then have the
> > > > > tables be guarded by ifdefs on i386 and x86-64. Then rather than the
> > > > > table just matching the host architecture the ELF machine is used for
> > > > > the executable running.
> > > > >
> > > > > Thanks,
> > > > > Ian
> > > > >
> > > > > > - Charlie
> > > > > >
> > > > > > >
> > > > > > > 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
> > > > > > >
> > > > > > > tools/perf/Makefile.perf | 10 +-
> > > > > > > tools/perf/arch/alpha/entry/syscalls/Kbuild | 2 -
> > > > > > > .../alpha/entry/syscalls/Makefile.syscalls | 5 -
> > > > > > > tools/perf/arch/alpha/include/syscall_table.h | 2 -
> > > > > > > tools/perf/arch/arc/entry/syscalls/Kbuild | 2 -
> > > > > > > .../arch/arc/entry/syscalls/Makefile.syscalls | 3 -
> > > > > > > tools/perf/arch/arc/include/syscall_table.h | 2 -
> > > > > > > tools/perf/arch/arm/entry/syscalls/Kbuild | 4 -
> > > > > > > .../arch/arm/entry/syscalls/Makefile.syscalls | 2 -
> > > > > > > tools/perf/arch/arm/include/syscall_table.h | 2 -
> > > > > > > tools/perf/arch/arm64/entry/syscalls/Kbuild | 3 -
> > > > > > > .../arm64/entry/syscalls/Makefile.syscalls | 6 -
> > > > > > > tools/perf/arch/arm64/include/syscall_table.h | 8 -
> > > > > > > tools/perf/arch/csky/entry/syscalls/Kbuild | 2 -
> > > > > > > .../csky/entry/syscalls/Makefile.syscalls | 3 -
> > > > > > > tools/perf/arch/csky/include/syscall_table.h | 2 -
> > > > > > > .../perf/arch/loongarch/entry/syscalls/Kbuild | 2 -
> > > > > > > .../entry/syscalls/Makefile.syscalls | 3 -
> > > > > > > .../arch/loongarch/include/syscall_table.h | 2 -
> > > > > > > tools/perf/arch/mips/entry/syscalls/Kbuild | 2 -
> > > > > > > .../mips/entry/syscalls/Makefile.syscalls | 5 -
> > > > > > > tools/perf/arch/mips/include/syscall_table.h | 2 -
> > > > > > > tools/perf/arch/parisc/entry/syscalls/Kbuild | 3 -
> > > > > > > .../parisc/entry/syscalls/Makefile.syscalls | 6 -
> > > > > > > .../perf/arch/parisc/include/syscall_table.h | 8 -
> > > > > > > tools/perf/arch/powerpc/entry/syscalls/Kbuild | 3 -
> > > > > > > .../powerpc/entry/syscalls/Makefile.syscalls | 6 -
> > > > > > > .../perf/arch/powerpc/include/syscall_table.h | 8 -
> > > > > > > tools/perf/arch/riscv/entry/syscalls/Kbuild | 2 -
> > > > > > > .../riscv/entry/syscalls/Makefile.syscalls | 4 -
> > > > > > > tools/perf/arch/riscv/include/syscall_table.h | 8 -
> > > > > > > tools/perf/arch/s390/entry/syscalls/Kbuild | 2 -
> > > > > > > .../s390/entry/syscalls/Makefile.syscalls | 5 -
> > > > > > > tools/perf/arch/s390/include/syscall_table.h | 2 -
> > > > > > > tools/perf/arch/sh/entry/syscalls/Kbuild | 2 -
> > > > > > > .../arch/sh/entry/syscalls/Makefile.syscalls | 4 -
> > > > > > > tools/perf/arch/sh/include/syscall_table.h | 2 -
> > > > > > > tools/perf/arch/sparc/entry/syscalls/Kbuild | 3 -
> > > > > > > .../sparc/entry/syscalls/Makefile.syscalls | 5 -
> > > > > > > tools/perf/arch/sparc/include/syscall_table.h | 8 -
> > > > > > > tools/perf/arch/x86/entry/syscalls/Kbuild | 3 -
> > > > > > > .../arch/x86/entry/syscalls/Makefile.syscalls | 6 -
> > > > > > > tools/perf/arch/x86/include/syscall_table.h | 8 -
> > > > > > > tools/perf/arch/xtensa/entry/syscalls/Kbuild | 2 -
> > > > > > > .../xtensa/entry/syscalls/Makefile.syscalls | 4 -
> > > > > > > .../perf/arch/xtensa/include/syscall_table.h | 2 -
> > > > > > > tools/perf/builtin-trace.c | 275 +++++++++++-------
> > > > > > > tools/perf/scripts/Makefile.syscalls | 61 ----
> > > > > > > tools/perf/scripts/syscalltbl.sh | 86 ------
> > > > > > > tools/perf/trace/beauty/syscalltbl.sh | 274 +++++++++++++++++
> > > > > > > tools/perf/util/syscalltbl.c | 142 ++++-----
> > > > > > > tools/perf/util/syscalltbl.h | 22 +-
> > > > > > > tools/perf/util/thread.c | 50 ++++
> > > > > > > tools/perf/util/thread.h | 14 +-
> > > > > > > 54 files changed, 598 insertions(+), 506 deletions(-)
> > > > > > > delete mode 100644 tools/perf/arch/alpha/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/alpha/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/alpha/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/arc/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/arc/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/arc/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/arm/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/arm/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/arm/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/arm64/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/arm64/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/arm64/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/csky/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/csky/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/csky/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/loongarch/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/loongarch/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/loongarch/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/mips/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/mips/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/mips/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/parisc/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/parisc/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/parisc/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/powerpc/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/powerpc/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/powerpc/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/riscv/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/riscv/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/riscv/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/s390/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/s390/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/s390/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/sh/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/sh/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/sh/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/sparc/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/sparc/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/sparc/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/x86/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/x86/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/x86/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/arch/xtensa/entry/syscalls/Kbuild
> > > > > > > delete mode 100644 tools/perf/arch/xtensa/entry/syscalls/Makefile.syscalls
> > > > > > > delete mode 100644 tools/perf/arch/xtensa/include/syscall_table.h
> > > > > > > delete mode 100644 tools/perf/scripts/Makefile.syscalls
> > > > > > > delete mode 100755 tools/perf/scripts/syscalltbl.sh
> > > > > > > create mode 100755 tools/perf/trace/beauty/syscalltbl.sh
> > > > > > >
> > > > > > > --
> > > > > > > 2.48.1.362.g079036d154-goog
> > > > > > >