| Message ID | 20200121150431.GA240246@chrisdown.name |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: btf: Always output invariant hit in pahole DWARF to BTF transform | expand |
On Tue, Jan 21, 2020 at 7:05 AM Chris Down <chris@chrisdown.name> 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 <chris@chrisdown.name> > Cc: Stanislav Fomichev <sdf@google.com> > Cc: Andrii Nakryiko <andriin@fb.com> > Cc: John Fastabend <john.fastabend@gmail.com> > Cc: linux-kernel@vger.kernel.org > Cc: netdev@vger.kernel.org > Cc: bpf@vger.kernel.org > Cc: kernel-team@fb.com > --- > 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 >
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.
On Tue, Jan 21, 2020 at 12:29 PM Chris Down <chris@chrisdown.name> 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!
Andrii Nakryiko writes: >On Tue, Jan 21, 2020 at 12:29 PM Chris Down <chris@chrisdown.name> 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! :-)
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
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 <chris@chrisdown.name> Cc: Stanislav Fomichev <sdf@google.com> Cc: Andrii Nakryiko <andriin@fb.com> Cc: John Fastabend <john.fastabend@gmail.com> Cc: linux-kernel@vger.kernel.org Cc: netdev@vger.kernel.org Cc: bpf@vger.kernel.org Cc: kernel-team@fb.com --- scripts/link-vmlinux.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)