Messages in this thread |  | | From | "Liang, Kan" <> | Subject | RE: [PATCH V4 02/15] perf mmap: introduce perf_mmap__read_init() | Date | Tue, 16 Jan 2018 18:12:41 +0000 |
| |
> On Mon, Jan 15, 2018 at 12:20:38PM -0800, kan.liang@intel.com wrote: > > From: Kan Liang <kan.liang@intel.com> > > > > The perf record has specific codes to calculate the ringbuffer position > > for both overwrite and non-overwrite mode. Now, only perf record > > supports both modes. The perf top will support both modes later. > > It is useful to make the specific codes generic. > > > > Introduce a new interface perf_mmap__read_init() to find ringbuffer > > position. The perf_mmap__read_init() is actually factored out from > > perf_mmap__push(). > > There are slight differences. > > - Add a check for map->refcnt > > - Add new return value logic, EAGAIN and EINVAL. > > not helpful.. I asked to separate those changes, > so we can clearly see what the refcnt check for > and what's behing that EAGAIN return > > please add separate: > 1) patch that adds perf_mmap__read_init into perf_mmap__push > with no functional change > 2) patch adds adds and explain the refcnt check > 3) patch that adds and explain the EAGAIN return >
Oh I see. I have misunderstood before. I will change it in V5.
Thanks, Kan
> thanks, > jirka > > > > > Signed-off-by: Kan Liang <kan.liang@intel.com> > > --- > > tools/perf/util/mmap.c | 43 > +++++++++++++++++++++++++++++++++++++++++++ > > tools/perf/util/mmap.h | 2 ++ > > 2 files changed, 45 insertions(+) > > > > diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c > > index 05076e6..414089f 100644 > > --- a/tools/perf/util/mmap.c > > +++ b/tools/perf/util/mmap.c > > @@ -267,6 +267,49 @@ static int overwrite_rb_find_range(void *buf, int > mask, u64 head, u64 *start, u6 > > return -1; > > } > > > > +/* > > + * Report the start and end of the available data in ringbuffer > > + */ > > +int perf_mmap__read_init(struct perf_mmap *map, bool overwrite, > > + u64 *startp, u64 *endp) > > +{ > > + unsigned char *data = map->base + page_size; > > + u64 head = perf_mmap__read_head(map); > > + u64 old = map->prev; > > + unsigned long size; > > + > > + /* > > + * Check if event was unmapped due to a POLLHUP/POLLERR. > > + */ > > + if (!refcount_read(&map->refcnt)) > > + return -EINVAL; > > + > > + *startp = overwrite ? head : old; > > + *endp = overwrite ? old : head; > > + > > + if (*startp == *endp) > > + return -EAGAIN; > > + > > + size = *endp - *startp; > > + if (size > (unsigned long)(map->mask) + 1) { > > + if (!overwrite) { > > + WARN_ONCE(1, "failed to keep up with mmap data. > (warn only once)\n"); > > + > > + map->prev = head; > > + perf_mmap__consume(map, overwrite); > > + return -EAGAIN; > > + } > > + > > + /* > > + * Backward ring buffer is full. We still have a chance to read > > + * most of data from it. > > + */ > > + if (overwrite_rb_find_range(data, map->mask, head, startp, > endp)) > > + return -EINVAL; > > + } > > + return 0; > > +} > > + > > int perf_mmap__push(struct perf_mmap *md, bool overwrite, > > void *to, int push(void *to, void *buf, size_t size)) > > { > > diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h > > index e43d7b5..0633308 100644 > > --- a/tools/perf/util/mmap.h > > +++ b/tools/perf/util/mmap.h > > @@ -94,4 +94,6 @@ int perf_mmap__push(struct perf_mmap *md, bool > backward, > > > > size_t perf_mmap__mmap_len(struct perf_mmap *map); > > > > +int perf_mmap__read_init(struct perf_mmap *map, bool overwrite, > > + u64 *startp, u64 *endp); > > #endif /*__PERF_MMAP_H */ > > -- > > 2.5.5 > >
|  |