From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933676AbdKBO7Q (ORCPT ); Thu, 2 Nov 2017 10:59:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56558 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933448AbdKBO7P (ORCPT ); Thu, 2 Nov 2017 10:59:15 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5CEB781DEB Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=jolsa@redhat.com Date: Thu, 2 Nov 2017 15:59:11 +0100 From: Jiri Olsa To: "Liang, Kan" Cc: Namhyung Kim , "Wangnan (F)" , "linux-kernel@vger.kernel.org" , "acme@kernel.org" , "kernel-team@lge.com" Subject: Re: [PATCH 1/2] perf mmap: Fix perf backward recording Message-ID: <20171102145911.GA19184@krava> References: <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> <37D7C6CF3E00A74B8858931C1DB2F077537DC1FA@SHSMSX103.ccr.corp.intel.com> <226626ec-4f96-d3f4-a6d5-62c17c897f32@huawei.com> <37D7C6CF3E00A74B8858931C1DB2F077537DC3EF@SHSMSX103.ccr.corp.intel.com> <20171102053405.GA23303@sejong> <37D7C6CF3E00A74B8858931C1DB2F077537DC909@SHSMSX103.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <37D7C6CF3E00A74B8858931C1DB2F077537DC909@SHSMSX103.ccr.corp.intel.com> User-Agent: Mutt/1.9.1 (2017-09-22) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Thu, 02 Nov 2017 14:59:15 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Nov 02, 2017 at 01:25:08PM +0000, Liang, Kan wrote: > Hi Namhyung, > > > On Wed, Nov 01, 2017 at 04:22:53PM +0000, Liang, Kan wrote: > > > > On 2017/11/1 21:57, Liang, Kan wrote: > > > > >> On 2017/11/1 20:00, Namhyung Kim wrote: > > > > >>> On Wed, Nov 01, 2017 at 06:32:50PM +0800, Wangnan (F) wrote: > > > > > There are only four test cases which set overwrite, > > > > > sw-clock,task-exit, mmap-basic, backward-ring-buffer. > > > > > Only backward-ring-buffer is 'backward overwrite'. > > > > > The rest three are all 'forward overwrite'. We just need to set > > > > > write_backward to convert them to 'backward overwrite'. > > > > > I think it's not hard to clean up. > > > > > > > > If we add a new rule that overwrite ring buffers are always backward > > > > then it is not hard to cleanup. However, the support of forward > > > > overwrite ring buffer has a long history and the code is not written > > > > by me. I'd like to check if there is some reason to keep support this > > configuration? > > > > > > > > > > As my observation, currently, there are no perf tools which support > > > 'forward overwrite'. > > > There are only three test cases (sw-clock, task-exit, mmap-basic) > > > which is set to 'forward overwrite'. I don’t see any reason it cannot > > > be changed to 'backward overwrite' > > > > > > Arnaldo? Jirka? Kim? > > > > > > What do you think? > > > > I think sw-clock, task-exit and mmap-basic test cases can be changed to use > > the forward non-overwrite mode. agreed, we can change them to forward non-overwrite mode > > The forward overwrite might be used by externel applications accessing the > > ring buffer directly but not needed for perf tools IMHO. > > The proposal is only for perf tool, not kernel. So external applications can still > use forward overwrite to access the ring buffer. > > > Let's keep the code simpler as much as possible. > > Agree. > Now, there are too many options to access the ring buffer. Not all of them are > supported. > I think we should only keep the crucial options (overwrite/non-overwrite), clearly > define them in the document and cleanup the code. as you said earlier only 2 modes make sense, so I think perf record should have: - forward non-overwrite mode by default - backward overwrite mode when '--overwrite' option is set and make it clear in the docs, maybe in special perf-record man page section so far I still like the '--overwrite' option more than --flight-recorder' ;-) also it's been out there for some time now jirka