linux-kernel - Re: [PATCH 00/12] libperf: Add events to perf/event.h

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 26 Aug 2019 19:08:10 -0300
From:   Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     Jiri Olsa <jolsa@...nel.org>, lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Michael Petlan <mpetlan@...hat.com>
Subject: Re: [PATCH 00/12] libperf: Add events to perf/event.h

Em Mon, Aug 26, 2019 at 06:47:34PM +0200, Jiri Olsa escreveu:
> On Mon, Aug 26, 2019 at 12:41:38PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Sun, Aug 25, 2019 at 08:17:40PM +0200, Jiri Olsa escreveu:
> > > hi,
> > > as a preparation for sampling libperf interface, moving event
> > > definitions into the library header. Moving just the kernel 
> > > non-AUX events now.
> > > 
> > > In order to keep libperf simple, we switch 'u64/u32/u16/u8'
> > > types used events to their generic '__u*' versions.
> > > 
> > > Perf added 'u*' types mainly to ease up printing __u64 values
> > > as stated in the linux/types.h comment:
> > > 
> > >   /*
> > >    * We define u64 as uint64_t for every architecture
> > >    * so that we can print it with "%"PRIx64 without getting warnings.
> > >    *
> > >    * typedef __u64 u64;
> > >    * typedef __s64 s64;
> > >    */
> > > 
> > > Adding and using new PRI_lu64 and PRI_lx64 macros to be used for
> > > that.  Using extra '_' to ease up the reading and differentiate
> > > them from standard PRI*64 macros.
> > 
> > I think we should take advantage of this moment to rename those structs
> > to have the 'perf_record_' prefix on them, I guess we could even remove
> > the _event from them, i.e.:
> > 
> > 'struct mmap_event' becomes 'perf_record_mmap', as it is the description
> > for the PERF_RECORD_MMAP meta-data event, are you ok with that?
> 
> hum, not sure about loosing the '_event' here, but we are
> not public yet, so we can always change back ;-) I do like
> it'd follow the enum name

So I'm making the already exported to libperf to be renamed to have the
same name as the PERF_RECORD_ enum they map to, just all lowercase.

Looks nice and also makes something exported by libperf to have a perf_
namespace prefix.

BTW: you forgot to move PERF_RECORD_CONTEXT_SWITCH :-)

> > I can go ahead and do it myself, updating each patch on this series to
> > do that.
> 
> sure, I thought we'd do it later, but feel free to do it,
> maybe in separate changes?

I did it as a separate patch, one patch for all the PERF_RECORD_ already
moved to libperf.

Also some other minor stuff, like having that
perf_event.{bpf,ksymbol}_event renamed to play perf_event.{bpf,ksymbol},
like the other records. so to make this idiom more compact and less
redundant:

    event->bpf_event

becomes:

    event->bpf

ditto for ksymbol_event.

- Arnaldo