linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] perf build: Make libunwind opt-in rather than opt-out
@ 2024-10-28 19:36 Ian Rogers
  2024-10-28 21:49 ` Arnaldo Carvalho de Melo
  2024-11-05 16:41 ` Namhyung Kim
  0 siblings, 2 replies; 3+ messages in thread
From: Ian Rogers @ 2024-10-28 19:36 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, linux-perf-users,
	linux-kernel

Having multiple unwinding libraries makes the perf code harder to
understand and we have unused/untested code paths.

Perf made BPF support an opt-out rather than opt-in feature. As libbpf
has a libelf dependency, elfutils that provides libelf will also
provide libdw. When libdw is present perf will use libdw unwinding
rather than libunwind unwinding even if libunwind support is compiled
in.

Rather than have libunwind built into perf and never used, explicitly
disable the support and make it opt-in.

Signed-off-by: Ian Rogers <irogers@google.com>
Closes: https://lore.kernel.org/linux-perf-users/CAP-5=fUXkp-d7gkzX4eF+nbjb2978dZsiHZ9abGHN=BN1qAcbg@mail.gmail.com/
---
v2: update build tests.
---
 tools/perf/Makefile.config | 4 ++++
 tools/perf/Makefile.perf   | 2 +-
 tools/perf/tests/make      | 6 +++---
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index b93ed2b7623f..707d7355ff18 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -91,6 +91,10 @@ ifneq ($(SRCARCH),$(filter $(SRCARCH),x86 arm arm64 powerpc s390 csky riscv loon
   NO_LIBDW_DWARF_UNWIND := 1
 endif
 
+ifneq ($(LIBUNWIND),1)
+  NO_LIBUNWIND := 1
+endif
+
 ifeq ($(LIBUNWIND_LIBS),)
   NO_LIBUNWIND := 1
 endif
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index b4dee7c20ed1..d74241a15131 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -52,7 +52,7 @@ include ../scripts/utilities.mak
 #
 # Define NO_LIBELF if you do not want libelf dependency (e.g. cross-builds)
 #
-# Define NO_LIBUNWIND if you do not want libunwind dependency for dwarf
+# Define LIBUNWIND if you do not want libunwind dependency for dwarf
 # backtrace post unwind.
 #
 # Define NO_BACKTRACE if you do not want stack backtrace debug feature
diff --git a/tools/perf/tests/make b/tools/perf/tests/make
index a5040772043f..a7fcbd589752 100644
--- a/tools/perf/tests/make
+++ b/tools/perf/tests/make
@@ -81,7 +81,7 @@ make_no_gtk2        := NO_GTK2=1
 make_no_ui          := NO_SLANG=1 NO_GTK2=1
 make_no_demangle    := NO_DEMANGLE=1
 make_no_libelf      := NO_LIBELF=1
-make_no_libunwind   := NO_LIBUNWIND=1
+make_libunwind      := LIBUNWIND=1
 make_no_libdw_dwarf_unwind := NO_LIBDW_DWARF_UNWIND=1
 make_no_backtrace   := NO_BACKTRACE=1
 make_no_libcapstone := NO_CAPSTONE=1
@@ -121,7 +121,7 @@ make_static         := LDFLAGS=-static NO_PERF_READ_VDSO32=1 NO_PERF_READ_VDSOX3
 
 # all the NO_* variable combined
 make_minimal        := NO_LIBPERL=1 NO_LIBPYTHON=1 NO_GTK2=1
-make_minimal        += NO_DEMANGLE=1 NO_LIBELF=1 NO_LIBUNWIND=1 NO_BACKTRACE=1
+make_minimal        += NO_DEMANGLE=1 NO_LIBELF=1 NO_BACKTRACE=1
 make_minimal        += NO_LIBNUMA=1 NO_LIBAUDIT=1 NO_LIBBIONIC=1
 make_minimal        += NO_LIBDW_DWARF_UNWIND=1 NO_AUXTRACE=1 NO_LIBBPF=1
 make_minimal        += NO_LIBCRYPTO=1 NO_SDT=1 NO_JVMTI=1 NO_LIBZSTD=1
@@ -153,7 +153,7 @@ run += make_no_gtk2
 run += make_no_ui
 run += make_no_demangle
 run += make_no_libelf
-run += make_no_libunwind
+run += make_libunwind
 run += make_no_libdw_dwarf_unwind
 run += make_no_backtrace
 run += make_no_libcapstone
-- 
2.47.0.163.g1226f6d8fa-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] perf build: Make libunwind opt-in rather than opt-out
  2024-10-28 19:36 [PATCH v2] perf build: Make libunwind opt-in rather than opt-out Ian Rogers
@ 2024-10-28 21:49 ` Arnaldo Carvalho de Melo
  2024-11-05 16:41 ` Namhyung Kim
  1 sibling, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-10-28 21:49 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
	linux-perf-users, linux-kernel

On Mon, Oct 28, 2024 at 12:36:19PM -0700, Ian Rogers wrote:
> Having multiple unwinding libraries makes the perf code harder to
> understand and we have unused/untested code paths.
 
> Perf made BPF support an opt-out rather than opt-in feature. As libbpf
> has a libelf dependency, elfutils that provides libelf will also
> provide libdw. When libdw is present perf will use libdw unwinding
> rather than libunwind unwinding even if libunwind support is compiled
> in.
 
> Rather than have libunwind built into perf and never used, explicitly
> disable the support and make it opt-in.

Looks ok and testing it does what is advertised:

⬢ [acme@toolbox perf-tools-next]$ ldd ~/bin/perf | grep libunwind
⬢ [acme@toolbox perf-tools-next]$ perf check feature libunwind
             libunwind: [ OFF ]  # HAVE_LIBUNWIND_SUPPORT
⬢ [acme@toolbox perf-tools-next]$

Since I have libunwind-devel installed:

Auto-detecting system features:
<SNIP
...                               libunwind: [ on  ]
<SNIP

If I uninstall libunwind-devel, just to complete the cycle, even this
patch not touching that part:

Auto-detecting system features:
<SNIP>
...                               libunwind: [ OFF ]
<SNIP>

Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> Closes: https://lore.kernel.org/linux-perf-users/CAP-5=fUXkp-d7gkzX4eF+nbjb2978dZsiHZ9abGHN=BN1qAcbg@mail.gmail.com/
> ---
> v2: update build tests.
> ---
>  tools/perf/Makefile.config | 4 ++++
>  tools/perf/Makefile.perf   | 2 +-
>  tools/perf/tests/make      | 6 +++---
>  3 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index b93ed2b7623f..707d7355ff18 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -91,6 +91,10 @@ ifneq ($(SRCARCH),$(filter $(SRCARCH),x86 arm arm64 powerpc s390 csky riscv loon
>    NO_LIBDW_DWARF_UNWIND := 1
>  endif
>  
> +ifneq ($(LIBUNWIND),1)
> +  NO_LIBUNWIND := 1
> +endif
> +
>  ifeq ($(LIBUNWIND_LIBS),)
>    NO_LIBUNWIND := 1
>  endif
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index b4dee7c20ed1..d74241a15131 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -52,7 +52,7 @@ include ../scripts/utilities.mak
>  #
>  # Define NO_LIBELF if you do not want libelf dependency (e.g. cross-builds)
>  #
> -# Define NO_LIBUNWIND if you do not want libunwind dependency for dwarf
> +# Define LIBUNWIND if you do not want libunwind dependency for dwarf
>  # backtrace post unwind.
>  #
>  # Define NO_BACKTRACE if you do not want stack backtrace debug feature
> diff --git a/tools/perf/tests/make b/tools/perf/tests/make
> index a5040772043f..a7fcbd589752 100644
> --- a/tools/perf/tests/make
> +++ b/tools/perf/tests/make
> @@ -81,7 +81,7 @@ make_no_gtk2        := NO_GTK2=1
>  make_no_ui          := NO_SLANG=1 NO_GTK2=1
>  make_no_demangle    := NO_DEMANGLE=1
>  make_no_libelf      := NO_LIBELF=1
> -make_no_libunwind   := NO_LIBUNWIND=1
> +make_libunwind      := LIBUNWIND=1
>  make_no_libdw_dwarf_unwind := NO_LIBDW_DWARF_UNWIND=1
>  make_no_backtrace   := NO_BACKTRACE=1
>  make_no_libcapstone := NO_CAPSTONE=1
> @@ -121,7 +121,7 @@ make_static         := LDFLAGS=-static NO_PERF_READ_VDSO32=1 NO_PERF_READ_VDSOX3
>  
>  # all the NO_* variable combined
>  make_minimal        := NO_LIBPERL=1 NO_LIBPYTHON=1 NO_GTK2=1
> -make_minimal        += NO_DEMANGLE=1 NO_LIBELF=1 NO_LIBUNWIND=1 NO_BACKTRACE=1
> +make_minimal        += NO_DEMANGLE=1 NO_LIBELF=1 NO_BACKTRACE=1
>  make_minimal        += NO_LIBNUMA=1 NO_LIBAUDIT=1 NO_LIBBIONIC=1
>  make_minimal        += NO_LIBDW_DWARF_UNWIND=1 NO_AUXTRACE=1 NO_LIBBPF=1
>  make_minimal        += NO_LIBCRYPTO=1 NO_SDT=1 NO_JVMTI=1 NO_LIBZSTD=1
> @@ -153,7 +153,7 @@ run += make_no_gtk2
>  run += make_no_ui
>  run += make_no_demangle
>  run += make_no_libelf
> -run += make_no_libunwind
> +run += make_libunwind
>  run += make_no_libdw_dwarf_unwind
>  run += make_no_backtrace
>  run += make_no_libcapstone
> -- 
> 2.47.0.163.g1226f6d8fa-goog

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] perf build: Make libunwind opt-in rather than opt-out
  2024-10-28 19:36 [PATCH v2] perf build: Make libunwind opt-in rather than opt-out Ian Rogers
  2024-10-28 21:49 ` Arnaldo Carvalho de Melo
@ 2024-11-05 16:41 ` Namhyung Kim
  1 sibling, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2024-11-05 16:41 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, linux-perf-users, linux-kernel, Ian Rogers

On Mon, 28 Oct 2024 12:36:19 -0700, Ian Rogers wrote:

> Having multiple unwinding libraries makes the perf code harder to
> understand and we have unused/untested code paths.
> 
> Perf made BPF support an opt-out rather than opt-in feature. As libbpf
> has a libelf dependency, elfutils that provides libelf will also
> provide libdw. When libdw is present perf will use libdw unwinding
> rather than libunwind unwinding even if libunwind support is compiled
> in.
> 
> [...]

Applied to perf-tools-next, thanks!

Best regards,
Namhyung


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-11-05 16:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-28 19:36 [PATCH v2] perf build: Make libunwind opt-in rather than opt-out Ian Rogers
2024-10-28 21:49 ` Arnaldo Carvalho de Melo
2024-11-05 16:41 ` Namhyung Kim

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).