Messages in this thread Patch in this message | | | From | "Liang, Kan" <> | Subject | RE: [PATCH 03/10] perf tool: new iterfaces to read event from ring buffer | Date | Tue, 10 Oct 2017 18:28:18 +0000 |
| |
> Em Tue, Oct 10, 2017 at 10:20:16AM -0700, kan.liang@intel.com escreveu: > > From: Kan Liang <kan.liang@intel.com> > > > > The perf_evlist__mmap_read only support forward mode. It needs a > > common function to support both forward and backward mode. > > > The perf_evlist__mmap_read_backward is buggy. > > So, what is the bug? You state that it is buggy, but don't spell out the bug, > please do so. >
union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, int idx) { struct perf_mmap *md = &evlist->mmap[idx]; <--- it should be backward_mmap
> If it fixes an existing bug, then it should go separate from this patchkit, right?
There is no one use perf_evlist__mmap_read_backward. So it doesn't trigger any issue. I just simply drop the codes in patch 10.
Thanks, Kan > > - Arnaldo > > > Introduce a new mmap read interface to support both forward and > > backward mode. It can read event one by one from the specific range of > > ring buffer. > > > > The standard process to mmap__read event would be as below. > > perf_mmap__read_init > > while (event = perf_mmap__read_event) { > > //process the event > > perf_mmap__consume > > } > > perf_mmap__read_done > > > > The following patches will use it to replace the old interfaces. > > > > Signed-off-by: Kan Liang <kan.liang@intel.com> > > --- > > tools/perf/util/evlist.c | 41 > > +++++++++++++++++++++++++++++++++++++++++ > > tools/perf/util/evlist.h | 2 ++ > > 2 files changed, 43 insertions(+) > > > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index > > 7d23cf5..b36211e 100644 > > --- a/tools/perf/util/evlist.c > > +++ b/tools/perf/util/evlist.c > > @@ -897,6 +897,47 @@ perf_mmap__read(struct perf_mmap *md, bool > check_messup, u64 start, > > return event; > > } > > > > +/* > > + * Read the first event in the specified range of the ring buffer. > > + * Used by most of the perf tools and tests */ union perf_event * > > +perf_mmap__read_event(struct perf_mmap_read *read) { > > + struct perf_mmap *md = read->md; > > + union perf_event *event; > > + > > + /* > > + * Check if event was unmapped due to a POLLHUP/POLLERR. > > + */ > > + if (!refcount_read(&md->refcnt)) > > + return NULL; > > + > > + /* In backward, the ringbuffer is already paused. */ > > + if (!read->backward) { > > + read->end = perf_mmap__read_head(md); > > + if (!read->end) > > + return NULL; > > + } > > + > > + event = perf_mmap__read(md, !read->backward && read- > >overwrite, > > + read->start, read->end, &md->prev); > > + read->start = md->prev; > > + return event; > > +} > > + > > +/* > > + * Mandatory for backward > > + * The direction of backward mmap__read_event is from head to tail. > > + * The last mmap__read_event will set tail to md->prev. > > + * Need to correct the md->prev. > > + */ > > +void > > +perf_mmap__read_done(struct perf_mmap_read *read) { > > + read->md->prev = read->head; > > +} > > + > > union perf_event *perf_mmap__read_forward(struct perf_mmap *md, > bool > > check_messup) { > > u64 head; > > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index > > 1ce4857..53baf26 100644 > > --- a/tools/perf/util/evlist.h > > +++ b/tools/perf/util/evlist.h > > @@ -207,6 +207,8 @@ int perf_mmap__read_init(struct perf_mmap *md, > struct perf_mmap_read *read, > > bool overwrite, bool backward); > > int perf_mmap__read_to_file(struct perf_mmap_read *read, > > struct perf_data_file *file); > > +union perf_event *perf_mmap__read_event(struct perf_mmap_read > *read); > > +void perf_mmap__read_done(struct perf_mmap_read *read); > > > > int perf_evlist__open(struct perf_evlist *evlist); void > > perf_evlist__close(struct perf_evlist *evlist); > > -- > > 2.5.5
| |