[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Z380gLRYAKVPVEet@ghost>
Date: Wed, 8 Jan 2025 18:29:20 -0800
From: Charlie Jenkins <charlie@...osinc.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Namhyung Kim <namhyung@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Mickaël Salaün <mic@...ikod.net>,
Günther Noack <gnoack@...gle.com>,
Christian Brauner <brauner@...nel.org>, Guo Ren <guoren@...nel.org>,
John Garry <john.g.garry@...cle.com>, Will Deacon <will@...nel.org>,
James Clark <james.clark@...aro.org>,
Mike Leach <mike.leach@...aro.org>, Leo Yan <leo.yan@...ux.dev>,
Jonathan Corbet <corbet@....net>, Arnd Bergmann <arnd@...db.de>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
linux-riscv@...ts.infradead.org,
linux-security-module@...r.kernel.org, bpf@...r.kernel.org,
linux-csky@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-doc@...r.kernel.org
Subject: Re: [PATCH v5 00/16] perf tools: Use generic syscall scripts for all
archs
On Wed, Jan 08, 2025 at 07:39:51PM -0300, Arnaldo Carvalho de Melo wrote:
> On Wed, Jan 08, 2025 at 09:57:44AM -0800, Namhyung Kim wrote:
> > Hello,
> >
> > On Tue, Jan 07, 2025 at 06:07:48PM -0800, Charlie Jenkins wrote:
> > > Standardize the generation of syscall headers around syscall tables.
> > > Previously each architecture independently selected how syscall headers
> > > would be generated, or would not define a way and fallback onto
> > > libaudit. Convert all architectures to use a standard syscall header
> > > generation script and allow each architecture to override the syscall
> > > table to use if they do not use the generic table.
> > >
> > > As a result of these changes, no architecture will require libaudit, and
> > > so the fallback case of using libaudit is removed by this series.
> > >
> > > Testing:
> > >
> > > I have tested that the syscall mappings of id to name generation works
> > > as expected for every architecture, but I have only validated that perf
> > > trace compiles and runs as expected on riscv, arm64, and x86_64.
> > >
> > > Signed-off-by: Charlie Jenkins <charlie@...osinc.com>
> > > Reviewed-by: Ian Rogers <irogers@...gle.com>
> > > Tested-by: Ian Rogers <irogers@...gle.com>
> >
> > Acked-by: Namhyung Kim <namhyung@...nel.org>
>
> So, somehow the first patch of this series didn't reach my inbox, b4
> found it, and in it perf now does;
>
> tools/perf/scripts/Makefile.syscalls
>
> include $(srctree)/scripts/Kbuild.include
>
> I.e. it uses a file that is outside tools/ so normal devel in the kernel
> community may end up breaking tools/ living code, something we decided
> not to have.
>
> I noticed this while doing a: "make -C tools/perf build-test", the first
> test creates a perf tarball and then tries to build it after
> uncompressing it somewhere out of the checked out kernel source tree:
>
> ⬢ [acme@...lbox perf-tools-next]$ make help | grep perf
> perf-tar-src-pkg - Build the perf source tarball with no compression
> perf-targz-src-pkg - Build the perf source tarball with gzip compression
> perf-tarbz2-src-pkg - Build the perf source tarball with bz2 compression
> perf-tarxz-src-pkg - Build the perf source tarball with xz compression
> perf-tarzst-src-pkg - Build the perf source tarball with zst compression
> ⬢ [acme@...lbox perf-tools-next]$ make perf-tarxz-src-pkg
> UPD .tmp_HEAD
> COPY .tmp_perf/HEAD
> GEN .tmp_perf/PERF-VERSION-FILE
> PERF_VERSION = 6.13.rc2.g48d3eefaa683
> ARCHIVE perf-6.13.0-rc2.tar.xz
> ⬢ [acme@...lbox perf-tools-next]$ mv perf-6.13.0-rc2.tar.xz ~
> ⬢ [acme@...lbox perf-tools-next]$ cd ~
> ⬢ [acme@...lbox ~]$ tar xvf perf-6.13.0-rc2.tar.xz | tail -5
> perf-6.13.0-rc2/tools/scripts/Makefile.include
> perf-6.13.0-rc2/tools/scripts/syscall.tbl
> perf-6.13.0-rc2/tools/scripts/utilities.mak
> perf-6.13.0-rc2/HEAD
> perf-6.13.0-rc2/PERF-VERSION-FILE
> ⬢ [acme@...lbox ~]$ cd perf-6.13.0-rc2/
> ⬢ [acme@...lbox perf-6.13.0-rc2]$ make -C tools/perf
> make: Entering directory '/home/acme/perf-6.13.0-rc2/tools/perf'
> BUILD: Doing 'make -j28' parallel build
> Warning: Skipped check-headers due to missing ../../include
>
> Auto-detecting system features:
> ... libdw: [ on ]
> ... glibc: [ on ]
> ... libbfd: [ on ]
> ... libbfd-buildid: [ on ]
> ... libelf: [ on ]
> ... libnuma: [ on ]
> ... numa_num_possible_cpus: [ on ]
> ... libperl: [ on ]
> ... libpython: [ on ]
> ... libcrypto: [ on ]
> ... libunwind: [ on ]
> ... libcapstone: [ on ]
> ... llvm-perf: [ on ]
> ... zlib: [ on ]
> ... lzma: [ on ]
> ... get_cpuid: [ on ]
> ... bpf: [ on ]
> ... libaio: [ on ]
> ... libzstd: [ on ]
>
> /home/acme/perf-6.13.0-rc2/tools/perf/scripts/Makefile.syscalls:18: /home/acme/perf-6.13.0-rc2/scripts/Kbuild.include: No such file or directory
> make[2]: *** No rule to make target '/home/acme/perf-6.13.0-rc2/scripts/Kbuild.include'. Stop.
> make[1]: *** [Makefile.perf:286: sub-make] Error 2
> make: *** [Makefile:76: all] Error 2
> make: Leaving directory '/home/acme/perf-6.13.0-rc2/tools/perf'
> ⬢ [acme@...lbox perf-6.13.0-rc2]$
>
> This would probably (it does, just tested, but read on) make it work:
>
> ⬢ [acme@...lbox perf-tools-next]$ git diff
> diff --git a/tools/perf/MANIFEST b/tools/perf/MANIFEST
> index dc42de1785cee715..83ef5d1365880929 100644
> --- a/tools/perf/MANIFEST
> +++ b/tools/perf/MANIFEST
> @@ -22,6 +22,7 @@ tools/lib/str_error_r.c
> tools/lib/vsprintf.c
> tools/lib/zalloc.c
> scripts/bpf_doc.py
> +scripts/Kbuild.include
> tools/bpf/bpftool
> kernel/bpf/disasm.c
> kernel/bpf/disasm.h
> ⬢ [acme@...lbox perf-tools-next]$
>
> As now we would find it, but then it references some other part of the
> kernel's Kbuild system:
>
> ⬢ [acme@...lbox perf-tools-next]$ grep -w srctree scripts/Kbuild.include
> build := -f $(srctree)/scripts/Makefile.build obj
> clean := -f $(srctree)/scripts/Makefile.clean obj
> ⬢ [acme@...lbox perf-tools-next]$
>
> And perf has:
>
> ⬢ [acme@...lbox perf-tools-next]$ find tools/ -name Makefile.build
> tools/build/Makefile.build
> ⬢ [acme@...lbox perf-tools-next]$
>
> And we also have:
>
> ⬢ [acme@...lbox perf-tools-next]$ ls -la tools/scripts/
> total 40
> drwxr-xr-x. 1 acme acme 106 Jan 8 19:13 .
> drwxr-xr-x. 1 acme acme 514 Jan 8 11:39 ..
> -rw-r--r--. 1 acme acme 1224 Jan 8 11:41 Makefile.arch
> -rw-r--r--. 1 acme acme 6205 Dec 20 21:48 Makefile.include
> -rw-r--r--. 1 acme acme 17401 Jan 8 19:13 syscall.tbl
> -rw-r--r--. 1 acme acme 6186 Dec 20 21:48 utilities.mak
> ⬢ [acme@...lbox perf-tools-next]$
>
> And:
>
> ⬢ [acme@...lbox perf-tools-next]$ grep -w build tools/build/Makefile.include
> build := -f $(srctree)/tools/build/Makefile.build dir=. obj
> $(SILENT_MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= $(OUTPUT)fixdep
> $(Q)$(MAKE) -C $(srctree)/tools/build clean
> ⬢ [acme@...lbox perf-tools-next]$
>
> That is also in:
>
> ⬢ [acme@...lbox perf-tools-next]$ grep -w build scripts/Kbuild.include
> # Shorthand for $(Q)$(MAKE) -f scripts/Makefile.build obj=
> # $(Q)$(MAKE) $(build)=dir
> build := -f $(srctree)/scripts/Makefile.build obj
> # the interrupted recipe. So, you can safely stop the build by Ctrl-C (Make
> # (1) PHONY targets are always build
> # (2) No target, so we better build it
> ⬢ [acme@...lbox perf-tools-next]$
>
> So it seems we need to look at what we're using from the kernel's
> scripts/Makefile.build to have it in a tools/build/ file.
>
> Its late here and I'll have to stop at this point, please take a look to
> see if this can be easily resolved so that we can merge your series, I
> very much like to say goodbye to one more tools/perf library dependency
> :-)
Thank you for pointing this out. We can use tools/build/Build.include
which seems to have everything that is required. A "space" convenience
variable needs to be added, but that is all. I'll send an updated
version with that change.
- Charlie
>
> Best regards,
>
> - Arnaldo
Powered by blists - more mailing lists