linux-csky.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Charlie Jenkins <charlie@rivosinc.com>
Cc: "Namhyung Kim" <namhyung@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Alexander Shishkin" <alexander.shishkin@linux.intel.com>,
	"Jiri Olsa" <jolsa@kernel.org>, "Ian Rogers" <irogers@google.com>,
	"Adrian Hunter" <adrian.hunter@intel.com>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Mickaël Salaün" <mic@digikod.net>,
	"Günther Noack" <gnoack@google.com>,
	"Christian Brauner" <brauner@kernel.org>,
	"Guo Ren" <guoren@kernel.org>,
	"John Garry" <john.g.garry@oracle.com>,
	"Will Deacon" <will@kernel.org>,
	"James Clark" <james.clark@linaro.org>,
	"Mike Leach" <mike.leach@linaro.org>,
	"Leo Yan" <leo.yan@linux.dev>, "Jonathan Corbet" <corbet@lwn.net>,
	"Arnd Bergmann" <arnd@arndb.de>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	linux-riscv@lists.infradead.org,
	linux-security-module@vger.kernel.org, bpf@vger.kernel.org,
	linux-csky@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH v5 00/16] perf tools: Use generic syscall scripts for all archs
Date: Wed, 8 Jan 2025 19:39:51 -0300	[thread overview]
Message-ID: <Z37-t9fhnmSghIPe@x1> (raw)
In-Reply-To: <Z368mNynBTDWPM6R@google.com>

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@rivosinc.com>
> > Reviewed-by: Ian Rogers <irogers@google.com>
> > Tested-by: Ian Rogers <irogers@google.com>
> 
> Acked-by: Namhyung Kim <namhyung@kernel.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@toolbox 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@toolbox 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@toolbox perf-tools-next]$ mv perf-6.13.0-rc2.tar.xz ~
⬢ [acme@toolbox perf-tools-next]$ cd ~
⬢ [acme@toolbox ~]$ 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@toolbox ~]$ cd perf-6.13.0-rc2/
⬢ [acme@toolbox 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@toolbox perf-6.13.0-rc2]$ 

This would probably (it does, just tested, but read on) make it work:

⬢ [acme@toolbox 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@toolbox perf-tools-next]$

As now we would find it, but then it references some other part of the
kernel's Kbuild system:

⬢ [acme@toolbox perf-tools-next]$ grep -w srctree scripts/Kbuild.include
build := -f $(srctree)/scripts/Makefile.build obj
clean := -f $(srctree)/scripts/Makefile.clean obj
⬢ [acme@toolbox perf-tools-next]$

And perf has:

⬢ [acme@toolbox perf-tools-next]$ find tools/ -name Makefile.build
tools/build/Makefile.build
⬢ [acme@toolbox perf-tools-next]$

And we also have:

⬢ [acme@toolbox 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@toolbox perf-tools-next]$

And:

⬢ [acme@toolbox 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@toolbox perf-tools-next]$

That is also in:

⬢ [acme@toolbox 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@toolbox 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
:-)

Best regards,

- Arnaldo

  reply	other threads:[~2025-01-08 22:39 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-08  2:07 [PATCH v5 00/16] perf tools: Use generic syscall scripts for all archs Charlie Jenkins
2025-01-08  2:07 ` [PATCH v5 01/16] perf tools: Create generic syscall table support Charlie Jenkins
2025-01-08  2:07 ` [PATCH v5 02/16] perf tools: arc: Support generic syscall headers Charlie Jenkins
2025-01-08  2:07 ` [PATCH v5 03/16] perf tools: csky: " Charlie Jenkins
2025-01-08  2:07 ` [PATCH v5 04/16] perf tools: arm: Support " Charlie Jenkins
2025-01-08  2:07 ` [PATCH v5 05/16] perf tools: sh: " Charlie Jenkins
2025-01-08  2:07 ` [PATCH v5 06/16] perf tools: sparc: " Charlie Jenkins
2025-01-08  2:07 ` [PATCH v5 07/16] perf tools: xtensa: Support syscall header Charlie Jenkins
2025-01-08  2:07 ` [PATCH v5 08/16] perf tools: x86: Use generic syscall scripts Charlie Jenkins
2025-01-08  2:07 ` [PATCH v5 09/16] perf tools: alpha: Support syscall header Charlie Jenkins
2025-01-08  2:07 ` [PATCH v5 10/16] perf tools: parisc: " Charlie Jenkins
2025-01-08  2:07 ` [PATCH v5 11/16] perf tools: arm64: Use syscall table Charlie Jenkins
2025-01-08  2:08 ` [PATCH v5 12/16] perf tools: loongarch: " Charlie Jenkins
2025-01-08  2:08 ` [PATCH v5 13/16] perf tools: mips: Use generic syscall scripts Charlie Jenkins
2025-01-08  2:08 ` [PATCH v5 14/16] perf tools: powerpc: Use generic syscall table scripts Charlie Jenkins
2025-01-08  2:08 ` [PATCH v5 15/16] perf tools: s390: " Charlie Jenkins
2025-01-08  2:08 ` [PATCH v5 16/16] perf tools: Remove dependency on libaudit Charlie Jenkins
2025-01-08 17:57 ` [PATCH v5 00/16] perf tools: Use generic syscall scripts for all archs Namhyung Kim
2025-01-08 22:39   ` Arnaldo Carvalho de Melo [this message]
2025-01-09  2:29     ` Charlie Jenkins

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z37-t9fhnmSghIPe@x1 \
    --to=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=arnd@arndb.de \
    --cc=bpf@vger.kernel.org \
    --cc=brauner@kernel.org \
    --cc=charlie@rivosinc.com \
    --cc=corbet@lwn.net \
    --cc=gnoack@google.com \
    --cc=guoren@kernel.org \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=john.g.garry@oracle.com \
    --cc=jolsa@kernel.org \
    --cc=leo.yan@linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-csky@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mic@digikod.net \
    --cc=mike.leach@linaro.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=peterz@infradead.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).