tools/bpf/Makefile | 7 ++- tools/bpf/bpf_jit_disasm.c | 5 +- tools/bpf/bpftool/Makefile | 7 ++- tools/bpf/bpftool/jit_disasm.c | 42 ++++++++++++--- tools/build/Makefile.feature | 4 +- tools/build/feature/Makefile | 4 ++ tools/build/feature/test-all.c | 4 ++ .../feature/test-disassembler-init-styled.c | 13 +++++ tools/include/tools/dis-asm-compat.h | 53 +++++++++++++++++++ tools/perf/Makefile.config | 8 +++ tools/perf/util/annotate.c | 7 +-- 11 files changed, 137 insertions(+), 17 deletions(-) create mode 100644 tools/build/feature/test-disassembler-init-styled.c create mode 100644 tools/include/tools/dis-asm-compat.h
binutils changed the signature of init_disassemble_info(), which now causes
compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
I first fixed this without introducing the compat header, as suggested by
Quentin, but I thought the amount of repeated boilerplate was a bit too
much. So instead I introduced a compat header to wrap the API changes. Even
tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
looks nicer this way.
I'm not regular contributor, so it very well might be my procedures are a
bit off...
I am not sure I added the right [number of] people to CC?
WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
in feature/Makefile and why -ldl isn't needed in the other places. But...
V2:
- split patches further, so that tools/bpf and tools/perf part are entirely
separate
- included a bit more information about tests I did in commit messages
- add a maybe_unused to fprintf_json_styled's style argument
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Sedat Dilek <sedat.dilek@gmail.com>
Cc: Quentin Monnet <quentin@isovalent.com>
To: bpf@vger.kernel.org
To: linux-kernel@vger.kernel.org
Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
Andres Freund (5):
tools build: add feature test for init_disassemble_info API changes
tools include: add dis-asm-compat.h to handle version differences
tools perf: Fix compilation error with new binutils
tools bpf_jit_disasm: Fix compilation error with new binutils
tools bpftool: Fix compilation error with new binutils
tools/bpf/Makefile | 7 ++-
tools/bpf/bpf_jit_disasm.c | 5 +-
tools/bpf/bpftool/Makefile | 7 ++-
tools/bpf/bpftool/jit_disasm.c | 42 ++++++++++++---
tools/build/Makefile.feature | 4 +-
tools/build/feature/Makefile | 4 ++
tools/build/feature/test-all.c | 4 ++
.../feature/test-disassembler-init-styled.c | 13 +++++
tools/include/tools/dis-asm-compat.h | 53 +++++++++++++++++++
tools/perf/Makefile.config | 8 +++
tools/perf/util/annotate.c | 7 +--
11 files changed, 137 insertions(+), 17 deletions(-)
create mode 100644 tools/build/feature/test-disassembler-init-styled.c
create mode 100644 tools/include/tools/dis-asm-compat.h
--
2.37.0.3.g30cc8d0f14
On Sun, Jul 3, 2022 at 11:25 PM Andres Freund <andres@anarazel.de> wrote:
>
> binutils changed the signature of init_disassemble_info(), which now causes
> compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> binutils commit:
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
>
> I first fixed this without introducing the compat header, as suggested by
> Quentin, but I thought the amount of repeated boilerplate was a bit too
> much. So instead I introduced a compat header to wrap the API changes. Even
> tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> looks nicer this way.
>
> I'm not regular contributor, so it very well might be my procedures are a
> bit off...
>
> I am not sure I added the right [number of] people to CC?
>
> WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> in feature/Makefile and why -ldl isn't needed in the other places. But...
>
> V2:
> - split patches further, so that tools/bpf and tools/perf part are entirely
> separate
> - included a bit more information about tests I did in commit messages
> - add a maybe_unused to fprintf_json_styled's style argument
>
[ CC Ben ]
The Debian kernel-team has integrated your patchset v2.
In case you build without libbfd support there is [1].
So, feel free to take this for v3.
-Sedat-
[1] https://salsa.debian.org/kernel-team/linux/-/blob/sid/debian/patches/bugfix/all/tools-perf-fix-build-without-libbfd.patch
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Quentin Monnet <quentin@isovalent.com>
> To: bpf@vger.kernel.org
> To: linux-kernel@vger.kernel.org
> Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
>
> Andres Freund (5):
> tools build: add feature test for init_disassemble_info API changes
> tools include: add dis-asm-compat.h to handle version differences
> tools perf: Fix compilation error with new binutils
> tools bpf_jit_disasm: Fix compilation error with new binutils
> tools bpftool: Fix compilation error with new binutils
>
> tools/bpf/Makefile | 7 ++-
> tools/bpf/bpf_jit_disasm.c | 5 +-
> tools/bpf/bpftool/Makefile | 7 ++-
> tools/bpf/bpftool/jit_disasm.c | 42 ++++++++++++---
> tools/build/Makefile.feature | 4 +-
> tools/build/feature/Makefile | 4 ++
> tools/build/feature/test-all.c | 4 ++
> .../feature/test-disassembler-init-styled.c | 13 +++++
> tools/include/tools/dis-asm-compat.h | 53 +++++++++++++++++++
> tools/perf/Makefile.config | 8 +++
> tools/perf/util/annotate.c | 7 +--
> 11 files changed, 137 insertions(+), 17 deletions(-)
> create mode 100644 tools/build/feature/test-disassembler-init-styled.c
> create mode 100644 tools/include/tools/dis-asm-compat.h
>
> --
> 2.37.0.3.g30cc8d0f14
>
On Thu, 2022-07-14 at 11:16 +0200, Sedat Dilek wrote:
> On Sun, Jul 3, 2022 at 11:25 PM Andres Freund <andres@anarazel.de> wrote:
> >
> > binutils changed the signature of init_disassemble_info(), which now causes
> > compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> > binutils commit:
> > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
> >
> > I first fixed this without introducing the compat header, as suggested by
> > Quentin, but I thought the amount of repeated boilerplate was a bit too
> > much. So instead I introduced a compat header to wrap the API changes. Even
> > tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> > looks nicer this way.
> >
> > I'm not regular contributor, so it very well might be my procedures are a
> > bit off...
> >
> > I am not sure I added the right [number of] people to CC?
> >
> > WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> > nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> > in feature/Makefile and why -ldl isn't needed in the other places. But...
> >
> > V2:
> > - split patches further, so that tools/bpf and tools/perf part are entirely
> > separate
> > - included a bit more information about tests I did in commit messages
> > - add a maybe_unused to fprintf_json_styled's style argument
> >
>
> [ CC Ben ]
>
> The Debian kernel-team has integrated your patchset v2.
>
> In case you build without libbfd support there is [1].
> So, feel free to take this for v3.
>
> -Sedat-
>
> [1] https://salsa.debian.org/kernel-team/linux/-/blob/sid/debian/patches/bugfix/all/tools-perf-fix-build-without-libbfd.patch
[...]
Thanks, I meant to send that fix upstream but got distracted. It
should really be folded into "tools perf: Fix compilation error with
new binutils".
Ben.
>
--
Ben Hutchings
Always try to do things in chronological order;
it's less confusing that way.
Hi, On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote: > Thanks, I meant to send that fix upstream but got distracted. It > should really be folded into "tools perf: Fix compilation error with > new binutils". I'll try to send a new version out soon. I think the right process is to add Signed-off-by: Ben Hutchings <benh@debian.org> to the patch I squash it with? Greetings, Andres Freund
Em Fri, Jul 15, 2022 at 12:16:41PM -0700, Andres Freund escreveu: > Hi, > > On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote: > > Thanks, I meant to send that fix upstream but got distracted. It > > should really be folded into "tools perf: Fix compilation error with > > new binutils". > > I'll try to send a new version out soon. I think the right process is to add > Signed-off-by: Ben Hutchings <benh@debian.org> > to the patch I squash it with? Hi, How is this going? Any new patch coming soon? :-) - Arnaldo
Hi, On 2022-07-27 12:47:16 -0300, Arnaldo Carvalho de Melo wrote: > Em Fri, Jul 15, 2022 at 12:16:41PM -0700, Andres Freund escreveu: > > On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote: > > > Thanks, I meant to send that fix upstream but got distracted. It > > > should really be folded into "tools perf: Fix compilation error with > > > new binutils". > > > > I'll try to send a new version out soon. I think the right process is to add > > Signed-off-by: Ben Hutchings <benh@debian.org> > > to the patch I squash it with? > > Hi, > > How is this going? Any new patch coming soon? :-) Sorry - had hoped to finish sending it out before my vacation (and then on the flight, but wifi didn't work...). Now back, will work on it asap. Greetings, Andres Freund
On Fri, 2022-07-15 at 12:16 -0700, Andres Freund wrote: > Hi, > > On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote: > > Thanks, I meant to send that fix upstream but got distracted. It > > should really be folded into "tools perf: Fix compilation error with > > new binutils". > > I'll try to send a new version out soon. I think the right process is to add > Signed-off-by: Ben Hutchings <benh@debian.org> > to the patch I squash it with? Yes, OK. Ben. -- Ben Hutchings Unix is many things to many people, but it's never been everything to anybody.
Em Fri, Jul 15, 2022 at 09:18:26PM +0200, Ben Hutchings escreveu: > On Fri, 2022-07-15 at 12:16 -0700, Andres Freund wrote: > > Hi, > > > > On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote: > > > Thanks, I meant to send that fix upstream but got distracted. It > > > should really be folded into "tools perf: Fix compilation error with > > > new binutils". > > > > I'll try to send a new version out soon. I think the right process is to add > > Signed-off-by: Ben Hutchings <benh@debian.org> > > to the patch I squash it with? > > Yes, OK. Ok, so you agreed on this, I'm adding Ben's Signed-off-by tag then, Thanks, - Arnaldo
On Sun, Jul 3, 2022 at 11:25 PM Andres Freund <andres@anarazel.de> wrote:
>
> binutils changed the signature of init_disassemble_info(), which now causes
> compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> binutils commit:
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
>
HI,
what was the base for this patchset?
I tried with Linux v5.19-rc5 and it doesn not apply cleanly.
Regards,
-Sedat-
> I first fixed this without introducing the compat header, as suggested by
> Quentin, but I thought the amount of repeated boilerplate was a bit too
> much. So instead I introduced a compat header to wrap the API changes. Even
> tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> looks nicer this way.
>
> I'm not regular contributor, so it very well might be my procedures are a
> bit off...
>
> I am not sure I added the right [number of] people to CC?
>
> WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> in feature/Makefile and why -ldl isn't needed in the other places. But...
>
> V2:
> - split patches further, so that tools/bpf and tools/perf part are entirely
> separate
> - included a bit more information about tests I did in commit messages
> - add a maybe_unused to fprintf_json_styled's style argument
>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Quentin Monnet <quentin@isovalent.com>
> To: bpf@vger.kernel.org
> To: linux-kernel@vger.kernel.org
> Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
>
> Andres Freund (5):
> tools build: add feature test for init_disassemble_info API changes
> tools include: add dis-asm-compat.h to handle version differences
> tools perf: Fix compilation error with new binutils
> tools bpf_jit_disasm: Fix compilation error with new binutils
> tools bpftool: Fix compilation error with new binutils
>
> tools/bpf/Makefile | 7 ++-
> tools/bpf/bpf_jit_disasm.c | 5 +-
> tools/bpf/bpftool/Makefile | 7 ++-
> tools/bpf/bpftool/jit_disasm.c | 42 ++++++++++++---
> tools/build/Makefile.feature | 4 +-
> tools/build/feature/Makefile | 4 ++
> tools/build/feature/test-all.c | 4 ++
> .../feature/test-disassembler-init-styled.c | 13 +++++
> tools/include/tools/dis-asm-compat.h | 53 +++++++++++++++++++
> tools/perf/Makefile.config | 8 +++
> tools/perf/util/annotate.c | 7 +--
> 11 files changed, 137 insertions(+), 17 deletions(-)
> create mode 100644 tools/build/feature/test-disassembler-init-styled.c
> create mode 100644 tools/include/tools/dis-asm-compat.h
>
> --
> 2.37.0.3.g30cc8d0f14
>
On Sun, Jul 10, 2022 at 1:43 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Sun, Jul 3, 2022 at 11:25 PM Andres Freund <andres@anarazel.de> wrote:
> >
> > binutils changed the signature of init_disassemble_info(), which now causes
> > compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> > binutils commit:
> > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
> >
>
> HI,
>
> what was the base for this patchset?
> I tried with Linux v5.19-rc5 and it doesn not apply cleanly.
>
More coffee.
$ egrep 'Auto-detecting|disassembler' make-log_perf-python3.10-install_bin.txt
15:Auto-detecting system features:
36:... disassembler-four-args: [ on ]
37:... disassembler-init-styled: [ on ]
Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM-14 (x86-64)
-Sedat-
>
> > I first fixed this without introducing the compat header, as suggested by
> > Quentin, but I thought the amount of repeated boilerplate was a bit too
> > much. So instead I introduced a compat header to wrap the API changes. Even
> > tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> > looks nicer this way.
> >
> > I'm not regular contributor, so it very well might be my procedures are a
> > bit off...
> >
> > I am not sure I added the right [number of] people to CC?
> >
> > WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> > nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> > in feature/Makefile and why -ldl isn't needed in the other places. But...
> >
> > V2:
> > - split patches further, so that tools/bpf and tools/perf part are entirely
> > separate
> > - included a bit more information about tests I did in commit messages
> > - add a maybe_unused to fprintf_json_styled's style argument
> >
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Sedat Dilek <sedat.dilek@gmail.com>
> > Cc: Quentin Monnet <quentin@isovalent.com>
> > To: bpf@vger.kernel.org
> > To: linux-kernel@vger.kernel.org
> > Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> > Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
> >
> > Andres Freund (5):
> > tools build: add feature test for init_disassemble_info API changes
> > tools include: add dis-asm-compat.h to handle version differences
> > tools perf: Fix compilation error with new binutils
> > tools bpf_jit_disasm: Fix compilation error with new binutils
> > tools bpftool: Fix compilation error with new binutils
> >
> > tools/bpf/Makefile | 7 ++-
> > tools/bpf/bpf_jit_disasm.c | 5 +-
> > tools/bpf/bpftool/Makefile | 7 ++-
> > tools/bpf/bpftool/jit_disasm.c | 42 ++++++++++++---
> > tools/build/Makefile.feature | 4 +-
> > tools/build/feature/Makefile | 4 ++
> > tools/build/feature/test-all.c | 4 ++
> > .../feature/test-disassembler-init-styled.c | 13 +++++
> > tools/include/tools/dis-asm-compat.h | 53 +++++++++++++++++++
> > tools/perf/Makefile.config | 8 +++
> > tools/perf/util/annotate.c | 7 +--
> > 11 files changed, 137 insertions(+), 17 deletions(-)
> > create mode 100644 tools/build/feature/test-disassembler-init-styled.c
> > create mode 100644 tools/include/tools/dis-asm-compat.h
> >
> > --
> > 2.37.0.3.g30cc8d0f14
> >
On Sun, Jul 03, 2022 at 02:25:46PM -0700, Andres Freund wrote:
> binutils changed the signature of init_disassemble_info(), which now causes
> compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> binutils commit:
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
>
> I first fixed this without introducing the compat header, as suggested by
> Quentin, but I thought the amount of repeated boilerplate was a bit too
> much. So instead I introduced a compat header to wrap the API changes. Even
> tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> looks nicer this way.
>
> I'm not regular contributor, so it very well might be my procedures are a
> bit off...
>
> I am not sure I added the right [number of] people to CC?
>
> WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> in feature/Makefile and why -ldl isn't needed in the other places. But...
>
> V2:
> - split patches further, so that tools/bpf and tools/perf part are entirely
> separate
> - included a bit more information about tests I did in commit messages
> - add a maybe_unused to fprintf_json_styled's style argument
>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Quentin Monnet <quentin@isovalent.com>
> To: bpf@vger.kernel.org
> To: linux-kernel@vger.kernel.org
> Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
>
> Andres Freund (5):
> tools build: add feature test for init_disassemble_info API changes
> tools include: add dis-asm-compat.h to handle version differences
> tools perf: Fix compilation error with new binutils
> tools bpf_jit_disasm: Fix compilation error with new binutils
> tools bpftool: Fix compilation error with new binutils
I think the disassembler checks should not be displayed by default,
with your change I can see all the time:
... disassembler-four-args: [ on ]
... disassembler-init-styled: [ OFF ]
could you please squash something like below in? moving disassembler
checks out of sight and do manual detection
thanks,
jirka
---
diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 339686b99a6e..bce9a9b52b2c 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -69,8 +69,6 @@ FEATURE_TESTS_BASIC := \
setns \
libaio \
libzstd \
- disassembler-four-args \
- disassembler-init-styled \
file-handle
# FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
@@ -106,7 +104,9 @@ FEATURE_TESTS_EXTRA := \
libbpf-bpf_create_map \
libpfm4 \
libdebuginfod \
- clang-bpf-co-re
+ clang-bpf-co-re \
+ disassembler-four-args \
+ disassembler-init-styled
FEATURE_TESTS ?= $(FEATURE_TESTS_BASIC)
@@ -135,9 +135,7 @@ FEATURE_DISPLAY ?= \
get_cpuid \
bpf \
libaio \
- libzstd \
- disassembler-four-args \
- disassembler-init-styled
+ libzstd
# Set FEATURE_CHECK_(C|LD)FLAGS-all for all FEATURE_TESTS features.
# If in the future we need per-feature checks/flags for features not
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index ee417c321adb..2aa0bad11f05 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -914,8 +914,6 @@ ifndef NO_LIBBFD
FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl
endif
endif
- $(call feature_check,disassembler-four-args)
- $(call feature_check,disassembler-init-styled)
endif
ifeq ($(feature-libbfd-buildid), 1)
@@ -1025,6 +1023,9 @@ ifdef HAVE_KVM_STAT_SUPPORT
CFLAGS += -DHAVE_KVM_STAT_SUPPORT
endif
+$(call feature_check,disassembler-four-args)
+$(call feature_check,disassembler-init-styled)
+
ifeq ($(feature-disassembler-four-args), 1)
CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
endif
Hi, On 2022-07-04 11:13:33 +0200, Jiri Olsa wrote: > I think the disassembler checks should not be displayed by default, > with your change I can see all the time: > > ... disassembler-four-args: [ on ] > ... disassembler-init-styled: [ OFF ] > > > could you please squash something like below in? moving disassembler > checks out of sight and do manual detection Makes sense - I was wondering why disassembler-four-args is displayed, but though it better to mirror the existing behaviour. Does "hiding" disassembler-four-args need to be its own set of commits? > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config > index ee417c321adb..2aa0bad11f05 100644 > --- a/tools/perf/Makefile.config > +++ b/tools/perf/Makefile.config > @@ -914,8 +914,6 @@ ifndef NO_LIBBFD > FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl > endif > endif > - $(call feature_check,disassembler-four-args) > - $(call feature_check,disassembler-init-styled) > endif > > ifeq ($(feature-libbfd-buildid), 1) > @@ -1025,6 +1023,9 @@ ifdef HAVE_KVM_STAT_SUPPORT > CFLAGS += -DHAVE_KVM_STAT_SUPPORT > endif > > +$(call feature_check,disassembler-four-args) > +$(call feature_check,disassembler-init-styled) > + > ifeq ($(feature-disassembler-four-args), 1) > CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE > endif This I don't understand - why do we want these to run under NO_LIBBFD etc? Greetings, Andres Freund
On Mon, Jul 04, 2022 at 01:19:22PM -0700, Andres Freund wrote: > Hi, > > On 2022-07-04 11:13:33 +0200, Jiri Olsa wrote: > > I think the disassembler checks should not be displayed by default, > > with your change I can see all the time: > > > > ... disassembler-four-args: [ on ] > > ... disassembler-init-styled: [ OFF ] > > > > > > could you please squash something like below in? moving disassembler > > checks out of sight and do manual detection > > Makes sense - I was wondering why disassembler-four-args is displayed, but > though it better to mirror the existing behaviour. Does "hiding" > disassembler-four-args need to be its own set of commits? I guess first hide the disassembler-four-args and add the new the same way > > > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config > > index ee417c321adb..2aa0bad11f05 100644 > > --- a/tools/perf/Makefile.config > > +++ b/tools/perf/Makefile.config > > @@ -914,8 +914,6 @@ ifndef NO_LIBBFD > > FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl > > endif > > endif > > - $(call feature_check,disassembler-four-args) > > - $(call feature_check,disassembler-init-styled) > > endif > > > > ifeq ($(feature-libbfd-buildid), 1) > > @@ -1025,6 +1023,9 @@ ifdef HAVE_KVM_STAT_SUPPORT > > CFLAGS += -DHAVE_KVM_STAT_SUPPORT > > endif > > > > +$(call feature_check,disassembler-four-args) > > +$(call feature_check,disassembler-init-styled) > > + > > ifeq ($(feature-disassembler-four-args), 1) > > CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE > > endif > > This I don't understand - why do we want these to run under NO_LIBBFD etc? when I was quickly testing that I did not have any of them detected and got compile fail.. so I moved it to safe place ;-) it might be placed in smarter place thanks, jirka > > Greetings, > > Andres Freund
Hi, On 2022-07-05 00:12:37 +0200, Jiri Olsa wrote: > On Mon, Jul 04, 2022 at 01:19:22PM -0700, Andres Freund wrote: > > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config > > > index ee417c321adb..2aa0bad11f05 100644 > > > --- a/tools/perf/Makefile.config > > > +++ b/tools/perf/Makefile.config > > > @@ -914,8 +914,6 @@ ifndef NO_LIBBFD > > > FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl > > > endif > > > endif > > > - $(call feature_check,disassembler-four-args) > > > - $(call feature_check,disassembler-init-styled) > > > endif > > > > > > ifeq ($(feature-libbfd-buildid), 1) > > > @@ -1025,6 +1023,9 @@ ifdef HAVE_KVM_STAT_SUPPORT > > > CFLAGS += -DHAVE_KVM_STAT_SUPPORT > > > endif > > > > > > +$(call feature_check,disassembler-four-args) > > > +$(call feature_check,disassembler-init-styled) > > > + > > > ifeq ($(feature-disassembler-four-args), 1) > > > CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE > > > endif > > > > This I don't understand - why do we want these to run under NO_LIBBFD etc? > > when I was quickly testing that I did not have any of them detected > and got compile fail.. so I moved it to safe place ;-) it might be > placed in smarter place I think that's because you'd removed them from FEATURE_TESTS_BASIC in Makefile.feature. In v3 I just sent out I just removed them from FEATURE_DISPLAY, without any more "structural" changes in tools/perf/Makefile.config. Greetings, Andres Freund
© 2016 - 2026 Red Hat, Inc.