Re: [RFC] libperf: Add support for user space counter access

From: Rob Herring <robh@kernel.org>
To: Jiri Olsa <jolsa@redhat.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Namhyung Kim <namhyung@kernel.org>
Subject: Re: [RFC] libperf: Add support for user space counter access
Date: Tue, 11 Aug 2020 10:49:30 -0600	[thread overview]
Message-ID: <CAL_Jsq+tsTrYCddSSb5d2vC7TTKUZTgjBQr4FHXBKCePo-beZQ@mail.gmail.com> (raw)
In-Reply-To: <20200811105027.GD699846@krava>

On Tue, Aug 11, 2020 at 4:50 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Aug 10, 2020 at 12:11:23PM -0600, Rob Herring wrote:
> > On Sat, Aug 8, 2020 at 4:22 AM Jiri Olsa <jolsa@redhat.com> wrote:
> > >
> > > On Fri, Aug 07, 2020 at 05:05:17PM -0600, Rob Herring wrote:
> > > > x86 and arm64 can both support direct access of event counters in
> > > > userspace. The access sequence is less than trivial and currently exists
> > > > in perf test code (tools/perf/arch/x86/tests/rdpmc.c) with copies in
> > > > projects such as PAPI and libpfm4.
> > > >
> > > > Patches to add arm64 userspace support are pending[1].
> > > >
> > > > For this RFC, looking for a yes, seems like a good idea, or no, go away we
> > > > don't want this in libperf.
> > >
> > > hi,
> > > looks great!
> > >
> > > I wanted to add this for very long time.. so yes, we want this ;-)
> >
> > Thanks for the quick feedback. Would this be better implemented as a
> > fast path for perf_evsel__read()? If so, how to get the mmap data
>
> if it works for all events, which I'm not sure of
>
> > which is associated with a evlist rather than a evsel?
>
> not sure what you mean, you can mmap evsel, not evlist

While yes the mmap is created from an evsel fd, they are ultimately
associated with the evlist struct and are per thread or cpu. If
there's more than 1 evsel, then the additional ones are set to the 1st
mmap with PERF_EVENT_IOC_SET_OUTPUT. Which I now realize means this
RFC only works for the first evsel. So I guess the API needs to work
something like this:

threads = perf_thread_map__new_dummy();
perf_thread_map__set_pid(threads, 0, 0);

evsel = perf_evsel__new(&attr);
perf_evsel__open(evsel, NULL, threads);

perf_evsel__mmap(evsel);    <--- *new*

perf_evsel__read(evsel, 0, 0, &counts);  // If we have an mmap, then
try a direct read

Perhaps some refactoring of the mmap code in evlist.c will be needed,
but the usage seems pretty orthogonal. I'd propose that mmapping via
perf_evlist__mmap() behavior remain unchanged and direct access is not
supported in that case.

Rob