2020-01-15 14:17:59

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: [PATCH bpf-next v2 02/10] tools/bpf/runqslower: Fix override option for VMLINUX_BTF

From: Toke Høiland-Jørgensen <[email protected]>

The runqslower tool refuses to build without a file to read vmlinux BTF
from. The build fails with an error message to override the location by
setting the VMLINUX_BTF variable if autodetection fails. However, the
Makefile doesn't actually work with that override - the error message is
still emitted.

Fix this by only doing auto-detection if no override is set. And while
we're at it, also look for a vmlinux file in the current kernel build dir
if none if found on the running kernel.

Fixes: 9c01546d26d2 ("tools/bpf: Add runqslower tool to tools/bpf")
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
---
tools/bpf/runqslower/Makefile | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
index cff2fbcd29a8..fb93ce2bf2fe 100644
--- a/tools/bpf/runqslower/Makefile
+++ b/tools/bpf/runqslower/Makefile
@@ -10,12 +10,16 @@ CFLAGS := -g -Wall

# Try to detect best kernel BTF source
KERNEL_REL := $(shell uname -r)
-ifneq ("$(wildcard /sys/kernel/btf/vmlinux)","")
-VMLINUX_BTF := /sys/kernel/btf/vmlinux
-else ifneq ("$(wildcard /boot/vmlinux-$(KERNEL_REL))","")
-VMLINUX_BTF := /boot/vmlinux-$(KERNEL_REL)
-else
-$(error "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly")
+ifeq ("$(VMLINUX_BTF)","")
+ ifneq ("$(wildcard /sys/kernel/btf/vmlinux)","")
+ VMLINUX_BTF := /sys/kernel/btf/vmlinux
+ else ifneq ("$(wildcard /boot/vmlinux-$(KERNEL_REL))","")
+ VMLINUX_BTF := /boot/vmlinux-$(KERNEL_REL)
+ else ifneq ("$(wildcard $(abspath ../../../vmlinux))","")
+ VMLINUX_BTF := $(abspath ../../../vmlinux)
+ else
+ $(error "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly")
+ endif
endif

abs_out := $(abspath $(OUTPUT))


2020-01-15 17:00:36

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH bpf-next v2 02/10] tools/bpf/runqslower: Fix override option for VMLINUX_BTF

On Wed, Jan 15, 2020 at 6:13 AM Toke Høiland-Jørgensen <[email protected]> wrote:
>
> From: Toke Høiland-Jørgensen <[email protected]>
>
> The runqslower tool refuses to build without a file to read vmlinux BTF
> from. The build fails with an error message to override the location by
> setting the VMLINUX_BTF variable if autodetection fails. However, the
> Makefile doesn't actually work with that override - the error message is
> still emitted.

Do you have example command with VMLINUX_BTF override that didn't work
(and what error message was emitted)?

>
> Fix this by only doing auto-detection if no override is set. And while
> we're at it, also look for a vmlinux file in the current kernel build dir
> if none if found on the running kernel.
>
> Fixes: 9c01546d26d2 ("tools/bpf: Add runqslower tool to tools/bpf")
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
> ---
> tools/bpf/runqslower/Makefile | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
> index cff2fbcd29a8..fb93ce2bf2fe 100644
> --- a/tools/bpf/runqslower/Makefile
> +++ b/tools/bpf/runqslower/Makefile
> @@ -10,12 +10,16 @@ CFLAGS := -g -Wall
>
> # Try to detect best kernel BTF source
> KERNEL_REL := $(shell uname -r)
> -ifneq ("$(wildcard /sys/kernel/btf/vmlinux)","")
> -VMLINUX_BTF := /sys/kernel/btf/vmlinux
> -else ifneq ("$(wildcard /boot/vmlinux-$(KERNEL_REL))","")
> -VMLINUX_BTF := /boot/vmlinux-$(KERNEL_REL)
> -else
> -$(error "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly")
> +ifeq ("$(VMLINUX_BTF)","")
> + ifneq ("$(wildcard /sys/kernel/btf/vmlinux)","")
> + VMLINUX_BTF := /sys/kernel/btf/vmlinux
> + else ifneq ("$(wildcard /boot/vmlinux-$(KERNEL_REL))","")
> + VMLINUX_BTF := /boot/vmlinux-$(KERNEL_REL)
> + else ifneq ("$(wildcard $(abspath ../../../vmlinux))","")
> + VMLINUX_BTF := $(abspath ../../../vmlinux)

