From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753054AbdJLBGO (ORCPT ); Wed, 11 Oct 2017 21:06:14 -0400 Received: from szxga04-in.huawei.com ([45.249.212.190]:7974 "EHLO szxga04-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751827AbdJLBGN (ORCPT ); Wed, 11 Oct 2017 21:06:13 -0400 Subject: Re: [PATCH] perf tools: fix: Force backward ring buffer mapped readonly To: "Liang, Kan" , "linux-kernel@vger.kernel.org" , "acme@kernel.org" References: <20171012140052.5658-1-wangnan0@huawei.com> <37D7C6CF3E00A74B8858931C1DB2F077537D214F@SHSMSX103.ccr.corp.intel.com> CC: Peter Zijlstra , Ingo Molnar , Jiri Olsa , Namhyung Kim , "Alexander Shishkin" , "Hunter, Adrian" , Andi Kleen , Li Zefan From: "Wangnan (F)" Message-ID: <398e9843-bcae-4825-f7ca-5d1f92617cb0@huawei.com> Date: Thu, 12 Oct 2017 09:03:25 +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: <37D7C6CF3E00A74B8858931C1DB2F077537D214F@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.0A0B0204.59DEBFD8.008E,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: 669ead51551d4b5a5e0bb4556330ce6a Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017/10/11 21:16, Liang, Kan wrote: >> perf record's --overwrite option doesn't work as we expect. >> For example: [SNIP] >> >> In the above example we get same records from the backward ring buffer >> all the time. Overwriting is not triggered. >> >> This commit maps backward ring buffers readonly, make it overwritable. >> It is safe because we assume backward ring buffer always overwritable >> in other part of code. >> >> After applying this patch: >> >> $ ~/linux/tools/perf$ sudo ./perf record -m 4 -e raw_syscalls:* -g -- >> overwrite \ >> --switch-output=1s --tail-synthesize dd >> if=/dev/zero of=/dev/null [SNIP] >> >> >> Signed-off-by: Wang Nan >> Cc: Liang Kan >> Cc: Arnaldo Carvalho de Melo >> Cc: Peter Zijlstra >> Cc: Ingo Molnar >> Cc: Jiri Olsa >> Cc: Namhyung Kim >> Cc: Alexander Shishkin >> Cc: Adrian Hunter >> Cc: Andi Kleen >> Cc: Li Zefan >> --- >> tools/perf/util/evlist.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c >> index c6c891e..a86b0d2 100644 >> --- a/tools/perf/util/evlist.c >> +++ b/tools/perf/util/evlist.c >> @@ -799,12 +799,14 @@ perf_evlist__should_poll(struct perf_evlist *evlist >> __maybe_unused, >> } >> >> static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx, >> - struct mmap_params *mp, int cpu_idx, >> + struct mmap_params *_mp, int cpu_idx, >> int thread, int *_output, int >> *_output_backward) >> { >> struct perf_evsel *evsel; >> int revent; >> int evlist_cpu = cpu_map__cpu(evlist->cpus, cpu_idx); >> + struct mmap_params *mp = _mp; >> + struct mmap_params backward_mp; >> >> evlist__for_each_entry(evlist, evsel) { >> struct perf_mmap *maps = evlist->mmap; >> @@ -815,6 +817,9 @@ static int perf_evlist__mmap_per_evsel(struct >> perf_evlist *evlist, int idx, >> if (evsel->attr.write_backward) { >> output = _output_backward; >> maps = evlist->backward_mmap; >> + backward_mp = *mp; >> + backward_mp.prot &= ~PROT_WRITE; >> + mp = &backward_mp; >> >> if (!maps) { >> maps = perf_evlist__alloc_mmap(evlist); > So it's trying to support per-event overwrite. > How about the global --overwrite option? Not only the per-event overwrite. See the example above. The overall --overwrite option is also respected. In perf_evsel__config, per-event evsel 'backward' setting is set based on overall '--overwrite' and per-event '/overwrite/' setting. > I think we should use opts->overwrite to replace the hard code 'false' for > perf_evlist__mmap_ex as well. > > Thanks, > Kan