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

[Arnaldo Carvalho de Melo]

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