Messages in this thread | | | Date | Thu, 18 Jul 2013 14:52:00 +0200 | From | Jiri Olsa <> | Subject | Re: [PATCH 05/23] perf tests: Add simple session read/write test |
| |
On Wed, Jul 17, 2013 at 04:33:13PM -0300, Arnaldo Carvalho de Melo wrote: > Em Wed, Jul 17, 2013 at 07:49:45PM +0200, Jiri Olsa escreveu: > > +static struct perf_evlist *get_evlist(void) > > +{ > > + int err; > > + struct perf_evlist *evlist = perf_evlist__new(); > > + if (evlist == NULL) { > > + pr_err("perf_evlist__new failed\n"); > > + return NULL; > > + } > > + err = perf_evlist__add_default(evlist); > > + if (err < 0) { > > + pr_err("Not enough memory to create evsel\n"); > > + perf_evlist__delete(evlist); > > + evlist = NULL; > > + } > > + return evlist; > > +} > > I think the above can be such a common idiom that having a new > constructor for it is doable, i.e.: > > evlist = perf_evlist__new_default(void); > if (evlist == NULL) > pr_debug, etc
ook
> > > + > > +static union perf_event *get_event_MMAP(void) > > +{ > > + static union perf_event event; > > + size_t size; > > + > > + size = snprintf(event.mmap.filename, sizeof(event.mmap.filename), > > + "krava") + 1; > > + size = PERF_ALIGN(size, sizeof(u64)); > > + > > + event.header.type = PERF_RECORD_MMAP; > > + event.header.misc = PERF_RECORD_MISC_KERNEL; > > + event.header.size = sizeof(event.mmap) - > > + (sizeof(event.mmap.filename) - size); > > + > > + event.mmap.pgoff = 10; > > + event.mmap.start = 0; > > + event.mmap.len = 10; > > + event.mmap.pid = 123; > > + > > + return &event; > > +} > > Don't use statics, here perhaps we can have another general purpose > helper: > > void perf_event__init_mmap(union perf_event *event) > > We then could use it on the synthesizing routines, etc.
ook
>
SNIP
> > + > > +static int store_event(int fd, union perf_event *event, size_t *size) > > +{ > > + *size += event->header.size; > > + return write(fd, event, event->header.size) > 0 ? 0 : -1; > > +} > > I think we need a writen like we have readn and then do it like: > > return writen(fd, event, event->header.size) == event->header.size ? 0 : -1;
hum, we could generalize write_output function from builtin-record
> > ? >
SNIP
> > + > > + TEST_ASSERT_VAL("failed to write header", > > + !perf_session__write_header(session, evlist, fd, true)); > > Humm, can't perf_session__write_header use session->fd? Just like you > did for some other routines in previous patches in this series?
right, but need to check if the builtin-inject caller uses the same fd
thanks, jirka
| |