[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fWrJoej0XprvPmYg5qa7SRU2s3yeCF-hWgj+3MTdTZLzQ@mail.gmail.com>
Date: Fri, 23 Jan 2026 08:30:22 -0800
From: Ian Rogers <irogers@...gle.com>
To: James Clark <james.clark@...aro.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, John Garry <john.g.garry@...cle.com>,
Will Deacon <will@...nel.org>, Leo Yan <leo.yan@...ux.dev>, Guo Ren <guoren@...nel.org>,
Paul Walmsley <pjw@...nel.org>, Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
Alexandre Ghiti <alex@...ti.fr>, Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Bill Wendling <morbo@...gle.com>,
Justin Stitt <justinstitt@...gle.com>, Zecheng Li <zecheng@...gle.com>,
Tianyou Li <tianyou.li@...el.com>, Thomas Falcon <thomas.falcon@...el.com>,
Julia Lawall <Julia.Lawall@...ia.fr>, Suchit Karunakaran <suchitkarunakaran@...il.com>,
Athira Rajeev <atrajeev@...ux.ibm.com>, Aditya Bodkhe <aditya.b1@...ux.ibm.com>,
Howard Chu <howardchu95@...il.com>,
Krzysztof Łopatowski <krzysztof.m.lopatowski@...il.com>,
"Dr. David Alan Gilbert" <linux@...blig.org>, Shimin Guo <shimin.guo@...dio.com>,
Sergei Trofimovich <slyich@...il.com>, linux-kernel@...r.kernel.org,
linux-perf-users@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-csky@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v3 00/12] perf annotate arch clean up
On Fri, Jan 23, 2026 at 3:02 AM James Clark <james.clark@...aro.org> wrote:
>
>
>
> On 22/01/2026 9:35 pm, Ian Rogers wrote:
> > 3 initial memory leak fix patches for perf annotate.
> >
> > Move the disasm.c #included arch files into a new util/annotate-arch
> > directory. Make the code compile standalone. Try to better encapsulate
> > architecture specific code within the files and with a
> > arch__new_<arch> function that allows additional data to be associated
> > with the struct arch using the standard container_of trick.
> >
> > Make the lookup of struct arch use the ELF machine gather from a
> > thread rather than an evsel. For example, the evsel can't determine
> > the difference between EM_386 and EM_X86_64 so generally the latter
> > has currently been used. This means the dwarf registers are off as
> > register 1 is CX with EM_386 and DX with EM_X86_64. To facilitate this
> > change map_symbol is refactored to use a struct thread rather than a
> > struct maps, the maps being obtainable from the thread.
> >
> > To make in and out arguments clearer, for safety, etc. try to constify
> > struct arch and its related structs.
> >
> > v3: Fix arch initialization on arm, arm64, powerpc and sparc as spotted by Suchit
> > Karunakaranw.
> >
> > v2: Add a patch to fix a domain info memory leak that was blocking
> > leak sanitizer analysis. Fix more map_symbol leaks or uses after
> > reference count put (Suchit Karunakaranw
> > <suchitkarunakaran@...il.com>). Try to repro annotate test crash
> > (James Clark) but suspect it was addressed in the other reference
> > count fixes.
> > https://lore.kernel.org/lkml/20260122165407.561888-1-irogers@google.com/
> >
> > v1: https://lore.kernel.org/lkml/20260122073127.375139-1-irogers@google.com/
> >
> > Ian Rogers (12):
> > perf header: Fix memory leaks in process_cpu_domain_info
> > perf annotate: Fix args leak of map_symbol
> > perf maps: Fix reference count leak in maps__find_ams
> > perf disasm: Constify use of struct arch
> > perf disasm: Constify use of struct ins_op
> > perf disasm: Constify use of struct ins
> > perf disasm: Rework the string arch__is to use the ELF machine
> > perf disasm: Don't include C files from the arch directory
> > perf disasm: Refactor ins__is_call/jump to avoid exposing arch ins_ops
> > perf map_symbol: Switch from holding maps to holding thread
> > perf disasm: Refactor arch__find and initialization of arch structs
> > perf disasm: Minor layout tweaks for struct arch
> >
> > tools/perf/arch/arc/annotate/instructions.c | 11 -
> > .../perf/arch/riscv64/annotate/instructions.c | 36 --
> > tools/perf/ui/browsers/annotate.c | 6 +-
> > tools/perf/ui/browsers/hists.c | 3 +-
> > tools/perf/util/Build | 1 +
> > tools/perf/util/addr_location.c | 4 -
> > tools/perf/util/addr_location.h | 1 -
> > tools/perf/util/annotate-arch/Build | 11 +
> > tools/perf/util/annotate-arch/annotate-arc.c | 18 +
> > .../annotate-arch/annotate-arm.c} | 50 +-
> > .../annotate-arch/annotate-arm64.c} | 59 +--
> > .../annotate-arch/annotate-csky.c} | 29 +-
> > .../annotate-arch/annotate-loongarch.c} | 71 +--
> > .../annotate-arch/annotate-mips.c} | 29 +-
> > .../annotate-arch/annotate-powerpc.c} | 127 +++++-
> > .../util/annotate-arch/annotate-riscv64.c | 41 ++
> > .../annotate-arch/annotate-s390.c} | 70 +--
> > .../annotate-arch/annotate-sparc.c} | 28 +-
> > .../annotate-arch/annotate-x86.c} | 89 ++--
> > tools/perf/util/annotate-data.c | 12 +-
> > tools/perf/util/annotate-data.h | 2 +-
> > tools/perf/util/annotate.c | 88 ++--
> > tools/perf/util/annotate.h | 10 +-
> > tools/perf/util/callchain.c | 12 +-
> > tools/perf/util/capstone.c | 18 +-
> > tools/perf/util/db-export.c | 1 -
> > tools/perf/util/disasm.c | 428 +++++++-----------
> > tools/perf/util/disasm.h | 95 ++--
> > tools/perf/util/event.c | 2 -
> > tools/perf/util/header.c | 17 +-
> > tools/perf/util/hist.c | 18 +-
> > tools/perf/util/llvm.c | 8 +-
> > tools/perf/util/machine.c | 13 +-
> > tools/perf/util/map_symbol.c | 5 +-
> > tools/perf/util/map_symbol.h | 3 +-
> > tools/perf/util/maps.c | 1 +
> > tools/perf/util/sort.c | 2 +-
> > tools/perf/util/unwind-libdw.c | 2 +-
> > tools/perf/util/unwind-libunwind-local.c | 2 +-
> > 39 files changed, 775 insertions(+), 648 deletions(-)
> > delete mode 100644 tools/perf/arch/arc/annotate/instructions.c
> > delete mode 100644 tools/perf/arch/riscv64/annotate/instructions.c
> > create mode 100644 tools/perf/util/annotate-arch/Build
> > create mode 100644 tools/perf/util/annotate-arch/annotate-arc.c
> > rename tools/perf/{arch/arm/annotate/instructions.c => util/annotate-arch/annotate-arm.c} (59%)
> > rename tools/perf/{arch/arm64/annotate/instructions.c => util/annotate-arch/annotate-arm64.c} (70%)
> > rename tools/perf/{arch/csky/annotate/instructions.c => util/annotate-arch/annotate-csky.c} (67%)
> > rename tools/perf/{arch/loongarch/annotate/instructions.c => util/annotate-arch/annotate-loongarch.c} (60%)
> > rename tools/perf/{arch/mips/annotate/instructions.c => util/annotate-arch/annotate-mips.c} (59%)
> > rename tools/perf/{arch/powerpc/annotate/instructions.c => util/annotate-arch/annotate-powerpc.c} (73%)
> > create mode 100644 tools/perf/util/annotate-arch/annotate-riscv64.c
> > rename tools/perf/{arch/s390/annotate/instructions.c => util/annotate-arch/annotate-s390.c} (66%)
> > rename tools/perf/{arch/sparc/annotate/instructions.c => util/annotate-arch/annotate-sparc.c} (85%)
> > rename tools/perf/{arch/x86/annotate/instructions.c => util/annotate-arch/annotate-x86.c} (93%)
> >
>
> Reviewed-by: James Clark <james.clark@...aro.org>
Thanks James!
Arnaldo, it'd be nice for this to land so I can look into addressing
the missing e_flags issue - namely, we should optionally read the
e_flags along with the e_machine so that we have a proper e_flags
value to make csky work properly (not an issue on anything else). At
the same time I think some of the e_machine should be made more robust
like with:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/commit/tools/perf?h=perf-tools-next&id=e99d544c7f3691eb321c88fdbadf04b777c114c4
but here:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/thread.c?h=perf-tools-next#n452
the code should probably be shared.
Anyway, this series does enough without adding all that to it and
waiting for more tags.
Thanks,
Ian
Powered by blists - more mailing lists