I'm planning to mirror runqslower into libbpf Github repo and this
../../../vmlinux piece will be completely out of place in that
context. Also it only will help when building kernel in-tree. So I'd
rather not add this.

> + else
> + $(error "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly")
> + endif
> endif
>
> abs_out := $(abspath $(OUTPUT))
>

2020-01-15 23:21:20

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH bpf-next v2 02/10] tools/bpf/runqslower: Fix override option for VMLINUX_BTF

Andrii Nakryiko <[email protected]> writes:

> On Wed, Jan 15, 2020 at 6:13 AM Toke Høiland-Jørgensen <[email protected]> wrote:
>>
>> From: Toke Høiland-Jørgensen <[email protected]>
>>
>> The runqslower tool refuses to build without a file to read vmlinux BTF
>> from. The build fails with an error message to override the location by
>> setting the VMLINUX_BTF variable if autodetection fails. However, the
>> Makefile doesn't actually work with that override - the error message is
>> still emitted.
>
> Do you have example command with VMLINUX_BTF override that didn't work
> (and what error message was emitted)?

Before this patch:

$ cd ~/build/linux/tools/bpf/runqslower
$ make
Makefile:18: *** "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly". Stop.

$ make VMLINUX_BTF=~/build/linux/vmlinux
Makefile:18: *** "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly". Stop.

>> Fix this by only doing auto-detection if no override is set. And while
>> we're at it, also look for a vmlinux file in the current kernel build dir
>> if none if found on the running kernel.
>>
>> Fixes: 9c01546d26d2 ("tools/bpf: Add runqslower tool to tools/bpf")
>> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
>> ---
>> tools/bpf/runqslower/Makefile | 16 ++++++++++------
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
>> index cff2fbcd29a8..fb93ce2bf2fe 100644
>> --- a/tools/bpf/runqslower/Makefile
>> +++ b/tools/bpf/runqslower/Makefile
>> @@ -10,12 +10,16 @@ CFLAGS := -g -Wall
>>
>> # Try to detect best kernel BTF source
>> KERNEL_REL := $(shell uname -r)
>> -ifneq ("$(wildcard /sys/kernel/btf/vmlinux)","")
>> -VMLINUX_BTF := /sys/kernel/btf/vmlinux
>> -else ifneq ("$(wildcard /boot/vmlinux-$(KERNEL_REL))","")
>> -VMLINUX_BTF := /boot/vmlinux-$(KERNEL_REL)
>> -else
>> -$(error "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly")
>> +ifeq ("$(VMLINUX_BTF)","")
>> + ifneq ("$(wildcard /sys/kernel/btf/vmlinux)","")
>> + VMLINUX_BTF := /sys/kernel/btf/vmlinux
>> + else ifneq ("$(wildcard /boot/vmlinux-$(KERNEL_REL))","")
>> + VMLINUX_BTF := /boot/vmlinux-$(KERNEL_REL)
>> + else ifneq ("$(wildcard $(abspath ../../../vmlinux))","")
>> + VMLINUX_BTF := $(abspath ../../../vmlinux)
>
> I'm planning to mirror runqslower into libbpf Github repo and this
> ../../../vmlinux piece will be completely out of place in that
> context. Also it only will help when building kernel in-tree. So I'd
> rather not add this.

Well building the kernel in-tree is something people sometimes want to do ;)

Specifically, the selftests depend on this, so we should at least fix
those; but I guess it could work to just pass in VMLINUX_BTF as part of
the make -C from the selftests dir? I'll try that...

-Toke

2020-01-16 00:10:20

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH bpf-next v2 02/10] tools/bpf/runqslower: Fix override option for VMLINUX_BTF

On Wed, Jan 15, 2020 at 2:06 PM Toke Høiland-Jørgensen <[email protected]> wrote:
>
> Andrii Nakryiko <[email protected]> writes:
>
> > On Wed, Jan 15, 2020 at 6:13 AM Toke Høiland-Jørgensen <[email protected]> wrote:
> >>
> >> From: Toke Høiland-Jørgensen <[email protected]>
> >>
> >> The runqslower tool refuses to build without a file to read vmlinux BTF
> >> from. The build fails with an error message to override the location by
> >> setting the VMLINUX_BTF variable if autodetection fails. However, the
> >> Makefile doesn't actually work with that override - the error message is
> >> still emitted.
> >
> > Do you have example command with VMLINUX_BTF override that didn't work
> > (and what error message was emitted)?
>
> Before this patch:
>
> $ cd ~/build/linux/tools/bpf/runqslower
> $ make
> Makefile:18: *** "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly". Stop.
>
> $ make VMLINUX_BTF=~/build/linux/vmlinux
> Makefile:18: *** "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly". Stop.

