lkml.org 
[lkml]   [2017]   [Oct]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 02/10] perf tool: fix: Don't discard prev in backward mode
From
Date


On 2017/10/11 2:23, Wangnan (F) wrote:
>
>
> On 2017/10/11 1:20, kan.liang@intel.com wrote:
>> From: Kan Liang <kan.liang@intel.com>
>>
>> Perf record can switch output. The new output should only store the data
>> after switching. However, in overwrite backward mode, the new output
>> still have the data from old output.
>>
>> At the end of mmap_read, the position of processed ring buffer is saved
>> in md->prev. Next mmap_read should be end in md->prev.
>> However, the md->prev is discarded. So next mmap_read has to process
>> whole valid ring buffer, which definitely include the old processed
>> data.
>>
>> Set the prev as the end of the range in backward mode.
>>
>> Signed-off-by: Kan Liang <kan.liang@intel.com>
>> ---
>> tools/perf/util/evlist.c | 14 +++++++++++++-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index 33b8837..7d23cf5 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -742,13 +742,25 @@ static int
>> rb_find_range(void *data, int mask, u64 head, u64 old,
>> u64 *start, u64 *end, bool backward)
>> {
>> + int ret;
>> +
>> if (!backward) {
>> *start = old;
>> *end = head;
>> return 0;
>> }
>> - return backward_rb_find_range(data, mask, head, start, end);
>> + ret = backward_rb_find_range(data, mask, head, start, end);
>> +
>> + /*
>> + * The start and end from backward_rb_find_range is the range
>> for all
>> + * valid data in ring buffer.
>> + * However, part of the data is processed previously.
>> + * Reset the end to drop the processed data
>> + */
>> + *end = old;
>> +

[SNIP]

>
> If you really want to avoid record duplication, you need to changes
> record__mmap_read()'s
> logic. Now it complains "failed to keep up with mmap data" and avoid
> dumping data when
> size of newly generated data is larger than the size of the ring
> buffer. It is reasonable
> for forward ring buffer because in this case you lost the head of the
> first record, the
> whole ring buffer is unparseable. However, it is wrong in backward
> case. What you
> should do in this case is dumping the whole ring buffer.
>

I think what you want should be something like this: (not tested)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index ee7d0a8..f621a8e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -173,7 +173,9 @@ rb_find_range(void *data, int mask, u64 head, u64 old,
return 0;
}

- return backward_rb_find_range(data, mask, head, start, end);
+ *start = head;
+ *end = old;
+ return 0;
}

static int
@@ -199,10 +201,15 @@ record__mmap_read(struct record *rec, struct
perf_mmap *md,

size = end - start;
if (size > (unsigned long)(md->mask) + 1) {
- WARN_ONCE(1, "failed to keep up with mmap data. (warn only
once)\n");
+ if (!backward) {
+ WARN_ONCE(1, "failed to keep up with mmap data. (warn only
once)\n");

- md->prev = head;
- perf_mmap__consume(md, overwrite || backward);
+ md->prev = head;
+ perf_mmap__consume(md, overwrite || backward);
+ } else {
+ backward_rb_find_range(data, md->mask, head, start, end);
+ /* FIXME: error processing */
+ }
return 0;
}

Use 'head' and 'old' to locate data position in ring buffer by default.
If overwrite happen, use
backward_rb_find_range() to fetch the last available record and dump the
whole ring buffer.

Thank you.


\
 
 \ /
  Last update: 2017-10-10 20:52    [W:1.837 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site