Message-ID: <aF7wesWHTv_Wp-8y@google.com>
Date: Fri, 27 Jun 2025 12:26:50 -0700
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 <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 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.

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?

Thanks,
Namhyung

Powered by blists - more mailing lists