[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Z5QGyGQ9SfXiE3WH@google.com>
Date: Fri, 24 Jan 2025 13:31:52 -0800
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: 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>,
Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Bill Wendling <morbo@...gle.com>,
Justin Stitt <justinstitt@...gle.com>,
"Steinar H. Gunderson" <sesse@...gle.com>,
Charlie Jenkins <charlie@...osinc.com>,
Changbin Du <changbin.du@...wei.com>,
"Masami Hiramatsu (Google)" <mhiramat@...nel.org>,
James Clark <james.clark@...aro.org>,
Kajol Jain <kjain@...ux.ibm.com>,
Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
Li Huafei <lihuafei1@...wei.com>,
Dmitry Vyukov <dvyukov@...gle.com>, Andi Kleen <ak@...ux.intel.com>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
llvm@...ts.linux.dev
Subject: Re: [PATCH v1 4/5] perf capstone: Support for dlopen-ing
libcapstone.so
On Thu, Jan 23, 2025 at 01:37:16PM -0800, Ian Rogers wrote:
> > > On Wed, Jan 22, 2025 at 2:27 PM Namhyung Kim <namhyung@...nel.org> wrote:
> > > > I think reverse engineering the types is fragile. Can we simply change
> > > > to dlopen if libcapstone is available?
>
> To understand the scope of the problem, here is the LLVM code:
> https://github.com/googleprodkernel/linux-perf/blob/google_tools_master/tools/perf/util/llvm.c#L22
> 3 typedefs and 5 #defines on 13 lines of code.
It doesn't look that bad than I thought. :)
>
> The capstone code is larger primarily due to needing to define cs_x86
> (and its dependencies) which are used for extra detail in printing:
> https://github.com/googleprodkernel/linux-perf/blob/google_tools_master/tools/perf/util/capstone.c#L23-L132
> We could opt out of providing the extra detail if capstone.h isn't
> present. I believe this can remove lines 49 to 123. This would mean
> there would be 37 lines of reverse engineered types - 2 structs and 5
> enums.
Probably, we don't need to carry the full details of instructions when
the library support was not built.
>
> My feeling is that this isn't significant and better than returning
> errors for capstone/llvm, having #ifdefs/__maybe_unused for the cases
> we lack the types throughout the code and relying on people building
> noticing a warning to realize they need to install a dependency - the
> alternative of failing the build if the capstone/LLVM header files
> aren't present, and unless some NO_... build option isn't passed,
> likely won't fly with perf developers.
I don't think failing the build due to the optional dependencies is an
option. I'm not against the idea of dlopen but my concern is to carry
those external definitions in the perf code base. Maybe it'd be ok if
we could keep it minimally.
Thanks,
Namhyung
Powered by blists - more mailing lists