Ok, so this is strange. Try make clean and run with V=1, it might help
to debug this. This could happen if ~/build/linux/vmlinux doesn't
exist, but I assume you double-checked that. It works for me just fine
(Makefile won't do VMLINUX_BTF := assignment, if it's defined through
make invocation, so your change should be a no-op in that regard):

$ make clean
$ make VMLINUX_BTF=~/linux-build/default/vmlinux V=1
...
.output/sbin/bpftool btf dump file ~/linux-build/default/vmlinux
format c > .output/vmlinux.h
...

Wonder what your output looks like?

>
> >> Fix this by only doing auto-detection if no override is set. And while
> >> we're at it, also look for a vmlinux file in the current kernel build dir
> >> if none if found on the running kernel.
> >>
> >> Fixes: 9c01546d26d2 ("tools/bpf: Add runqslower tool to tools/bpf")
> >> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
> >> ---
> >> tools/bpf/runqslower/Makefile | 16 ++++++++++------
> >> 1 file changed, 10 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
> >> index cff2fbcd29a8..fb93ce2bf2fe 100644
> >> --- a/tools/bpf/runqslower/Makefile
> >> +++ b/tools/bpf/runqslower/Makefile
> >> @@ -10,12 +10,16 @@ CFLAGS := -g -Wall
> >>
> >> # Try to detect best kernel BTF source
> >> KERNEL_REL := $(shell uname -r)
> >> -ifneq ("$(wildcard /sys/kernel/btf/vmlinux)","")
> >> -VMLINUX_BTF := /sys/kernel/btf/vmlinux
> >> -else ifneq ("$(wildcard /boot/vmlinux-$(KERNEL_REL))","")
> >> -VMLINUX_BTF := /boot/vmlinux-$(KERNEL_REL)
> >> -else
> >> -$(error "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly")
> >> +ifeq ("$(VMLINUX_BTF)","")
> >> + ifneq ("$(wildcard /sys/kernel/btf/vmlinux)","")
> >> + VMLINUX_BTF := /sys/kernel/btf/vmlinux
> >> + else ifneq ("$(wildcard /boot/vmlinux-$(KERNEL_REL))","")
> >> + VMLINUX_BTF := /boot/vmlinux-$(KERNEL_REL)
> >> + else ifneq ("$(wildcard $(abspath ../../../vmlinux))","")
> >> + VMLINUX_BTF := $(abspath ../../../vmlinux)
> >
> > I'm planning to mirror runqslower into libbpf Github repo and this
> > ../../../vmlinux piece will be completely out of place in that
> > context. Also it only will help when building kernel in-tree. So I'd
> > rather not add this.
>
> Well building the kernel in-tree is something people sometimes want to do ;)
>
> Specifically, the selftests depend on this, so we should at least fix
> those; but I guess it could work to just pass in VMLINUX_BTF as part of
> the make -C from the selftests dir? I'll try that...

Yes, it can be handled through VMLINUX_BTF override for selftests. As
I said, this will be a self-contained example in libbpf's Github repo,
so this "in kernel tree" assumption doesn't stand there.


>
> -Toke
>

2020-01-16 09:15:36

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH bpf-next v2 02/10] tools/bpf/runqslower: Fix override option for VMLINUX_BTF

Andrii Nakryiko <[email protected]> writes:

> On Wed, Jan 15, 2020 at 2:06 PM Toke Høiland-Jørgensen <[email protected]> wrote:
>>
>> Andrii Nakryiko <[email protected]> writes:
>>
>> > On Wed, Jan 15, 2020 at 6:13 AM Toke Høiland-Jørgensen <[email protected]> wrote:
>> >>
>> >> From: Toke Høiland-Jørgensen <[email protected]>
>> >>
>> >> The runqslower tool refuses to build without a file to read vmlinux BTF
>> >> from. The build fails with an error message to override the location by
>> >> setting the VMLINUX_BTF variable if autodetection fails. However, the
>> >> Makefile doesn't actually work with that override - the error message is
>> >> still emitted.
>> >
>> > Do you have example command with VMLINUX_BTF override that didn't work
>> > (and what error message was emitted)?
>>
>> Before this patch:
>>
>> $ cd ~/build/linux/tools/bpf/runqslower
>> $ make
>> Makefile:18: *** "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly". Stop.
>>
>> $ make VMLINUX_BTF=~/build/linux/vmlinux
>> Makefile:18: *** "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly". Stop.
>
> Ok, so this is strange. Try make clean and run with V=1, it might help
> to debug this. This could happen if ~/build/linux/vmlinux doesn't
> exist, but I assume you double-checked that. It works for me just fine
> (Makefile won't do VMLINUX_BTF := assignment, if it's defined through
> make invocation, so your change should be a no-op in that regard):
>
> $ make clean
> $ make VMLINUX_BTF=~/linux-build/default/vmlinux V=1
> ...
> .output/sbin/bpftool btf dump file ~/linux-build/default/vmlinux
> format c > .output/vmlinux.h
> ...
>
> Wonder what your output looks like?

