Message-ID: <CAP-5=fU+t=pB1TmE5DBGphaunZLCdGnRtHdxy3suCQMhxFjOiQ@mail.gmail.com>
Date: Fri, 27 Jun 2025 16:11:02 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
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 <nick.desaulniers+lkml@...il.com>, 
	Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>, 
	Aditya Gupta <adityag@...ux.ibm.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>, Chaitanya S Prakash <chaitanyas.prakash@....com>, 
	linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org, 
	llvm@...ts.linux.dev, Song Liu <song@...nel.org>, bpf@...r.kernel.org
Subject: Re: [PATCH v4 06/19] perf capstone: Support for dlopen-ing libcapstone.so

On Fri, Jun 27, 2025 at 12:26 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> On Fri, Jun 27, 2025 at 09:44:02AM -0700, Ian Rogers wrote:
> > On Thu, Jun 26, 2025 at 9:53 PM Ian Rogers <irogers@...gle.com> wrote:
> > >
> > > On Thu, Jun 26, 2025 at 4:19 PM Namhyung Kim <namhyung@...nel.org> wrote:
> > > >
> > > > On Thu, Apr 17, 2025 at 04:07:27PM -0700, Ian Rogers wrote:
> > > > > If perf wasn't built against libcapstone, no HAVE_LIBCAPSTONE_SUPPORT,
> > > > > support dlopen-ing libcapstone.so and then calling the necessary
> > > > > functions by looking them up using dlsym. Reverse engineer the types
> > > > > in the API using pahole, adding only what's used in the perf code or
> > > > > necessary for the sake of struct size and alignment.
> > > >
> > > > I still think it's simpler to require capstone headers at build time and
> > > > add LIBCAPSTONE_DYNAMIC=1 or something to support dlopen.
> > >
> > > I agree, having a header file avoids the need to declare the header
> > > file values. This is simpler. Can we make the build require
> > > libcapstone and libLLVM in the same way that libtraceevent is
> > > required? That is you have to explicitly build with NO_LIBTRACEEVENT=1
> > > to get a no libtraceevent build to succeed. If we don't do this then
> > > having LIBCAPSTONE_DYNAMIC will most likely be an unused option and
> > > not worth carrying in the code base, I think that's sad. If we require
> > > the libraries I don't like the idea of people arguing, "why do I need
> > > to install libcapstone and libLLVM just to get the kernel/perf to
> > > build now?" The non-simple, but still not very complex, approach taken
> > > here was taken as a compromise to get the best result (a perf that
> > > gets faster, BPF support, .. when libraries are available without
> > > explicitly depending on them) while trying not to offend kernel
> > > developers who are often trying to build on minimal systems.
> >
> > Fwiw, a situation that I think is analogous (and was playing on my
> > mind while writing the code) is that we don't require python to build
> > perf and carry around empty-pmu-events.c:
> > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/empty-pmu-events.c?h=perf-tools-next
> > It would be simpler (in the code base and in general) to require
> > everyone building perf to have python.
> > Having python on a system seems less of a stretch than requiring
> > libcapstone and libLLVM.
> >
> > If we keep the existing build approach, optional capstone and libLLVM
> > by detecting it as a feature, then just linking against the libraries
> > is natural. Someone would need to know they care about optionality and
> > enable LIBCAPSTONE_DYNAMIC=1. An average build where the libraries
> > weren't present would lose the libcapstone and libLLVM support. We
> > could warn about this situation but some people are upset about build
> > warnings, and if we do warn we could be pushing people into just
> > linking against libcapstone and libLLVM which seems like we'll fall
> > foul of the, "perf has too many library dependencies," complaint. We
> > could warn about linking against libraries when there is a _DYNAMIC
> > alternative like this available, but again people don't like build
> > warnings and they could legitimately want to link against libcapstone
> > or libLLVM.
> >
> > Anyway, that's why I ended up with the code in this state, to best try
> > to play off all the different compromises and complaints that have
> > been dealt with in the past.
>
> I can see your point.  Adding new build flags is likely to be unused and
> forgotten.

There's also more code to support the neither linked or nor dlopened approach.

> But I also think is that this dlopen support is mostly useful to distro
> package managers who want to support more flexible environment and
> regular dynamic linking is preferred to local builds over dlopen.  Then
> adding a note to a pull request and contacting them directly (if needed)
> might work?

If you want to run with this then I don't mind.

Thanks,
Ian

> Thanks,
> Namhyung
>

Powered by blists - more mailing lists