linux-kernel - RE: [PATCH 02/10] perf tool: fix: Don't discard prev in backward mode

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 12 Oct 2017 12:49:09 +0000
From:   "Liang, Kan" <kan.liang@...el.com>
To:     "Wangnan (F)" <wangnan0@...wei.com>,
        "'acme@...nel.org'" <acme@...nel.org>,
        "'peterz@...radead.org'" <peterz@...radead.org>,
        "'mingo@...hat.com'" <mingo@...hat.com>,
        "'linux-kernel@...r.kernel.org'" <linux-kernel@...r.kernel.org>
CC:     "'jolsa@...nel.org'" <jolsa@...nel.org>,
        "'hekuang@...wei.com'" <hekuang@...wei.com>,
        "'namhyung@...nel.org'" <namhyung@...nel.org>,
        "'alexander.shishkin@...ux.intel.com'" 
        <alexander.shishkin@...ux.intel.com>,
        "Hunter, Adrian" <adrian.hunter@...el.com>,
        "'ak@...ux.intel.com'" <ak@...ux.intel.com>
Subject: RE: [PATCH 02/10] perf tool: fix: Don't discard prev in backward
 mode

> >  From 8b058ea6977a97e5705aa2f64bdd014fd76d1247 Mon Sep 17
> 00:00:00
> > 2001
> > From: Kan Liang <Kan.liang@...el.com>
> > Date: Wed, 11 Oct 2017 07:39:34 -0700
> > Subject: [PATCH] perf tool: fix: Don't discard prev in backward mode
> >
> > Perf record can switch output. The new output should only store the
> > data after switching. However, in overwrite backward mode, the new
> > output still have the data from old output. That also brings extra overhead.
> >
> > At the end of mmap_read, the position of processed ring buffer is
> > saved in md->prev. Next mmap_read should be end in md->prev if it is
> > not overwriten. That avoids to process duplicate data.
> > However, the md->prev is discarded. So next mmap_read has to process
> > whole valid ring buffer, which probably include the old processed
> > data.
> >
> > Introduce fast path for backward_rb_find_range. Stop searching when
> > md->prev is detected.
> >
> > Signed-off-by: Kan Liang <Kan.liang@...el.com>
> > ---
> >   tools/perf/util/mmap.c | 11 +++++++++--
> >   1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c index
> > 9fe5f9c..36b459a 100644
> > --- a/tools/perf/util/mmap.c
> > +++ b/tools/perf/util/mmap.c
> > @@ -254,7 +254,8 @@ int perf_mmap__mmap(struct perf_mmap *map,
> struct mmap_params *mp, int fd)
> >   	return 0;
> >   }
> >
> > -static int backward_rb_find_range(void *buf, int mask, u64 head, u64
> > *start, u64 *end)
> > +static int backward_rb_find_range(void *buf, int mask, u64 head,
> > +				  u64 old, u64 *start, u64 *end)
> >   {
> >   	struct perf_event_header *pheader;
> >   	u64 evt_head = head;
> > @@ -282,6 +283,12 @@ static int backward_rb_find_range(void *buf, int
> > mask, u64 head, u64 *start, u64
> >
> >   		evt_head += pheader->size;
> >   		pr_debug3("move evt_head: %"PRIx64"\n", evt_head);
> > +
> > +		/* fast path: avoid to process duplicate data */
> > +		if (old == evt_head) {
> > +			*end = evt_head;
> > +			return 0;
> > +		}
> 
> You still need to parse the whole ring buffer.

I don't think so. I'm not using (old & mask).
The upper bit of old is good enough to tell if there is overwriting.

Here are some debugging logs.
The start and end is the result from backward_rb_find_range.
If there is overwriting, it finishes with 'rewind', not 'fast path'.

sudo ./perf record -m1 -e cycles:P -C0 --overwrite --switch-output=1s
backward_rb_find_range: buf=0x7f9ea7a93000, head=fffffffffffff550
Finished reading backward ring buffer: fast path
start   0xfffffffffffff550 end 0x0

backward_rb_find_range: buf=0x7f9ea7a93000, head=ffffffffffffab30
Finished reading backward ring buffer: rewind
start   0xffffffffffffab30 end 0xffffffffffffbb10

backward_rb_find_range: buf=0x7f9ea7a93000, head=ffffffffffff28b0
Finished reading backward ring buffer: rewind
start   0xffffffffffff28b0 end 0xffffffffffff3898

backward_rb_find_range: buf=0x7f9ea7a93000, head=fffffffffffe3e40
Finished reading backward ring buffer: rewind
start   0xfffffffffffe3e40 end 0xfffffffffffe4e40

backward_rb_find_range: buf=0x7f9ea7a93000, head=fffffffffffe3aa0
Finished reading backward ring buffer: fast path
start   0xfffffffffffe3aa0 end 0xfffffffffffe3e40

backward_rb_find_range: buf=0x7f9ea7a93000, head=fffffffffffe3470
Finished reading backward ring buffer: fast path
start   0xfffffffffffe3470 end 0xfffffffffffe3aa0

backward_rb_find_range: buf=0x7f9ea7a93000, head=fffffffffffe0610
Finished reading backward ring buffer: rewind
start   0xfffffffffffe0610 end 0xfffffffffffe1610

backward_rb_find_range: buf=0x7f9ea7a93000, head=fffffffffffe00d0
Finished reading backward ring buffer: fast path
start   0xfffffffffffe00d0 end 0xfffffffffffe0610

backward_rb_find_range: buf=0x7f9ea7a93000, head=fffffffffffdfe90
Finished reading backward ring buffer: fast path
start   0xfffffffffffdfe90 end 0xfffffffffffe00d0

Thanks,
Kan

> You can use 'old - head' to check how many bytes are written into the ring
> buffer since you last read. If its size is less than the ring buffer size, there's no
> overwriting happen. In this case a simple copy should be enough.
> 
> Thank you.