Re: [PATCH v1 1/4] perf parse-events: Remove BPF event support
From: Arnaldo Carvalho de Melo
Date: Thu Oct 19 2023 - 17:08:42 EST
Em Thu, Oct 19, 2023 at 12:45:08PM -0700, Manu Bretelle escreveu:
> cc @quentin
>
> On Fri, Aug 11, 2023 at 12:41:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Aug 11, 2023 at 11:43:22AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Right now it is not applying due to some clash with other changes and
> > > when I tried to apply it manually there were some formatting issues:
> > >
> > > ⬢[acme@toolbox perf-tools-next]$ head ~/wb/1.patch
> > > From SRS0=EALy=D3=flex--irogers.bounces.google.com=3IDHVZAcKBAUnwtljwxlttlqj.htrfhrjpjwsjq.twl@xxxxxxxxxx Thu Aug 10 17:53:46 2023
> > > Delivered-To: arnaldo.melo@xxxxxxxxx
> > > Received: from imap.gmail.com [64.233.186.109]
> > > by quaco with IMAP (fetchmail-6.4.37)
> > > for <acme@localhost> (single-drop); Thu, 10 Aug 2023 17:53:46 -0300 (-03)
> > > Received: by 2002:a0c:ab03:0:b0:63d:780e:9480 with SMTP id h3csp908198qvb;
> > > Thu, 10 Aug 2023 11:49:52 -0700 (PDT)
> > > X-Google-Smtp-Source: AGHT+IH9N/knUCyQ0tQ2Q0XBH0gqf8A8DB8/37YHWAJDKBmz7AGSV9CvCKYDuE3EwxriZFBwtZMs
> > > X-Received: by 2002:a4a:6b4f:0:b0:56c:b2ab:9820 with SMTP id
> > > h15-20020a4a6b4f000000b0056cb2ab9820mr2695332oof.8.1691693392493; Thu, 10 Aug
> > > ⬢[acme@toolbox perf-tools-next]$ patch -p1 < ~/wb/1.patch
> > > patching file tools/perf/Documentation/perf-config.txt
> > > patch: **** malformed patch at line 234: ith
> > >
> > > ⬢[acme@toolbox perf-tools-next]$
> > >
> > > I'm trying to apply it manually.
> >
> > I have this extracted from this patch as the first patch in the series:
> >
> > >From adc61b5774a9de62f34d593f164ca02daa6fb44c Mon Sep 17 00:00:00 2001
> > From: Ian Rogers <irogers@xxxxxxxxxx>
> > Date: Fri, 11 Aug 2023 12:19:48 -0300
> > Subject: [PATCH 1/1] perf bpf: Remove support for embedding clang for
> > compiling BPF events (-e foo.c)
> >
> > This never was in the default build for perf, is difficult to maintain
> > as it uses clang/llvm internals so ditch it, keeping, for now, the
> > external compilation of .c BPF into .o bytecode and its subsequent
> > loading, that is also going to be removed, do it separately to help
> > bisection and to properly document what is being removed and why.
> >
> > Committer notes:
> >
> > Extracted from a larger patch and removed some leftovers, namely
> > deleting these now unused feature tests:
> >
> > tools/build/feature/test-clang.cpp
> > tools/build/feature/test-cxx.cpp
> > tools/build/feature/test-llvm-version.cpp
> > tools/build/feature/test-llvm.cpp
> >
>
> This seem to have broken `llvm` feature detection for `bpftool`.
>
> The feature detections are still available in `tools/build/Makefile.feature` [0]
> but the .cpp files are gone.
>
> `bpftool` still rely on the `llvm` feature:
>
> $ git --no-pager grep 'feature-llvm'
> tools/bpf/bpftool/Makefile:ifeq ($(feature-llvm),1)
>
> The result of testing llvm feature is:
>
> $ cat tools/build/feature/test-llvm.make.output
> cc1plus: fatal error: test-llvm.cpp: No such file or directory
> compilation terminated.
>
> With current head:
>
> make -j $((4*$(nproc))) -C tools/bpf/bpftool && ./tools/bpf/bpftool/bpftool --version
> ...
> Auto-detecting system features:
> ... clang-bpf-co-re: [ on ]
> ... llvm: [ OFF ]
> ... libcap: [ on ]
> ... libbfd: [ on ]
> ...
> ...
> ...
> bpftool v7.3.0
> using libbpf v1.3
> features: libbfd, skeletons
>
> After applying
>
> git show 56b11a2126bf2f422831ecf6112b87a4485b221b tools/build/feature | \
> patch -p1 -R
Ouch, so probably we need just to reintroduce that one
tools/build/feature/test-llvm.cpp file.
Building perf these days ends up using bpftool, and the end result as
noticed with me testing perf, perf trace with bpf, etc didn't change, so
I didn't notice :-\
And:
ifeq ($(feature-llvm),1)
# If LLVM is available, use it for JIT disassembly
CFLAGS += -DHAVE_LLVM_SUPPORT
LLVM_CONFIG_LIB_COMPONENTS := mcdisassembler all-targets
CFLAGS += $(shell $(LLVM_CONFIG) --cflags --libs $(LLVM_CONFIG_LIB_COMPONENTS))
LIBS += $(shell $(LLVM_CONFIG) --libs $(LLVM_CONFIG_LIB_COMPONENTS))
ifeq ($(shell $(LLVM_CONFIG) --shared-mode),static)
LIBS += $(shell $(LLVM_CONFIG) --system-libs $(LLVM_CONFIG_LIB_COMPONENTS))
LIBS += -lstdc++
endif
LDFLAGS += $(shell $(LLVM_CONFIG) --ldflags)
else
# Fall back on libbfd
ifeq ($(feature-libbfd),1)
LIBS += -lbfd -ldl -lopcodes
else ifeq ($(feature-libbfd-liberty),1)
LIBS += -lbfd -ldl -lopcodes -liberty
else ifeq ($(feature-libbfd-liberty-z),1)
LIBS += -lbfd -ldl -lopcodes -liberty -lz
endif
# If one of the above feature combinations is set, we support libbfd
ifneq ($(filter -lbfd,$(LIBS)),)
CFLAGS += -DHAVE_LIBBFD_SUPPORT
# Libbfd interface changed over time, figure out what we need
ifeq ($(feature-disassembler-four-args), 1)
CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
endif
ifeq ($(feature-disassembler-init-styled), 1)
CFLAGS += -DDISASM_INIT_STYLED
endif
endif
endif
And there is a fallback to using binutils, so most people ended up not
noticing.
I wonder how to improve the current situation to detect these kinds of
problems in the future, i.e. how to notice that some file needed by some
Makefile, etc got removed or that some feature test fails because some
change in the test .c files makes them fail and thus activates fallbacks
like the one above :-\
So if I just get this back:
⬢[acme@toolbox perf-tools-next]$ cat tools/build/feature/test-llvm.cpp
// SPDX-License-Identifier: GPL-2.0
#include "llvm/Support/ManagedStatic.h"
#include "llvm/Support/raw_ostream.h"
#define NUM_VERSION (((LLVM_VERSION_MAJOR) << 16) + (LLVM_VERSION_MINOR << 8) + LLVM_VERSION_PATCH)
#if NUM_VERSION < 0x030900
# error "LLVM version too low"
#endif
int main()
{
llvm::errs() << "Hello World!\n";
llvm::llvm_shutdown();
return 0;
}
⬢[acme@toolbox perf-tools-next]$
And install the llvm-devel package then it back working:
⬢[acme@toolbox perf-tools-next]$ make -C tools/bpf/bpftool
make: Entering directory '/home/acme/git/perf-tools-next/tools/bpf/bpftool'
Auto-detecting system features:
... clang-bpf-co-re: [ on ]
... llvm: [ on ]
... libcap: [ on ]
... libbfd: [ on ]
<SNIP>
⬢[acme@toolbox perf-tools-next]$ cat tools/build/feature/test-llvm.make.output
⬢[acme@toolbox perf-tools-next]$ ls -la tools/build/feature/test-llvm.
test-llvm.bin test-llvm.cpp test-llvm.d test-llvm.make.output
⬢[acme@toolbox perf-tools-next]$ ls -la tools/build/feature/test-llvm.bin
-rwxr-xr-x. 1 acme acme 17712 Oct 19 18:04 tools/build/feature/test-llvm.bin
⬢[acme@toolbox perf-tools-next]$ ldd tools/build/feature/test-llvm.bin
linux-vdso.so.1 (0x00007ffcaf5d9000)
libLLVM-16.so => /lib64/libLLVM-16.so (0x00007fc4faefa000)
libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007fc4faca6000)
libm.so.6 => /lib64/libm.so.6 (0x00007fc4fabc5000)
libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007fc4faba1000)
libc.so.6 => /lib64/libc.so.6 (0x00007fc4fa9c3000)
libffi.so.8 => /lib64/libffi.so.8 (0x00007fc4fa9b7000)
libedit.so.0 => /lib64/libedit.so.0 (0x00007fc4fa978000)
libz.so.1 => /lib64/libz.so.1 (0x00007fc4fa95e000)
libtinfo.so.6 => /lib64/libtinfo.so.6 (0x00007fc4fa92b000)
/lib64/ld-linux-x86-64.so.2 (0x00007fc502404000)
⬢[acme@toolbox perf-tools-next]$ sudo dnf install llvm-devel
I'll get this merged in my perf-tools-fixes-for-v6.6 that I'll submit
tomorrow to Linus, thanks for reporting!
I'll add your:
Reported-by: Manu Bretelle <chantr4@xxxxxxxxx>
And:
Fixes: 56b11a2126bf2f42 ("perf bpf: Remove support for embedding clang for compiling BPF events (-e foo.c)")
Ok?
- Arnaldo