| Message ID | 1461723563-67451-4-git-send-email-wangnan0@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 2016/5/6 4:07, Arnaldo Carvalho de Melo wrote: > Em Wed, Apr 27, 2016 at 02:19:22AM +0000, Wang Nan escreveu: >> perf_evlist__mmap_read_backward() is introduced for reading backward >> ring buffer. Different from reading forward, before reading, caller >> needs to call perf_evlist__mmap_read_catchup() first. >> >> Backward ring buffer should be read from 'head' pointer, not '0'. >> perf_evlist__mmap_read_catchup() saves 'head' to 'md->prev', then >> make it remember the start position after each reading. >> >> Signed-off-by: Wang Nan <wangnan0@huawei.com> >> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> >> Cc: Peter Zijlstra <peterz@infradead.org> >> Cc: Zefan Li <lizefan@huawei.com> >> Cc: pi3orama@163.com >> --- >> tools/perf/util/evlist.c | 39 +++++++++++++++++++++++++++++++++++++++ >> tools/perf/util/evlist.h | 4 ++++ >> 2 files changed, 43 insertions(+) >> >> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c >> index 17cd014..2e0b7b0 100644 >> --- a/tools/perf/util/evlist.c >> +++ b/tools/perf/util/evlist.c >> @@ -766,6 +766,45 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx) >> return perf_mmap__read(md, evlist->overwrite, old, head, &md->prev); >> } >> >> +union perf_event * >> +perf_evlist__mmap_read_backward(struct perf_evlist *evlist, int idx) >> +{ >> + struct perf_mmap *md = &evlist->mmap[idx]; >> + u64 head, end; >> + u64 start = md->prev; >> + >> + /* >> + * Check if event was unmapped due to a POLLHUP/POLLERR. >> + */ >> + if (!atomic_read(&md->refcnt)) >> + return NULL; >> + >> + /* NOTE: head is negative in this case */ > What is this comment for, can you ellaborate? Are you double sure this > arithmetic with u64, negative values, that -head below are all ok? Yes. In backward ring buffer, kernel write data from '0'. Each time it write a record, it minus sizeof(record) from 'head' pointer. So '-head' means number of bytes already written. > I've applied the first two patches in this series. > > I also need to check why we now need that catchup thing :-\ I think catchup is necessary. I tried to get rid of it by setting md->prev to -1 and initialize it to 0 when reading forwardly and to head when reading backwardly, but it require too much code adjustments. >> + head = perf_mmap__read_head(md); >> + >> + if (!head) >> + return NULL; >> + >> + end = head + md->mask + 1; >> + >> + if ((end - head) > -head) >> + end = 0; >> + This '-head' is used to detect wrapping. Never use positive pointer. >> + return perf_mmap__read(md, false, start, end, &md->prev); >> +} >> + >> +void perf_evlist__mmap_read_catchup(struct perf_evlist *evlist, int idx) >> +{ >> + struct perf_mmap *md = &evlist->mmap[idx]; >> + u64 head; >> + >> + if (!atomic_read(&md->refcnt)) >> + return; >> + >> + head = perf_mmap__read_head(md); >> + md->prev = head; >> +} >> + >> static bool perf_mmap__empty(struct perf_mmap *md) >> { >> return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base; >> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h >> index 208897a..85d1b59 100644 >> --- a/tools/perf/util/evlist.h >> +++ b/tools/perf/util/evlist.h >> @@ -129,6 +129,10 @@ struct perf_sample_id *perf_evlist__id2sid(struct perf_evlist *evlist, u64 id); >> >> union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx); >> >> +union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, >> + int idx); >> +void perf_evlist__mmap_read_catchup(struct perf_evlist *evlist, int idx); >> + >> void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx); >> >> int perf_evlist__open(struct perf_evlist *evlist); >> -- >> 1.8.3.4
On 2016/5/6 21:40, Wangnan (F) wrote: > > > On 2016/5/6 4:07, Arnaldo Carvalho de Melo wrote: >> Em Wed, Apr 27, 2016 at 02:19:22AM +0000, Wang Nan escreveu: >>> perf_evlist__mmap_read_backward() is introduced for reading backward >>> ring buffer. Different from reading forward, before reading, caller >>> needs to call perf_evlist__mmap_read_catchup() first. >>> >>> Backward ring buffer should be read from 'head' pointer, not '0'. >>> perf_evlist__mmap_read_catchup() saves 'head' to 'md->prev', then >>> make it remember the start position after each reading. >>> >>> Signed-off-by: Wang Nan <wangnan0@huawei.com> >>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> >>> Cc: Peter Zijlstra <peterz@infradead.org> >>> Cc: Zefan Li <lizefan@huawei.com> >>> Cc: pi3orama@163.com >>> --- >>> tools/perf/util/evlist.c | 39 +++++++++++++++++++++++++++++++++++++++ >>> tools/perf/util/evlist.h | 4 ++++ >>> 2 files changed, 43 insertions(+) >>> >>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c >>> index 17cd014..2e0b7b0 100644 >>> --- a/tools/perf/util/evlist.c >>> +++ b/tools/perf/util/evlist.c >>> @@ -766,6 +766,45 @@ union perf_event *perf_evlist__mmap_read(struct >>> perf_evlist *evlist, int idx) >>> return perf_mmap__read(md, evlist->overwrite, old, head, >>> &md->prev); >>> } >>> +union perf_event * >>> +perf_evlist__mmap_read_backward(struct perf_evlist *evlist, int idx) >>> +{ >>> + struct perf_mmap *md = &evlist->mmap[idx]; >>> + u64 head, end; >>> + u64 start = md->prev; >>> + >>> + /* >>> + * Check if event was unmapped due to a POLLHUP/POLLERR. >>> + */ >>> + if (!atomic_read(&md->refcnt)) >>> + return NULL; >>> + >>> + /* NOTE: head is negative in this case */ >> What is this comment for, can you ellaborate? Are you double sure this >> arithmetic with u64, negative values, that -head below are all ok? > > Yes. In backward ring buffer, kernel write data from '0'. Each > time it write a record, it minus sizeof(record) from 'head' > pointer. So '-head' means number of bytes already written. > >> I've applied the first two patches in this series. >> >> I also need to check why we now need that catchup thing :-\ > > I think catchup is necessary. I tried to get rid of it by setting > md->prev to -1 and initialize it to 0 when reading forwardly and > to head when reading backwardly, but it require too much code > adjustments. > In addtion, reader of backward ring buffer tends to fetch the most recent record. Since each reading gets earlier record, a catchup function is always required. Thank you. >>> + head = perf_mmap__read_head(md); >>> + >>> + if (!head) >>> + return NULL; >>> + >>> + end = head + md->mask + 1; >>> + >>> + if ((end - head) > -head) >>> + end = 0; >>> + > > This '-head' is used to detect wrapping. Never use positive pointer. > >>> + return perf_mmap__read(md, false, start, end, &md->prev); >>> +} >>> + >>> +void perf_evlist__mmap_read_catchup(struct perf_evlist *evlist, int >>> idx) >>> +{ >>> + struct perf_mmap *md = &evlist->mmap[idx]; >>> + u64 head; >>> + >>> + if (!atomic_read(&md->refcnt)) >>> + return; >>> + >>> + head = perf_mmap__read_head(md); >>> + md->prev = head; >>> +} >>> + >>> static bool perf_mmap__empty(struct perf_mmap *md) >>> { >>> return perf_mmap__read_head(md) == md->prev && >>> !md->auxtrace_mmap.base; >>> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h >>> index 208897a..85d1b59 100644 >>> --- a/tools/perf/util/evlist.h >>> +++ b/tools/perf/util/evlist.h >>> @@ -129,6 +129,10 @@ struct perf_sample_id >>> *perf_evlist__id2sid(struct perf_evlist *evlist, u64 id); >>> union perf_event *perf_evlist__mmap_read(struct perf_evlist >>> *evlist, int idx); >>> +union perf_event *perf_evlist__mmap_read_backward(struct >>> perf_evlist *evlist, >>> + int idx); >>> +void perf_evlist__mmap_read_catchup(struct perf_evlist *evlist, int >>> idx); >>> + >>> void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx); >>> int perf_evlist__open(struct perf_evlist *evlist); >>> -- >>> 1.8.3.4 >
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 17cd014..2e0b7b0 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -766,6 +766,45 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx) return perf_mmap__read(md, evlist->overwrite, old, head, &md->prev); } +union perf_event * +perf_evlist__mmap_read_backward(struct perf_evlist *evlist, int idx) +{ + struct perf_mmap *md = &evlist->mmap[idx]; + u64 head, end; + u64 start = md->prev; + + /* + * Check if event was unmapped due to a POLLHUP/POLLERR. + */ + if (!atomic_read(&md->refcnt)) + return NULL; + + /* NOTE: head is negative in this case */ + head = perf_mmap__read_head(md); + + if (!head) + return NULL; + + end = head + md->mask + 1; + + if ((end - head) > -head) + end = 0; + + return perf_mmap__read(md, false, start, end, &md->prev); +} + +void perf_evlist__mmap_read_catchup(struct perf_evlist *evlist, int idx) +{ + struct perf_mmap *md = &evlist->mmap[idx]; + u64 head; + + if (!atomic_read(&md->refcnt)) + return; + + head = perf_mmap__read_head(md); + md->prev = head; +} + static bool perf_mmap__empty(struct perf_mmap *md) { return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base; diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index 208897a..85d1b59 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -129,6 +129,10 @@ struct perf_sample_id *perf_evlist__id2sid(struct perf_evlist *evlist, u64 id); union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx); +union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, + int idx); +void perf_evlist__mmap_read_catchup(struct perf_evlist *evlist, int idx); + void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx); int perf_evlist__open(struct perf_evlist *evlist);
perf_evlist__mmap_read_backward() is introduced for reading backward ring buffer. Different from reading forward, before reading, caller needs to call perf_evlist__mmap_read_catchup() first. Backward ring buffer should be read from 'head' pointer, not '0'. perf_evlist__mmap_read_catchup() saves 'head' to 'md->prev', then make it remember the start position after each reading. Signed-off-by: Wang Nan <wangnan0@huawei.com> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Zefan Li <lizefan@huawei.com> Cc: pi3orama@163.com --- tools/perf/util/evlist.c | 39 +++++++++++++++++++++++++++++++++++++++ tools/perf/util/evlist.h | 4 ++++ 2 files changed, 43 insertions(+) -- 1.8.3.4