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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200812130517.GB4872@krava.homenet.telecomitalia.it>
Date:   Wed, 12 Aug 2020 15:05:17 +0200
From:   Jiri Olsa <jolsa@...hat.com>
To:     Rob Herring <robh@...nel.org>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        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>,
        Namhyung Kim <namhyung@...nel.org>
Subject: Re: [RFC] libperf: Add support for user space counter access

On Tue, Aug 11, 2020 at 10:49:30AM -0600, Rob Herring wrote:
> On Tue, Aug 11, 2020 at 4:50 AM Jiri Olsa <jolsa@...hat.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@...hat.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);
> 

hum, I wonder we should remove maps from perf_evsel__open
args and factor out some perf_evsel__set_map function..

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

.. because you'll need those maps in here, right?

> 
> 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.

seems ok to me, perf_evlist__mmap and perf_evsel__mmap would
mean to mutually exclusive usages

thanks,
jirka