$ make clean
Makefile:18: *** "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly". Stop.
$ make VMLINUX_BTF=~/build/linux/vmlinux V=1
Makefile:18: *** "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly". Stop.

Take another look at the relevant part of the makefile:

ifneq ("$(wildcard /sys/kernel/btf/vmlinux)","")
VMLINUX_BTF := /sys/kernel/btf/vmlinux
else ifneq ("$(wildcard /boot/vmlinux-$(KERNEL_REL))","")
VMLINUX_BTF := /boot/vmlinux-$(KERNEL_REL)
else
$(error "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly")
endif

That if/else doesn't actually consider the value of VMLINUX_BTF; so the
override only works if one of the files being considered by the
auto-detection actually exists... :)

-Toke

2020-01-16 21:04:09

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH bpf-next v2 02/10] tools/bpf/runqslower: Fix override option for VMLINUX_BTF

On Thu, Jan 16, 2020 at 1:05 AM Toke Høiland-Jørgensen <[email protected]> wrote:
>
> Andrii Nakryiko <[email protected]> writes:
>
> > On Wed, Jan 15, 2020 at 2:06 PM Toke Høiland-Jørgensen <[email protected]> wrote:
> >>
> >> Andrii Nakryiko <[email protected]> writes:
> >>
> >> > On Wed, Jan 15, 2020 at 6:13 AM Toke Høiland-Jørgensen <[email protected]> wrote:
> >> >>
> >> >> From: Toke Høiland-Jørgensen <[email protected]>
> >> >>
> >> >> The runqslower tool refuses to build without a file to read vmlinux BTF
> >> >> from. The build fails with an error message to override the location by
> >> >> setting the VMLINUX_BTF variable if autodetection fails. However, the
> >> >> Makefile doesn't actually work with that override - the error message is
> >> >> still emitted.
> >> >
> >> > Do you have example command with VMLINUX_BTF override that didn't work
> >> > (and what error message was emitted)?
> >>
> >> Before this patch:
> >>
> >> $ cd ~/build/linux/tools/bpf/runqslower
> >> $ make
> >> Makefile:18: *** "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly". Stop.
> >>
> >> $ make VMLINUX_BTF=~/build/linux/vmlinux
> >> Makefile:18: *** "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly". Stop.
> >
> > Ok, so this is strange. Try make clean and run with V=1, it might help
> > to debug this. This could happen if ~/build/linux/vmlinux doesn't
> > exist, but I assume you double-checked that. It works for me just fine
> > (Makefile won't do VMLINUX_BTF := assignment, if it's defined through
> > make invocation, so your change should be a no-op in that regard):
> >
> > $ make clean
> > $ make VMLINUX_BTF=~/linux-build/default/vmlinux V=1
> > ...
> > .output/sbin/bpftool btf dump file ~/linux-build/default/vmlinux
> > format c > .output/vmlinux.h
> > ...
> >
> > Wonder what your output looks like?
>
> $ make clean
> Makefile:18: *** "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly". Stop.
> $ make VMLINUX_BTF=~/build/linux/vmlinux V=1
> Makefile:18: *** "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly". Stop.
>
> Take another look at the relevant part of the makefile:
>
> ifneq ("$(wildcard /sys/kernel/btf/vmlinux)","")
> VMLINUX_BTF := /sys/kernel/btf/vmlinux
> else ifneq ("$(wildcard /boot/vmlinux-$(KERNEL_REL))","")
> VMLINUX_BTF := /boot/vmlinux-$(KERNEL_REL)
> else
> $(error "Can't detect kernel BTF, use VMLINUX_BTF to specify it explicitly")
> endif
>
> That if/else doesn't actually consider the value of VMLINUX_BTF; so the
> override only works if one of the files being considered by the
> auto-detection actually exists... :)

Ah, right, unconditional $(error), completely missed that few times, thanks!

>
> -Toke
>