2020-01-21 15:06:42

by Chris Down

[permalink] [raw]
Subject: [PATCH] bpf: btf: Always output invariant hit in pahole DWARF to BTF transform

When trying to compile with CONFIG_DEBUG_INFO_BTF enabled, I got this
error:

% make -s
Failed to generate BTF for vmlinux
Try to disable CONFIG_DEBUG_INFO_BTF
make[3]: *** [vmlinux] Error 1

Compiling again without -s shows the true error (that pahole is
missing), but since this is fatal, we should show the error
unconditionally on stderr as well, not silence it using the `info`
function. With this patch:

% make -s
BTF: .tmp_vmlinux.btf: pahole (pahole) is not available
Failed to generate BTF for vmlinux
Try to disable CONFIG_DEBUG_INFO_BTF
make[3]: *** [vmlinux] Error 1

Signed-off-by: Chris Down <[email protected]>
Cc: Stanislav Fomichev <[email protected]>
Cc: Andrii Nakryiko <[email protected]>
Cc: John Fastabend <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
scripts/link-vmlinux.sh | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index c287ad9b3a67..c8e9f49903a0 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -108,13 +108,15 @@ gen_btf()
local bin_arch

if ! [ -x "$(command -v ${PAHOLE})" ]; then
- info "BTF" "${1}: pahole (${PAHOLE}) is not available"
+ printf 'BTF: %s: pahole (%s) is not available\n' \
+ "${1}" "${PAHOLE}" >&2
return 1
fi

pahole_ver=$(${PAHOLE} --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/')
if [ "${pahole_ver}" -lt "113" ]; then
- info "BTF" "${1}: pahole version $(${PAHOLE} --version) is too old, need at least v1.13"
+ printf 'BTF: %s: pahole version %s is too old, need at least v1.13\n' \
+ "${1}" "$(${PAHOLE} --version)" >&2
return 1
fi

--
2.25.0


2020-01-21 19:07:57

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH] bpf: btf: Always output invariant hit in pahole DWARF to BTF transform

On Tue, Jan 21, 2020 at 7:05 AM Chris Down <[email protected]> wrote:
>
> When trying to compile with CONFIG_DEBUG_INFO_BTF enabled, I got this
> error:
>
> % make -s
> Failed to generate BTF for vmlinux
> Try to disable CONFIG_DEBUG_INFO_BTF
> make[3]: *** [vmlinux] Error 1
>
> Compiling again without -s shows the true error (that pahole is
> missing), but since this is fatal, we should show the error
> unconditionally on stderr as well, not silence it using the `info`
> function. With this patch:
>
> % make -s
> BTF: .tmp_vmlinux.btf: pahole (pahole) is not available
> Failed to generate BTF for vmlinux
> Try to disable CONFIG_DEBUG_INFO_BTF
> make[3]: *** [vmlinux] Error 1
>
> Signed-off-by: Chris Down <[email protected]>
> Cc: Stanislav Fomichev <[email protected]>
> Cc: Andrii Nakryiko <[email protected]>
> Cc: John Fastabend <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> scripts/link-vmlinux.sh | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index c287ad9b3a67..c8e9f49903a0 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -108,13 +108,15 @@ gen_btf()
> local bin_arch
>
> if ! [ -x "$(command -v ${PAHOLE})" ]; then
> - info "BTF" "${1}: pahole (${PAHOLE}) is not available"
> + printf 'BTF: %s: pahole (%s) is not available\n' \
> + "${1}" "${PAHOLE}" >&2

any reason not to use echo instead of printf? would be more minimal change

> return 1
> fi
>
> pahole_ver=$(${PAHOLE} --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/')
> if [ "${pahole_ver}" -lt "113" ]; then
> - info "BTF" "${1}: pahole version $(${PAHOLE} --version) is too old, need at least v1.13"
> + printf 'BTF: %s: pahole version %s is too old, need at least v1.13\n' \
> + "${1}" "$(${PAHOLE} --version)" >&2
> return 1
> fi
>
> --
> 2.25.0
>

2020-01-21 20:30:28

by Chris Down

[permalink] [raw]
Subject: Re: [PATCH] bpf: btf: Always output invariant hit in pahole DWARF to BTF transform

Andrii Nakryiko writes:
>> --- a/scripts/link-vmlinux.sh
>> +++ b/scripts/link-vmlinux.sh
>> @@ -108,13 +108,15 @@ gen_btf()
>> local bin_arch
>>
>> if ! [ -x "$(command -v ${PAHOLE})" ]; then
>> - info "BTF" "${1}: pahole (${PAHOLE}) is not available"
>> + printf 'BTF: %s: pahole (%s) is not available\n' \
>> + "${1}" "${PAHOLE}" >&2
>
>any reason not to use echo instead of printf? would be more minimal change

I generally avoid using echo because it has a bunch of portability gotchas
which printf mostly doesn't have. If you'd prefer echo, that's fine though,
just let me know and I can send v2.

2020-01-21 21:47:57

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH] bpf: btf: Always output invariant hit in pahole DWARF to BTF transform

On Tue, Jan 21, 2020 at 12:29 PM Chris Down <[email protected]> wrote:
>
> Andrii Nakryiko writes:
> >> --- a/scripts/link-vmlinux.sh
> >> +++ b/scripts/link-vmlinux.sh
> >> @@ -108,13 +108,15 @@ gen_btf()
> >> local bin_arch
> >>
> >> if ! [ -x "$(command -v ${PAHOLE})" ]; then
> >> - info "BTF" "${1}: pahole (${PAHOLE}) is not available"
> >> + printf 'BTF: %s: pahole (%s) is not available\n' \
> >> + "${1}" "${PAHOLE}" >&2
> >
> >any reason not to use echo instead of printf? would be more minimal change
>
> I generally avoid using echo because it has a bunch of portability gotchas
> which printf mostly doesn't have. If you'd prefer echo, that's fine though,
> just let me know and I can send v2.

The rest of the script is using echo for errors, so let's stick to it
for consistency. Thanks!

2020-01-21 23:52:14

by Chris Down

[permalink] [raw]
Subject: Re: [PATCH] bpf: btf: Always output invariant hit in pahole DWARF to BTF transform

Andrii Nakryiko writes:
>On Tue, Jan 21, 2020 at 12:29 PM Chris Down <[email protected]> wrote:
>>
>> Andrii Nakryiko writes:
>> >> --- a/scripts/link-vmlinux.sh
>> >> +++ b/scripts/link-vmlinux.sh
>> >> @@ -108,13 +108,15 @@ gen_btf()
>> >> local bin_arch
>> >>
>> >> if ! [ -x "$(command -v ${PAHOLE})" ]; then
>> >> - info "BTF" "${1}: pahole (${PAHOLE}) is not available"
>> >> + printf 'BTF: %s: pahole (%s) is not available\n' \
>> >> + "${1}" "${PAHOLE}" >&2
>> >
>> >any reason not to use echo instead of printf? would be more minimal change
>>
>> I generally avoid using echo because it has a bunch of portability gotchas
>> which printf mostly doesn't have. If you'd prefer echo, that's fine though,
>> just let me know and I can send v2.
>
>The rest of the script is using echo for errors, so let's stick to it
>for consistency. Thanks!

Sure thing, I'll send v2. Thanks! :-)