Messages in this thread | | | Subject | Re: [PATCH 02/14] perf trace: Apply new perf_mmap__read_event() interface | From | "Liang, Kan" <> | Date | Mon, 5 Mar 2018 09:50:10 -0500 |
| |
On 3/5/2018 8:46 AM, Arnaldo Carvalho de Melo wrote: > Em Mon, Mar 05, 2018 at 10:03:39AM -0300, Arnaldo Carvalho de Melo escreveu: >> Em Sat, Mar 03, 2018 at 12:30:22AM +0100, Jiri Olsa escreveu: >>> On Thu, Mar 01, 2018 at 06:08:59PM -0500, kan.liang@linux.intel.com wrote: >>>> From: Kan Liang <kan.liang@linux.intel.com> >>>> The perf trace still use the legacy interface. >>>> +++ b/tools/perf/builtin-trace.c >>>> @@ -2472,8 +2472,14 @@ static int trace__run(struct trace *trace, int argc, const char **argv) >>>> >>>> for (i = 0; i < evlist->nr_mmaps; i++) { >>>> union perf_event *event; >>>> + struct perf_mmap *md; >>>> + u64 end, start; >>>> >>>> - while ((event = perf_evlist__mmap_read(evlist, i)) != NULL) { >>>> + md = &evlist->mmap[i]; >>>> + if (perf_mmap__read_init(md, 0, &start, &end) < 0) >>>> + continue; >>>> + >>>> + while ((event = perf_mmap__read_event(md, 0, &start, end)) != NULL) { >>>> struct perf_sample sample; >>>> >>>> ++trace->nr_events; >>>> @@ -2486,7 +2492,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv) >>>> >>>> trace__handle_event(trace, event, &sample); >>>> next_event: >>>> - perf_evlist__mmap_consume(evlist, i); >>>> + perf_mmap__consume(md, 0); >>> >>> could you call this with 'false' instead of 0, it's 'bool overwrite' >>> applies also to the rest of the patchset >> >> I'm doing this, the argument is 'bool', so the value should be false or >> true, even '0' being way shorter... > > While doing that I wonder why is that we can't do it without those > explicit start/end variables in all the call sites and passing > overwrite, start and end as parameters... > > Can't we just add 'overwrite, 'start' and 'end' to struct perf_mmap, > then have perf_mmap__read_init() with just 'md' and 'overwrite' as > parameters, initialize md->{start,end} and set md->overwrite to the > 'overwrite' parameter in 'perf_mmap__read_init()' and then use just > md as parameters for both perf_mmap__read_event() and > perf_mmap__consume()? >
I don't see any reason we cannot do this. I will do some test. If it works well, I will submit the follow-up patch to clean up the interface.
Thanks, Kan
> What am I missing to have this much simpler without all this > boilerplate? > > Anyway, finishing the s/0/false/g, will leave this for later, but please > comment on it. > > - Arnaldo >
| |