Messages in this thread | | | From | Arnaldo Carvalho de Melo <> | Date | Mon, 26 Aug 2019 19:08:10 -0300 | 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
| |