From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754638AbdKAM5e (ORCPT ); Wed, 1 Nov 2017 08:57:34 -0400 Received: from szxga04-in.huawei.com ([45.249.212.190]:9933 "EHLO szxga04-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751525AbdKAM5d (ORCPT ); Wed, 1 Nov 2017 08:57:33 -0400 Subject: Re: [PATCH 1/2] perf mmap: Fix perf backward recording To: Jiri Olsa References: <20171101055327.141281-1-wangnan0@huawei.com> <20171101055327.141281-2-wangnan0@huawei.com> <20171101094929.GB25146@danjae.aot.lge.com> <20171101120007.GA26623@danjae.aot.lge.com> <109f02ef-5dc2-94f9-e850-572c498781ee@huawei.com> <20171101123957.GA27215@krava> CC: Namhyung Kim , , , , From: "Wangnan (F)" Message-ID: <4eeabf43-4de0-467c-04b0-85217040b036@huawei.com> Date: Wed, 1 Nov 2017 20:56:32 +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: <20171101123957.GA27215@krava> 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.0A020206.59F9C4B1.0166,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: 0d161d64bdde9e4f8b274cbbea18d420 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017/11/1 20:39, Jiri Olsa wrote: > On Wed, Nov 01, 2017 at 08:10:49PM +0800, Wangnan (F) wrote: > > SNIP > >>>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c >>>>> index c6c891e154a6..27ebe355e794 100644 >>>>> --- a/tools/perf/util/evlist.c >>>>> +++ b/tools/perf/util/evlist.c >>>>> @@ -838,6 +838,11 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx, >>>>> if (*output == -1) { >>>>> *output = fd; >>>>> + if (evsel->attr.write_backward) >>>>> + mp->prot = PROT_READ; >>>>> + else >>>>> + mp->prot = PROT_READ | PROT_WRITE; >>>>> + >>>> If evlist->overwrite is true, PROT_WRITE should be unset even if >>>> write_backward is >>>> not set. If you want to delay the setting of mp->prot, you need to consider >>>> both evlist->overwrite and evsel->attr.write_backward. >>> I thought evsel->attr.write_backward should be set when >>> evlist->overwrite is set. Do you mean following case? >>> >>> perf record --overwrite -e 'cycles/no-overwrite/' >>> >> No. evlist->overwrite is unrelated to '--overwrite'. This is why I >> said the concept of 'overwrite' and 'backward' is ambiguous. >> >> perf record never sets 'evlist->overwrite'. What '--overwrite' actually >> does is setting write_backward. Some testcases needs overwrite evlist. > did not check deeply, but so why can't we do the below? > > jirka > > > --- > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index f4d9fc54b382..4643c487bd29 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -300,7 +300,7 @@ static int record__mmap_evlist(struct record *rec, > struct record_opts *opts = &rec->opts; > char msg[512]; > > - if (perf_evlist__mmap_ex(evlist, opts->mmap_pages, false, > + if (perf_evlist__mmap_ex(evlist, opts->mmap_pages, opts->overwrite, > opts->auxtrace_mmap_pages, > opts->auxtrace_snapshot_mode) < 0) { perf_evlist__mmap_ex's overwrite argument is used to control evlist->mmap. Consider Namhyung's example: perf record --overwrite -e 'cycles/no-overwrite/' In this case both evlist->mmap and evlist->backward_mmap would be set to overwrite. 'cycles' will be put into evlist->mmap, so it will be set to overwrite incorrectly. Patch 2/2 clarifies these concepts. What we want is the flight recorder mode, not only a read only ring buffer. Even flight recorder mode is selected, we can still have a normal ring buffer which keep output data. Thank you.