From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934960AbdJJUTz (ORCPT ); Tue, 10 Oct 2017 16:19:55 -0400 Received: from szxga05-in.huawei.com ([45.249.212.191]:7546 "EHLO szxga05-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934375AbdJJUTu (ORCPT ); Tue, 10 Oct 2017 16:19:50 -0400 Subject: Re: [PATCH 02/10] perf tool: fix: Don't discard prev in backward mode To: "Liang, Kan" , "acme@kernel.org" , "peterz@infradead.org" , "mingo@redhat.com" , "linux-kernel@vger.kernel.org" References: <1507656023-177125-1-git-send-email-kan.liang@intel.com> <1507656023-177125-3-git-send-email-kan.liang@intel.com> <37b6527e-cc03-db99-e30f-4652c06b5cb0@huawei.com> <37664610-cd3f-57e2-7a05-d0f3d81e8a9d@huawei.com> <37D7C6CF3E00A74B8858931C1DB2F077537D1BE6@SHSMSX103.ccr.corp.intel.com> CC: "jolsa@kernel.org" , "hekuang@huawei.com" , "namhyung@kernel.org" , "alexander.shishkin@linux.intel.com" , "Hunter, Adrian" , "ak@linux.intel.com" From: "Wangnan (F)" Message-ID: <50710aff-20c6-6c80-1ab0-05440b87e53d@huawei.com> Date: Wed, 11 Oct 2017 04:18:57 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <37D7C6CF3E00A74B8858931C1DB2F077537D1BE6@SHSMSX103.ccr.corp.intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.111.194.139] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090201.59DD2B43.002D,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 1e9e906a7da20ed1681dcd6e21d0d7d2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017/10/11 3:50, Liang, Kan wrote: >> On 2017/10/11 2:23, Wangnan (F) wrote: >>> >>> On 2017/10/11 1:20, kan.liang@intel.com wrote: >>>> From: Kan Liang >>>> >>>> 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 >>>> --- >>>> 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) >> > No. That's not what I want. > My test code never trigger the WARN_ONCE. The existing code never trigger that warning because the size computed by rb_find_range is never larger than size of ring buffer. After applying your patch, I believe it will trigger this WARN_ONCE and drop the whole ring buffer. Please set a smaller ring buffer and try again. > I think you will see the problem, if you simply run the command as below. > sudo ./perf record -e cycles:P -C0 --overwrite --switch-output=1s > > The output size keep increasing. Because the new output always include the old outputs. > What I want is the 'start' and 'end' for the increase, not everything. This is my test result: add a '-m 1' for 'perf record' for shrinking ring buffer, start a while loop on CPU 0 to increase data rate. It stops increasing after the ring buffer is full: $:~/linux/tools/perf$ sudo ./perf record -m1 -e cycles:P -C0 --overwrite --switch-output=1s Warning: File /home/w00229757/.perfconfig not owned by current user or root, ignoring it. [ perf record: dump data: Woken up 1 times ] [ perf record: Dump perf.data.2017101212165072 ] [ perf record: dump data: Woken up 1 times ] [ perf record: Dump perf.data.2017101212165175 ] [ perf record: dump data: Woken up 1 times ] [ perf record: Dump perf.data.2017101212165278 ] [ perf record: dump data: Woken up 1 times ] [ perf record: Dump perf.data.2017101212165381 ] [ perf record: dump data: Woken up 1 times ] [ perf record: Dump perf.data.2017101212165484 ] [ perf record: dump data: Woken up 1 times ] [ perf record: Dump perf.data.2017101212165586 ] ^C[ perf record: Woken up 1 times to write data ] [ perf record: Dump perf.data.2017101212165653 ] [ perf record: Captured and wrote 1.013 MB perf.data. ] $ ls -l ./perf.data* -rw------- 1 root root 538988 Oct 12 12:16 ./perf.data.2017101212165072 -rw------- 1 root root 538988 Oct 12 12:16 ./perf.data.2017101212165175 -rw------- 1 root root 538988 Oct 12 12:16 ./perf.data.2017101212165278 -rw------- 1 root root 538988 Oct 12 12:16 ./perf.data.2017101212165381 -rw------- 1 root root 538988 Oct 12 12:16 ./perf.data.2017101212165484 -rw------- 1 root root 538988 Oct 12 12:16 ./perf.data.2017101212165586 -rw------- 1 root root 1067812 Oct 12 12:16 ./perf.data.2017101212165653 You see the result keep getting larger because the ring buffer is never full in your case. Thank you.