linux-kernel - Re: [PATCH 1/2] perf mmap: Fix perf backward recording

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 2 Nov 2017 15:59:11 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     "Liang, Kan" <kan.liang@...el.com>
Cc:     Namhyung Kim <namhyung@...nel.org>,
        "Wangnan (F)" <wangnan0@...wei.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "acme@...nel.org" <acme@...nel.org>,
        "kernel-team@....com" <kernel-team@....com>
Subject: Re: [PATCH 1/2] perf mmap: Fix perf backward recording

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