[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 10 Oct 2017 16:00:14 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Wang Nan <wangnan0@...wei.com>
Cc: "Liang, Kan" <kan.liang@...el.com>,
"peterz@...radead.org" <peterz@...radead.org>,
"mingo@...hat.com" <mingo@...hat.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"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 03/10] perf tool: new iterfaces to read event from ring
buffer
Em Tue, Oct 10, 2017 at 03:36:28PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Oct 10, 2017 at 03:34:55PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Tue, Oct 10, 2017 at 06:28:18PM +0000, Liang, Kan escreveu:
> > > > Em Tue, Oct 10, 2017 at 10:20:16AM -0700, kan.liang@...el.com escreveu:
> > > > > From: Kan Liang <kan.liang@...el.com>
> > > > >
> > > > > The perf_evlist__mmap_read only support forward mode. It needs a
> > > > > common function to support both forward and backward mode.
> > > >
> > > > > The perf_evlist__mmap_read_backward is buggy.
> > > >
> > > > So, what is the bug? You state that it is buggy, but don't spell out the bug,
> > > > please do so.
> > > >
> > >
> > > union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, int idx)
> > > {
> > > struct perf_mmap *md = &evlist->mmap[idx]; <--- it should be backward_mmap
> > >
> > > > If it fixes an existing bug, then it should go separate from this patchkit, right?
> > >
> > > There is no one use perf_evlist__mmap_read_backward. So it doesn't trigger any issue.
> >
> > There is no one at the end of your patchkit? Or no user _right now_? If
> > there is a user now, lemme see... yeah, no user right now, so _that_ is
> > yet another bug, i.e. it should be used, no? If this is just a left
> > over, then we should just throw it away, now, its a cleanup.
>
> Wang, can you take a look at these two issues?
So it looks leftover that should've been removed by the following cset, right Wang?
- Arnaldo
commit a0c6f451f90204847ce5f91c3268d83a76bde1b6
Author: Wang Nan <wangnan0@...wei.com>
Date: Thu Jul 14 08:34:41 2016 +0000
perf evlist: Drop evlist->backward
Now there's no real user of evlist->backward. Drop it. We are going to
use evlist->backward_mmap as a container for backward ring buffer.
Powered by blists - more mailing lists