From: Jiri Olsa <jolsa@redhat.com>
To: "Liang, Kan" <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>,
"Wangnan (F)" <wangnan0@huawei.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"acme@kernel.org" <acme@kernel.org>,
"kernel-team@lge.com" <kernel-team@lge.com>
Subject: Re: [PATCH 1/2] perf mmap: Fix perf backward recording
Date: Thu, 2 Nov 2017 15:59:11 +0100 [thread overview]
Message-ID: <20171102145911.GA19184@krava> (raw)
In-Reply-To: <37D7C6CF3E00A74B8858931C1DB2F077537DC909@SHSMSX103.ccr.corp.intel.com>
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
next prev parent reply other threads:[~2017-11-02 14:59 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-01 5:53 [PATCH 0/2] perf record: Fix --overwrite and clarify concepts Wang Nan
2017-11-01 5:53 ` [PATCH 1/2] perf mmap: Fix perf backward recording Wang Nan
2017-11-01 9:49 ` Namhyung Kim
2017-11-01 10:32 ` Wangnan (F)
2017-11-01 12:00 ` Namhyung Kim
2017-11-01 12:10 ` Wangnan (F)
2017-11-01 12:39 ` Jiri Olsa
2017-11-01 12:56 ` Wangnan (F)
2017-11-02 15:12 ` Jiri Olsa
2017-11-01 13:57 ` Liang, Kan
2017-11-01 16:12 ` Wangnan (F)
2017-11-01 16:22 ` Liang, Kan
2017-11-02 5:34 ` Namhyung Kim
2017-11-02 13:25 ` Liang, Kan
2017-11-02 14:59 ` Jiri Olsa [this message]
2017-11-01 5:53 ` [PATCH 2/2] perf record: Replace 'overwrite' by 'flightrecorder' for better naming Wang Nan
2017-11-01 10:03 ` Namhyung Kim
2017-11-01 10:17 ` Wangnan (F)
2017-11-01 12:03 ` Namhyung Kim
2017-11-01 13:26 ` Liang, Kan
2017-11-01 14:05 ` Wangnan (F)
2017-11-01 14:22 ` Liang, Kan
2017-11-01 14:44 ` Wangnan (F)
2017-11-01 15:04 ` Liang, Kan
2017-11-01 16:00 ` Wangnan (F)
2017-11-01 16:13 ` Liang, Kan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171102145911.GA19184@krava \
--to=jolsa@redhat.com \
--cc=acme@kernel.org \
--cc=kan.liang@intel.com \
--cc=kernel-team@lge.com \
--cc=linux-kernel@vger.kernel.org \
--cc=namhyung@kernel.org \
--cc=wangnan0@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).