linux-kernel - Re: [PATCH v1 11/11] perf build: Rename PERF_HAVE_DWARF_REGS to PERF_HAVE_LIBDW

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Zv95OTflePPTMi7I@google.com>
Date: Thu, 3 Oct 2024 22:12:25 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Masami Hiramatsu <mhiramat@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	John Garry <john.g.garry@...cle.com>, Will Deacon <will@...nel.org>,
	James Clark <james.clark@...aro.org>,
	Mike Leach <mike.leach@...aro.org>, Leo Yan <leo.yan@...ux.dev>,
	Guo Ren <guoren@...nel.org>,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Albert Ou <aou@...s.berkeley.edu>, Nick Terrell <terrelln@...com>,
	Guilherme Amadio <amadio@...too.org>,
	Changbin Du <changbin.du@...wei.com>,
	"Steinar H. Gunderson" <sesse@...gle.com>,
	Aditya Gupta <adityag@...ux.ibm.com>,
	Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
	Masahiro Yamada <masahiroy@...nel.org>,
	Huacai Chen <chenhuacai@...nel.org>, Bibo Mao <maobibo@...ngson.cn>,
	Kajol Jain <kjain@...ux.ibm.com>, Anup Patel <anup@...infault.org>,
	Shenlin Liang <liangshenlin@...incomputing.com>,
	Atish Patra <atishp@...osinc.com>,
	Oliver Upton <oliver.upton@...ux.dev>,
	Chen Pei <cp0613@...ux.alibaba.com>,
	Dima Kogan <dima@...retsauce.net>,
	Alexander Lobakin <aleksander.lobakin@...el.com>,
	"David S. Miller" <davem@...emloft.net>,
	Przemek Kitszel <przemyslaw.kitszel@...el.com>,
	linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
	Yang Jihong <yangjihong@...edance.com>
Subject: Re: [PATCH v1 11/11] perf build: Rename PERF_HAVE_DWARF_REGS to
 PERF_HAVE_LIBDW_REGS

On Thu, Oct 03, 2024 at 05:58:13PM -0700, Ian Rogers wrote:
> On Thu, Oct 3, 2024 at 3:48 PM Namhyung Kim <namhyung@...nel.org> wrote:
> > I agree renaming libdw-specific parts.  But the register is for DWARF,
> > not libdw even if it's currently used by libdw only.   So I don't want
> > to rename it.
> 
> So your objection is that we have files called:
> tools/perf/arch/*/util/dwarf-regs.c
> and PERF_HAVE_DRWARF_REGS is an indication that this file exists. This
> file declares a single get_arch_regnum function. The building of the
> file after this series is:
> perf-util-$(CONFIG_LIBDW)     += dwarf-regs.o

Well.. I think we can even make it

perf-util-y += dwarf-regs.o

since it doesn't have any dependency on libdw.  But it'd be inefficent
to ship the dead code and data.  Anyway we may remove the condition to
define the PERF_HAVE_DWARF_REGS like below.

diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
index 67b4969a673836eb..f1eb1ee1ea25ca53 100644
--- a/tools/perf/arch/x86/Makefile
+++ b/tools/perf/arch/x86/Makefile
@@ -1,7 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-ifndef NO_DWARF
 PERF_HAVE_DWARF_REGS := 1
-endif
 HAVE_KVM_STAT_SUPPORT := 1
 PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
 PERF_HAVE_JITDUMP := 1

> 
> My objection is that PERF_HAVE_DWARF_REGS is controlling the #define
> HAVE_LIBDW_SUPPORT, so dwarf (that can mean libunwind, libdw, etc.) is
> guarding having libdw which is backward and part of what this series
> has been trying to clean up.

Why not?  If the arch doesn't define DWARF registers, it can refuse
libdw support because it won't work well.

> 
> If we rename tools/perf/arch/*/util/dwarf-regs.c to
> tools/perf/arch/*/util/libdw-helpers.c the PERF_HAVE_DWARF_REGS can be
> renamed to PERF_HAVE_LIBDW_HELPERS to align. Then
> PERF_HAVE_LIBDW_HELPERS guarding the #define PERF_HAVE_LIBDW makes
> sense to me and I think we achieve the filename alignment you are
> looking for.

I don't think it's a good idea.  The logic is not specific to libdw.

> 
> Yes get_arch_regnum could make sense out of libdw and needn't just be
> a helper for it, but let's worry about that when there's a need.
> What's confusing at the moment is does libdw provide dwarf support,
> which I'd say is expected, or does dwarf provide libdw support?

As I said, it's about refusing libdw.

  ifndef NO_LIBDW
    ifeq ($(origin PERF_HAVE_DWARF_REGS), undefined)
      $(warning DWARF register mappings have not been defined for architecture $(SRCARCH), DWARF support disabled)
      NO_LIBDW := 1
    else
      CFLAGS += -DHAVE_DWARF_SUPPORT $(LIBDW_CFLAGS)
      LDFLAGS += $(LIBDW_LDFLAGS)
      EXTLIBS += ${DWARFLIBS}
      $(call detected,CONFIG_DWARF)
    endif # PERF_HAVE_DWARF_REGS
  endif # NO_LIBDW

Thanks,
Namhyung