* [PATCH] perf build: Fix build error when NO_LIBBPF=1
@ 2023-01-05 17:22 Mike Leach
2023-01-05 19:02 ` Ian Rogers
0 siblings, 1 reply; 6+ messages in thread
From: Mike Leach @ 2023-01-05 17:22 UTC (permalink / raw)
To: linux-perf-users, linux-kernel, bpf, acme, irogers
Cc: peterz, mingo, mark.rutland, alexander.shishkin, jolsa, namhyung,
Mike Leach
Recent updates to perf build result in the following output when cross
compiling to aarch64, with libelf unavailable, and therefore
NO_LIBBPF=1 set.
```
$make -C tools/perf
<cut>
Makefile.config:428: No libelf found. Disables 'probe' tool, jvmti
and BPF support in 'perf record'. Please install libelf-dev,
libelf-devel or elfutils-libelf-devel
<cut>
libbpf.c:46:10: fatal error: libelf.h: No such file or directory
46 | #include <libelf.h>
| ^~~~~~~~~~
compilation terminated.
./tools/build/Makefile.build:96: recipe for target
'.tools/perf/libbpf/staticobjs/libbpf.o' failed
```
plus one other include error for <gelf.h>
The issue is that the commit noted below adds libbpf to the prepare:
target but no longer accounts for the NO_LIBBPF define. Additionally
changing the include directories means that even if the libbpf target
build is prevented, bpf headers are missing in other parts of the build.
This patch ensures that in the case of NO_LIBBPF=1, the build target is
changed to a header only target, and the headers are installed, without
attempting to build the libbpf.a target.
Applies to perf/core
Fixes: 746bd29e348f ("perf build: Use tools/lib headers from install path")
Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
tools/perf/Makefile.perf | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 13e7d26e77f0..ee08ecf469f6 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -305,7 +305,11 @@ else
endif
LIBBPF_DESTDIR = $(LIBBPF_OUTPUT)
LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include
+ifndef NO_LIBBPF
LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a
+else
+LIBBPF = $(LIBBPF_INCLUDE)/bpf/bpf.h
+endif
CFLAGS += -I$(LIBBPF_OUTPUT)/include
ifneq ($(OUTPUT),)
@@ -826,10 +830,16 @@ $(LIBAPI)-clean:
$(call QUIET_CLEAN, libapi)
$(Q)$(RM) -r -- $(LIBAPI_OUTPUT)
+ifndef NO_LIBBPF
$(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
$(Q)$(MAKE) -C $(LIBBPF_DIR) FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) \
O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \
$@ install_headers
+else
+$(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
+ $(Q)$(MAKE) -C $(LIBBPF_DIR) OUTPUT=$(LIBBPF_OUTPUT)/ \
+ DESTDIR=$(LIBBPF_DESTDIR) prefix= install_headers
+endif
$(LIBBPF)-clean:
$(call QUIET_CLEAN, libbpf)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] perf build: Fix build error when NO_LIBBPF=1
2023-01-05 17:22 [PATCH] perf build: Fix build error when NO_LIBBPF=1 Mike Leach
@ 2023-01-05 19:02 ` Ian Rogers
2023-01-05 23:40 ` Mike Leach
0 siblings, 1 reply; 6+ messages in thread
From: Ian Rogers @ 2023-01-05 19:02 UTC (permalink / raw)
To: Mike Leach
Cc: linux-perf-users, linux-kernel, bpf, acme, peterz, mingo,
mark.rutland, alexander.shishkin, jolsa, namhyung
On Thu, Jan 5, 2023 at 9:22 AM Mike Leach <mike.leach@linaro.org> wrote:
>
> Recent updates to perf build result in the following output when cross
> compiling to aarch64, with libelf unavailable, and therefore
> NO_LIBBPF=1 set.
>
> ```
> $make -C tools/perf
>
> <cut>
>
> Makefile.config:428: No libelf found. Disables 'probe' tool, jvmti
> and BPF support in 'perf record'. Please install libelf-dev,
> libelf-devel or elfutils-libelf-devel
>
> <cut>
>
> libbpf.c:46:10: fatal error: libelf.h: No such file or directory
> 46 | #include <libelf.h>
> | ^~~~~~~~~~
> compilation terminated.
>
> ./tools/build/Makefile.build:96: recipe for target
> '.tools/perf/libbpf/staticobjs/libbpf.o' failed
>
> ```
>
> plus one other include error for <gelf.h>
Ouch, apologies for the breakage. You wouldn't happen to have
something like a way with say a docker image to repro the problem? The
make line above is somewhat minimal.
> The issue is that the commit noted below adds libbpf to the prepare:
> target but no longer accounts for the NO_LIBBPF define. Additionally
> changing the include directories means that even if the libbpf target
> build is prevented, bpf headers are missing in other parts of the build.
>
> This patch ensures that in the case of NO_LIBBPF=1, the build target is
> changed to a header only target, and the headers are installed, without
> attempting to build the libbpf.a target.
>
> Applies to perf/core
>
> Fixes: 746bd29e348f ("perf build: Use tools/lib headers from install path")
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
> tools/perf/Makefile.perf | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 13e7d26e77f0..ee08ecf469f6 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -305,7 +305,11 @@ else
> endif
> LIBBPF_DESTDIR = $(LIBBPF_OUTPUT)
> LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include
> +ifndef NO_LIBBPF
> LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a
> +else
> +LIBBPF = $(LIBBPF_INCLUDE)/bpf/bpf.h
This seems strange, don't we want to avoid libbpf targets?
> +endif
> CFLAGS += -I$(LIBBPF_OUTPUT)/include
>
> ifneq ($(OUTPUT),)
> @@ -826,10 +830,16 @@ $(LIBAPI)-clean:
> $(call QUIET_CLEAN, libapi)
> $(Q)$(RM) -r -- $(LIBAPI_OUTPUT)
>
> +ifndef NO_LIBBPF
> $(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
> $(Q)$(MAKE) -C $(LIBBPF_DIR) FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) \
> O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \
> $@ install_headers
> +else
> +$(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
> + $(Q)$(MAKE) -C $(LIBBPF_DIR) OUTPUT=$(LIBBPF_OUTPUT)/ \
> + DESTDIR=$(LIBBPF_DESTDIR) prefix= install_headers
> +endif
Shouldn't we just be able to conditionalize having $(LIBBPF) as a
dependency for the perf binary? If there is no dependency then the
targets won't be built and we shouldn't need to conditionalize here.
Thanks!
Ian
> $(LIBBPF)-clean:
> $(call QUIET_CLEAN, libbpf)
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf build: Fix build error when NO_LIBBPF=1
2023-01-05 19:02 ` Ian Rogers
@ 2023-01-05 23:40 ` Mike Leach
2023-01-06 6:24 ` Ian Rogers
0 siblings, 1 reply; 6+ messages in thread
From: Mike Leach @ 2023-01-05 23:40 UTC (permalink / raw)
To: Ian Rogers
Cc: linux-perf-users, linux-kernel, bpf, acme, peterz, mingo,
mark.rutland, alexander.shishkin, jolsa, namhyung
Hi,
On Thu, 5 Jan 2023 at 19:03, Ian Rogers <irogers@google.com> wrote:
>
> On Thu, Jan 5, 2023 at 9:22 AM Mike Leach <mike.leach@linaro.org> wrote:
> >
> > Recent updates to perf build result in the following output when cross
> > compiling to aarch64, with libelf unavailable, and therefore
> > NO_LIBBPF=1 set.
> >
> > ```
> > $make -C tools/perf
> >
> > <cut>
> >
> > Makefile.config:428: No libelf found. Disables 'probe' tool, jvmti
> > and BPF support in 'perf record'. Please install libelf-dev,
> > libelf-devel or elfutils-libelf-devel
> >
> > <cut>
> >
> > libbpf.c:46:10: fatal error: libelf.h: No such file or directory
> > 46 | #include <libelf.h>
> > | ^~~~~~~~~~
> > compilation terminated.
> >
> > ./tools/build/Makefile.build:96: recipe for target
> > '.tools/perf/libbpf/staticobjs/libbpf.o' failed
> >
> > ```
> >
> > plus one other include error for <gelf.h>
>
> Ouch, apologies for the breakage. You wouldn't happen to have
> something like a way with say a docker image to repro the problem? The
> make line above is somewhat minimal.
>
Unfortunately not - I was cross compiling on my main workstation.
However, in theory
$make -C tools/perf NO_LIBBPF=1
should explicitly exclude the library from the build - which without
the fix it does not.
> > The issue is that the commit noted below adds libbpf to the prepare:
> > target but no longer accounts for the NO_LIBBPF define. Additionally
> > changing the include directories means that even if the libbpf target
> > build is prevented, bpf headers are missing in other parts of the build.
> >
> > This patch ensures that in the case of NO_LIBBPF=1, the build target is
> > changed to a header only target, and the headers are installed, without
> > attempting to build the libbpf.a target.
> >
> > Applies to perf/core
> >
> > Fixes: 746bd29e348f ("perf build: Use tools/lib headers from install path")
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > ---
> > tools/perf/Makefile.perf | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index 13e7d26e77f0..ee08ecf469f6 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -305,7 +305,11 @@ else
> > endif
> > LIBBPF_DESTDIR = $(LIBBPF_OUTPUT)
> > LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include
> > +ifndef NO_LIBBPF
> > LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a
> > +else
> > +LIBBPF = $(LIBBPF_INCLUDE)/bpf/bpf.h
>
> This seems strange, don't we want to avoid libbpf targets?
>
This is a header only target - see my continuation comment below....
> > +endif
> > CFLAGS += -I$(LIBBPF_OUTPUT)/include
> >
> > ifneq ($(OUTPUT),)
> > @@ -826,10 +830,16 @@ $(LIBAPI)-clean:
> > $(call QUIET_CLEAN, libapi)
> > $(Q)$(RM) -r -- $(LIBAPI_OUTPUT)
> >
> > +ifndef NO_LIBBPF
> > $(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
> > $(Q)$(MAKE) -C $(LIBBPF_DIR) FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) \
> > O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \
> > $@ install_headers
> > +else
> > +$(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
> > + $(Q)$(MAKE) -C $(LIBBPF_DIR) OUTPUT=$(LIBBPF_OUTPUT)/ \
> > + DESTDIR=$(LIBBPF_DESTDIR) prefix= install_headers
> > +endif
>
> Shouldn't we just be able to conditionalize having $(LIBBPF) as a
> dependency for the perf binary? If there is no dependency then the
> targets won't be built and we shouldn't need to conditionalize here.
>
I did try doing just that, but the build process does two things when
building libbpf
a) builds the library
b) installs the bpf headers in the libbpf output location.
Now what the original patch - "perf build: Use tools/lib headers from
install path" - does is to also change the include paths to the
compiler to pick up the headers,
removing the line:
INC_FLAGS += -I$(srctree)/tools/lib/
from tools/perf/Makefile.config and adding the line
CFLAGS += -I$(LIBBPF_OUTPUT)/include
in tools/perf/Makefile.perf (along with similar lines for libperf, libapi etc)
The result of this is that if you only remove the library build, the
headers are not installed and other compilation units fail as the
headers are still included even if the library is not in use.
These were originally satisfied by the now removed INC_FLAGS +=
-I$(srctree)/tools/lib.
Thus when NO_LIBBPF=1 even though we do not build the library - we
still need to install the headers to retain the consistency - hence a
"header only" target, that only installs the headers without building
the library.
This avoids restoring the original -I$(srctree)/tools/lib/, which
would potentially mess up the oher library builds that have changed
their header include paths.
Regards
Mike
> Thanks!
> Ian
>
> > $(LIBBPF)-clean:
> > $(call QUIET_CLEAN, libbpf)
> > --
> > 2.17.1
> >
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf build: Fix build error when NO_LIBBPF=1
2023-01-05 23:40 ` Mike Leach
@ 2023-01-06 6:24 ` Ian Rogers
2023-01-06 9:58 ` Mike Leach
0 siblings, 1 reply; 6+ messages in thread
From: Ian Rogers @ 2023-01-06 6:24 UTC (permalink / raw)
To: Mike Leach
Cc: linux-perf-users, linux-kernel, bpf, acme, peterz, mingo,
mark.rutland, alexander.shishkin, jolsa, namhyung
On Thu, Jan 5, 2023 at 3:40 PM Mike Leach <mike.leach@linaro.org> wrote:
>
> Hi,
>
> On Thu, 5 Jan 2023 at 19:03, Ian Rogers <irogers@google.com> wrote:
> >
> > On Thu, Jan 5, 2023 at 9:22 AM Mike Leach <mike.leach@linaro.org> wrote:
> > >
> > > Recent updates to perf build result in the following output when cross
> > > compiling to aarch64, with libelf unavailable, and therefore
> > > NO_LIBBPF=1 set.
> > >
> > > ```
> > > $make -C tools/perf
> > >
> > > <cut>
> > >
> > > Makefile.config:428: No libelf found. Disables 'probe' tool, jvmti
> > > and BPF support in 'perf record'. Please install libelf-dev,
> > > libelf-devel or elfutils-libelf-devel
> > >
> > > <cut>
> > >
> > > libbpf.c:46:10: fatal error: libelf.h: No such file or directory
> > > 46 | #include <libelf.h>
> > > | ^~~~~~~~~~
> > > compilation terminated.
> > >
> > > ./tools/build/Makefile.build:96: recipe for target
> > > '.tools/perf/libbpf/staticobjs/libbpf.o' failed
> > >
> > > ```
> > >
> > > plus one other include error for <gelf.h>
> >
> > Ouch, apologies for the breakage. You wouldn't happen to have
> > something like a way with say a docker image to repro the problem? The
> > make line above is somewhat minimal.
> >
>
> Unfortunately not - I was cross compiling on my main workstation.
> However, in theory
> $make -C tools/perf NO_LIBBPF=1
> should explicitly exclude the library from the build - which without
> the fix it does not.
>
> > > The issue is that the commit noted below adds libbpf to the prepare:
> > > target but no longer accounts for the NO_LIBBPF define. Additionally
> > > changing the include directories means that even if the libbpf target
> > > build is prevented, bpf headers are missing in other parts of the build.
> > >
> > > This patch ensures that in the case of NO_LIBBPF=1, the build target is
> > > changed to a header only target, and the headers are installed, without
> > > attempting to build the libbpf.a target.
> > >
> > > Applies to perf/core
> > >
> > > Fixes: 746bd29e348f ("perf build: Use tools/lib headers from install path")
> > > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > > ---
> > > tools/perf/Makefile.perf | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > > index 13e7d26e77f0..ee08ecf469f6 100644
> > > --- a/tools/perf/Makefile.perf
> > > +++ b/tools/perf/Makefile.perf
> > > @@ -305,7 +305,11 @@ else
> > > endif
> > > LIBBPF_DESTDIR = $(LIBBPF_OUTPUT)
> > > LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include
> > > +ifndef NO_LIBBPF
> > > LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a
> > > +else
> > > +LIBBPF = $(LIBBPF_INCLUDE)/bpf/bpf.h
> >
> > This seems strange, don't we want to avoid libbpf targets?
> >
>
> This is a header only target - see my continuation comment below....
>
> > > +endif
> > > CFLAGS += -I$(LIBBPF_OUTPUT)/include
> > >
> > > ifneq ($(OUTPUT),)
> > > @@ -826,10 +830,16 @@ $(LIBAPI)-clean:
> > > $(call QUIET_CLEAN, libapi)
> > > $(Q)$(RM) -r -- $(LIBAPI_OUTPUT)
> > >
> > > +ifndef NO_LIBBPF
> > > $(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
> > > $(Q)$(MAKE) -C $(LIBBPF_DIR) FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) \
> > > O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \
> > > $@ install_headers
> > > +else
> > > +$(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
> > > + $(Q)$(MAKE) -C $(LIBBPF_DIR) OUTPUT=$(LIBBPF_OUTPUT)/ \
> > > + DESTDIR=$(LIBBPF_DESTDIR) prefix= install_headers
> > > +endif
> >
> > Shouldn't we just be able to conditionalize having $(LIBBPF) as a
> > dependency for the perf binary? If there is no dependency then the
> > targets won't be built and we shouldn't need to conditionalize here.
> >
>
> I did try doing just that, but the build process does two things when
> building libbpf
> a) builds the library
> b) installs the bpf headers in the libbpf output location.
>
> Now what the original patch - "perf build: Use tools/lib headers from
> install path" - does is to also change the include paths to the
> compiler to pick up the headers,
> removing the line:
>
> INC_FLAGS += -I$(srctree)/tools/lib/
>
> from tools/perf/Makefile.config and adding the line
>
> CFLAGS += -I$(LIBBPF_OUTPUT)/include
>
> in tools/perf/Makefile.perf (along with similar lines for libperf, libapi etc)
>
> The result of this is that if you only remove the library build, the
> headers are not installed and other compilation units fail as the
> headers are still included even if the library is not in use.
> These were originally satisfied by the now removed INC_FLAGS +=
> -I$(srctree)/tools/lib.
>
> Thus when NO_LIBBPF=1 even though we do not build the library - we
> still need to install the headers to retain the consistency - hence a
> "header only" target, that only installs the headers without building
> the library.
>
> This avoids restoring the original -I$(srctree)/tools/lib/, which
> would potentially mess up the oher library builds that have changed
> their header include paths.
>
> Regards
>
> Mike
Thanks Mike,
The -I is needed for the libbpf headers but if NO_LIBBPF is enabled
then the C define HAVE_LIBBPF_SUPPORT isn't and we shouldn't include
any of these headers. This means updating the CFLAGS for libbpf should
only be done if we actually build the static libbpf.a, the dynamic
version's headers should already be on the include path. I sent out a
variant of this fix doing that here:
https://lore.kernel.org/lkml/20230106061631.571659-1-irogers@google.com/
Apologies again for the breakage, I can buy you a beer the next time
I'm home in Manchester.
Ian
>
> > Thanks!
> > Ian
> >
> > > $(LIBBPF)-clean:
> > > $(call QUIET_CLEAN, libbpf)
> > > --
> > > 2.17.1
> > >
>
>
>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf build: Fix build error when NO_LIBBPF=1
2023-01-06 6:24 ` Ian Rogers
@ 2023-01-06 9:58 ` Mike Leach
2023-01-06 14:36 ` Ian Rogers
0 siblings, 1 reply; 6+ messages in thread
From: Mike Leach @ 2023-01-06 9:58 UTC (permalink / raw)
To: Ian Rogers
Cc: linux-perf-users, linux-kernel, bpf, acme, peterz, mingo,
mark.rutland, alexander.shishkin, jolsa, namhyung
Hi Ian,
On Fri, 6 Jan 2023 at 06:24, Ian Rogers <irogers@google.com> wrote:
>
> On Thu, Jan 5, 2023 at 3:40 PM Mike Leach <mike.leach@linaro.org> wrote:
> >
> > Hi,
> >
> > On Thu, 5 Jan 2023 at 19:03, Ian Rogers <irogers@google.com> wrote:
> > >
> > > On Thu, Jan 5, 2023 at 9:22 AM Mike Leach <mike.leach@linaro.org> wrote:
> > > >
> > > > Recent updates to perf build result in the following output when cross
> > > > compiling to aarch64, with libelf unavailable, and therefore
> > > > NO_LIBBPF=1 set.
> > > >
> > > > ```
> > > > $make -C tools/perf
> > > >
> > > > <cut>
> > > >
> > > > Makefile.config:428: No libelf found. Disables 'probe' tool, jvmti
> > > > and BPF support in 'perf record'. Please install libelf-dev,
> > > > libelf-devel or elfutils-libelf-devel
> > > >
> > > > <cut>
> > > >
> > > > libbpf.c:46:10: fatal error: libelf.h: No such file or directory
> > > > 46 | #include <libelf.h>
> > > > | ^~~~~~~~~~
> > > > compilation terminated.
> > > >
> > > > ./tools/build/Makefile.build:96: recipe for target
> > > > '.tools/perf/libbpf/staticobjs/libbpf.o' failed
> > > >
> > > > ```
> > > >
> > > > plus one other include error for <gelf.h>
> > >
> > > Ouch, apologies for the breakage. You wouldn't happen to have
> > > something like a way with say a docker image to repro the problem? The
> > > make line above is somewhat minimal.
> > >
> >
> > Unfortunately not - I was cross compiling on my main workstation.
> > However, in theory
> > $make -C tools/perf NO_LIBBPF=1
> > should explicitly exclude the library from the build - which without
> > the fix it does not.
> >
> > > > The issue is that the commit noted below adds libbpf to the prepare:
> > > > target but no longer accounts for the NO_LIBBPF define. Additionally
> > > > changing the include directories means that even if the libbpf target
> > > > build is prevented, bpf headers are missing in other parts of the build.
> > > >
> > > > This patch ensures that in the case of NO_LIBBPF=1, the build target is
> > > > changed to a header only target, and the headers are installed, without
> > > > attempting to build the libbpf.a target.
> > > >
> > > > Applies to perf/core
> > > >
> > > > Fixes: 746bd29e348f ("perf build: Use tools/lib headers from install path")
> > > > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > > > ---
> > > > tools/perf/Makefile.perf | 10 ++++++++++
> > > > 1 file changed, 10 insertions(+)
> > > >
> > > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > > > index 13e7d26e77f0..ee08ecf469f6 100644
> > > > --- a/tools/perf/Makefile.perf
> > > > +++ b/tools/perf/Makefile.perf
> > > > @@ -305,7 +305,11 @@ else
> > > > endif
> > > > LIBBPF_DESTDIR = $(LIBBPF_OUTPUT)
> > > > LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include
> > > > +ifndef NO_LIBBPF
> > > > LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a
> > > > +else
> > > > +LIBBPF = $(LIBBPF_INCLUDE)/bpf/bpf.h
> > >
> > > This seems strange, don't we want to avoid libbpf targets?
> > >
> >
> > This is a header only target - see my continuation comment below....
> >
> > > > +endif
> > > > CFLAGS += -I$(LIBBPF_OUTPUT)/include
> > > >
> > > > ifneq ($(OUTPUT),)
> > > > @@ -826,10 +830,16 @@ $(LIBAPI)-clean:
> > > > $(call QUIET_CLEAN, libapi)
> > > > $(Q)$(RM) -r -- $(LIBAPI_OUTPUT)
> > > >
> > > > +ifndef NO_LIBBPF
> > > > $(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
> > > > $(Q)$(MAKE) -C $(LIBBPF_DIR) FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) \
> > > > O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \
> > > > $@ install_headers
> > > > +else
> > > > +$(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
> > > > + $(Q)$(MAKE) -C $(LIBBPF_DIR) OUTPUT=$(LIBBPF_OUTPUT)/ \
> > > > + DESTDIR=$(LIBBPF_DESTDIR) prefix= install_headers
> > > > +endif
> > >
> > > Shouldn't we just be able to conditionalize having $(LIBBPF) as a
> > > dependency for the perf binary? If there is no dependency then the
> > > targets won't be built and we shouldn't need to conditionalize here.
> > >
> >
> > I did try doing just that, but the build process does two things when
> > building libbpf
> > a) builds the library
> > b) installs the bpf headers in the libbpf output location.
> >
> > Now what the original patch - "perf build: Use tools/lib headers from
> > install path" - does is to also change the include paths to the
> > compiler to pick up the headers,
> > removing the line:
> >
> > INC_FLAGS += -I$(srctree)/tools/lib/
> >
> > from tools/perf/Makefile.config and adding the line
> >
> > CFLAGS += -I$(LIBBPF_OUTPUT)/include
> >
> > in tools/perf/Makefile.perf (along with similar lines for libperf, libapi etc)
> >
> > The result of this is that if you only remove the library build, the
> > headers are not installed and other compilation units fail as the
> > headers are still included even if the library is not in use.
> > These were originally satisfied by the now removed INC_FLAGS +=
> > -I$(srctree)/tools/lib.
> >
> > Thus when NO_LIBBPF=1 even though we do not build the library - we
> > still need to install the headers to retain the consistency - hence a
> > "header only" target, that only installs the headers without building
> > the library.
> >
> > This avoids restoring the original -I$(srctree)/tools/lib/, which
> > would potentially mess up the oher library builds that have changed
> > their header include paths.
> >
> > Regards
> >
> > Mike
>
>
> Thanks Mike,
>
> The -I is needed for the libbpf headers but if NO_LIBBPF is enabled
> then the C define HAVE_LIBBPF_SUPPORT isn't and we shouldn't include
> any of these headers. This means updating the CFLAGS for libbpf should
> only be done if we actually build the static libbpf.a, the dynamic
> version's headers should already be on the include path. I sent out a
> variant of this fix doing that here:
> https://lore.kernel.org/lkml/20230106061631.571659-1-irogers@google.com/
>
> Apologies again for the breakage, I can buy you a beer the next time
> I'm home in Manchester.
> Ian
>
Applying your new patch to perf/core and building I get:-
CC builtin-stat.o
In file included from builtin-stat.c:71:
util/bpf_counter.h:7:10: fatal error: bpf/bpf.h: No such file or directory
7 | #include <bpf/bpf.h>
| ^~~~~~~~~~~
compilation terminated.
/datadisk/mike/work/kernel-ups/tools/build/Makefile.build:96: recipe
for target 'builtin-stat.o' failed
make[3]: *** [builtin-stat.o] Error 1
make[3]: *** Waiting for unfinished jobs....
LD pmu-events/pmu-events-in.o
Makefile.perf:673: recipe for target 'perf-in.o' failed
make[2]: *** [perf-in.o] Error 2
Makefile.perf:235: recipe for target 'sub-make' failed
make[1]: *** [sub-make] Error 2
Makefile:69: recipe for target 'all' failed
make: *** [all] Error 2
which is a result of the bpf headers not being installed in their new
location and the removal of the -I from the old location as mentioned
in my last.
So perhaps the issue is less about the build operations and more about
the lack of #ifdef HAVE_LIBBPF_SUPPORT in certain other source files.
However, if I put the #ifdef HAVE_LIBBPF_SUPPORT around the #include
of util/bpf_counter.h, then compilation fails with multiple
builtin-stat.c: In function ‘read_bpf_map_counters’:
builtin-stat.c:463:9: error: implicit declaration of function
‘bpf_counter__read’; did you mean ‘refcount_read’?
[-Werror=implicit-function-declaration]
463 | err = bpf_counter__read(counter);
| ^~~~~~~~~~~~~~~~~
| refcount_read
type errors.
Turns out that bpf_counter.h has inline stubs for these functions
bracketed by #ifdef HAVE_BPF_SKEL / #else / #endif, which I presume
are used in the non-bpf case.
I can get a clean build with your patch if I adjust the HAVE_BFP_SKEL
bracketing to encompass everything (including header includes, struct
defines and other functions) other than the stubs in the #ifdef case
and only the stubs in the #else case - but I have no idea if this
will have an adverse effect on other tools which may use the same
header.
Thanks and Regards
Mike
> >
> > > Thanks!
> > > Ian
> > >
> > > > $(LIBBPF)-clean:
> > > > $(call QUIET_CLEAN, libbpf)
> > > > --
> > > > 2.17.1
> > > >
> >
> >
> >
> > --
> > Mike Leach
> > Principal Engineer, ARM Ltd.
> > Manchester Design Centre. UK
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf build: Fix build error when NO_LIBBPF=1
2023-01-06 9:58 ` Mike Leach
@ 2023-01-06 14:36 ` Ian Rogers
0 siblings, 0 replies; 6+ messages in thread
From: Ian Rogers @ 2023-01-06 14:36 UTC (permalink / raw)
To: Mike Leach
Cc: linux-perf-users, linux-kernel, bpf, acme, peterz, mingo,
mark.rutland, alexander.shishkin, jolsa, namhyung
On Fri, Jan 6, 2023 at 1:58 AM Mike Leach <mike.leach@linaro.org> wrote:
>
> Hi Ian,
>
> On Fri, 6 Jan 2023 at 06:24, Ian Rogers <irogers@google.com> wrote:
> >
> > On Thu, Jan 5, 2023 at 3:40 PM Mike Leach <mike.leach@linaro.org> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, 5 Jan 2023 at 19:03, Ian Rogers <irogers@google.com> wrote:
> > > >
> > > > On Thu, Jan 5, 2023 at 9:22 AM Mike Leach <mike.leach@linaro.org> wrote:
> > > > >
> > > > > Recent updates to perf build result in the following output when cross
> > > > > compiling to aarch64, with libelf unavailable, and therefore
> > > > > NO_LIBBPF=1 set.
> > > > >
> > > > > ```
> > > > > $make -C tools/perf
> > > > >
> > > > > <cut>
> > > > >
> > > > > Makefile.config:428: No libelf found. Disables 'probe' tool, jvmti
> > > > > and BPF support in 'perf record'. Please install libelf-dev,
> > > > > libelf-devel or elfutils-libelf-devel
> > > > >
> > > > > <cut>
> > > > >
> > > > > libbpf.c:46:10: fatal error: libelf.h: No such file or directory
> > > > > 46 | #include <libelf.h>
> > > > > | ^~~~~~~~~~
> > > > > compilation terminated.
> > > > >
> > > > > ./tools/build/Makefile.build:96: recipe for target
> > > > > '.tools/perf/libbpf/staticobjs/libbpf.o' failed
> > > > >
> > > > > ```
> > > > >
> > > > > plus one other include error for <gelf.h>
> > > >
> > > > Ouch, apologies for the breakage. You wouldn't happen to have
> > > > something like a way with say a docker image to repro the problem? The
> > > > make line above is somewhat minimal.
> > > >
> > >
> > > Unfortunately not - I was cross compiling on my main workstation.
> > > However, in theory
> > > $make -C tools/perf NO_LIBBPF=1
> > > should explicitly exclude the library from the build - which without
> > > the fix it does not.
> > >
> > > > > The issue is that the commit noted below adds libbpf to the prepare:
> > > > > target but no longer accounts for the NO_LIBBPF define. Additionally
> > > > > changing the include directories means that even if the libbpf target
> > > > > build is prevented, bpf headers are missing in other parts of the build.
> > > > >
> > > > > This patch ensures that in the case of NO_LIBBPF=1, the build target is
> > > > > changed to a header only target, and the headers are installed, without
> > > > > attempting to build the libbpf.a target.
> > > > >
> > > > > Applies to perf/core
> > > > >
> > > > > Fixes: 746bd29e348f ("perf build: Use tools/lib headers from install path")
> > > > > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > > > > ---
> > > > > tools/perf/Makefile.perf | 10 ++++++++++
> > > > > 1 file changed, 10 insertions(+)
> > > > >
> > > > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > > > > index 13e7d26e77f0..ee08ecf469f6 100644
> > > > > --- a/tools/perf/Makefile.perf
> > > > > +++ b/tools/perf/Makefile.perf
> > > > > @@ -305,7 +305,11 @@ else
> > > > > endif
> > > > > LIBBPF_DESTDIR = $(LIBBPF_OUTPUT)
> > > > > LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include
> > > > > +ifndef NO_LIBBPF
> > > > > LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a
> > > > > +else
> > > > > +LIBBPF = $(LIBBPF_INCLUDE)/bpf/bpf.h
> > > >
> > > > This seems strange, don't we want to avoid libbpf targets?
> > > >
> > >
> > > This is a header only target - see my continuation comment below....
> > >
> > > > > +endif
> > > > > CFLAGS += -I$(LIBBPF_OUTPUT)/include
> > > > >
> > > > > ifneq ($(OUTPUT),)
> > > > > @@ -826,10 +830,16 @@ $(LIBAPI)-clean:
> > > > > $(call QUIET_CLEAN, libapi)
> > > > > $(Q)$(RM) -r -- $(LIBAPI_OUTPUT)
> > > > >
> > > > > +ifndef NO_LIBBPF
> > > > > $(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
> > > > > $(Q)$(MAKE) -C $(LIBBPF_DIR) FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) \
> > > > > O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \
> > > > > $@ install_headers
> > > > > +else
> > > > > +$(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
> > > > > + $(Q)$(MAKE) -C $(LIBBPF_DIR) OUTPUT=$(LIBBPF_OUTPUT)/ \
> > > > > + DESTDIR=$(LIBBPF_DESTDIR) prefix= install_headers
> > > > > +endif
> > > >
> > > > Shouldn't we just be able to conditionalize having $(LIBBPF) as a
> > > > dependency for the perf binary? If there is no dependency then the
> > > > targets won't be built and we shouldn't need to conditionalize here.
> > > >
> > >
> > > I did try doing just that, but the build process does two things when
> > > building libbpf
> > > a) builds the library
> > > b) installs the bpf headers in the libbpf output location.
> > >
> > > Now what the original patch - "perf build: Use tools/lib headers from
> > > install path" - does is to also change the include paths to the
> > > compiler to pick up the headers,
> > > removing the line:
> > >
> > > INC_FLAGS += -I$(srctree)/tools/lib/
> > >
> > > from tools/perf/Makefile.config and adding the line
> > >
> > > CFLAGS += -I$(LIBBPF_OUTPUT)/include
> > >
> > > in tools/perf/Makefile.perf (along with similar lines for libperf, libapi etc)
> > >
> > > The result of this is that if you only remove the library build, the
> > > headers are not installed and other compilation units fail as the
> > > headers are still included even if the library is not in use.
> > > These were originally satisfied by the now removed INC_FLAGS +=
> > > -I$(srctree)/tools/lib.
> > >
> > > Thus when NO_LIBBPF=1 even though we do not build the library - we
> > > still need to install the headers to retain the consistency - hence a
> > > "header only" target, that only installs the headers without building
> > > the library.
> > >
> > > This avoids restoring the original -I$(srctree)/tools/lib/, which
> > > would potentially mess up the oher library builds that have changed
> > > their header include paths.
> > >
> > > Regards
> > >
> > > Mike
> >
> >
> > Thanks Mike,
> >
> > The -I is needed for the libbpf headers but if NO_LIBBPF is enabled
> > then the C define HAVE_LIBBPF_SUPPORT isn't and we shouldn't include
> > any of these headers. This means updating the CFLAGS for libbpf should
> > only be done if we actually build the static libbpf.a, the dynamic
> > version's headers should already be on the include path. I sent out a
> > variant of this fix doing that here:
> > https://lore.kernel.org/lkml/20230106061631.571659-1-irogers@google.com/
> >
> > Apologies again for the breakage, I can buy you a beer the next time
> > I'm home in Manchester.
> > Ian
> >
>
> Applying your new patch to perf/core and building I get:-
>
> CC builtin-stat.o
> In file included from builtin-stat.c:71:
> util/bpf_counter.h:7:10: fatal error: bpf/bpf.h: No such file or directory
> 7 | #include <bpf/bpf.h>
> | ^~~~~~~~~~~
> compilation terminated.
> /datadisk/mike/work/kernel-ups/tools/build/Makefile.build:96: recipe
> for target 'builtin-stat.o' failed
> make[3]: *** [builtin-stat.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
> LD pmu-events/pmu-events-in.o
> Makefile.perf:673: recipe for target 'perf-in.o' failed
> make[2]: *** [perf-in.o] Error 2
> Makefile.perf:235: recipe for target 'sub-make' failed
> make[1]: *** [sub-make] Error 2
> Makefile:69: recipe for target 'all' failed
> make: *** [all] Error 2
>
> which is a result of the bpf headers not being installed in their new
> location and the removal of the -I from the old location as mentioned
> in my last.
> So perhaps the issue is less about the build operations and more about
> the lack of #ifdef HAVE_LIBBPF_SUPPORT in certain other source files.
>
> However, if I put the #ifdef HAVE_LIBBPF_SUPPORT around the #include
> of util/bpf_counter.h, then compilation fails with multiple
>
> builtin-stat.c: In function ‘read_bpf_map_counters’:
> builtin-stat.c:463:9: error: implicit declaration of function
> ‘bpf_counter__read’; did you mean ‘refcount_read’?
> [-Werror=implicit-function-declaration]
> 463 | err = bpf_counter__read(counter);
> | ^~~~~~~~~~~~~~~~~
> | refcount_read
>
> type errors.
>
> Turns out that bpf_counter.h has inline stubs for these functions
> bracketed by #ifdef HAVE_BPF_SKEL / #else / #endif, which I presume
> are used in the non-bpf case.
>
> I can get a clean build with your patch if I adjust the HAVE_BFP_SKEL
> bracketing to encompass everything (including header includes, struct
> defines and other functions) other than the stubs in the #ifdef case
> and only the stubs in the #else case - but I have no idea if this
> will have an adverse effect on other tools which may use the same
> header.
>
> Thanks and Regards
>
> Mike
Thanks Mike,
With "apt remove libbpf-devel" I was able to repro this and I've sent
out a v2 [1] adding the guards and moving the helper functions under
HAVE_BPF_SKEL as you say. The patch was sufficient to fix the build
for me, could you double check it? The builtin-trace fix addresses a
missing guard not present since November 2018 in '744fafc787de perf
trace: See if there is a map named "filtered_pids"' but builtin-trace
has some optionality around whether it is included in a build so I
didn't include the Fixes tag.
Thanks,
Ian
[1] https://lore.kernel.org/lkml/20230106142537.607399-2-irogers@google.com/
> > >
> > > > Thanks!
> > > > Ian
> > > >
> > > > > $(LIBBPF)-clean:
> > > > > $(call QUIET_CLEAN, libbpf)
> > > > > --
> > > > > 2.17.1
> > > > >
> > >
> > >
> > >
> > > --
> > > Mike Leach
> > > Principal Engineer, ARM Ltd.
> > > Manchester Design Centre. UK
>
>
>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-06 14:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-05 17:22 [PATCH] perf build: Fix build error when NO_LIBBPF=1 Mike Leach
2023-01-05 19:02 ` Ian Rogers
2023-01-05 23:40 ` Mike Leach
2023-01-06 6:24 ` Ian Rogers
2023-01-06 9:58 ` Mike Leach
2023-01-06 14:36 ` Ian Rogers
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).