Message-ID: <37D7C6CF3E00A74B8858931C1DB2F07701941C44@shsmsx102.ccr.corp.intel.com>
Date:	Fri, 25 Sep 2015 14:57:14 +0000
From:	"Liang, Kan" <kan.liang@...el.com>
To:	Jiri Olsa <jolsa@...hat.com>
CC:	"acme@...nel.org" <acme@...nel.org>,
	"jolsa@...nel.org" <jolsa@...nel.org>,
	"a.p.zijlstra@...llo.nl" <a.p.zijlstra@...llo.nl>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"namhyung@...nel.org" <namhyung@...nel.org>,
	"ak@...ux.intel.com" <ak@...ux.intel.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH RFC 00/10] counter read during perf sampling


> On Thu, Sep 24, 2015 at 07:47:40PM +0000, Liang, Kan wrote:
> >
> >
> > > On Tue, Sep 22, 2015 at 10:13:33AM -0400, kan.liang@...el.com wrote:
> > > > From: Kan Liang <kan.liang@...el.com>
> > > >
> > > > The patch series intends to read counter statistics with fixed
> > > > frequency during sampling. The instant benefit is that we can read
> > > > memory bandwidth from uncore event during cpu PMU event is
> > > sampling.
> > > >
> > > > Introduce 'C' event/group modifier. The event with this modifier
> > > > will do counting not sampling. If a group with this modifier, only
> > > > group leader do sampling. The counter statistics will be wrote in
> > > > new RECORD type PERF_RECORD_COUNTER_READ and stored in
> perf.data.
> > > > So perf report can present the counter statistics data accordingly.
> > > >
> > > > There may be an alternative way to get counter statistics during
> > > > sampling by running perf record and perf stat together by script.
> > > > But the script way have various issue and complex to parses the
> > > > output.
> > >
> > > just a thought, but isn't the way then llow to store the data from
> > > perf stat? ;-) and be able to merge perf.data-s from perf record and
> > > stat afterwards
> >
> > Yes, the way to store the data from perf stat is better than pure
> > script way. I guess your patch "perf stat record" can do that, right?
> >
> > If so, how should we run perf record and stat in parallel? By scripts
> > or modify perf record/stat?
> >
> > Also, we need an option in perf report to merge the perf.data-s. Right?
> 
> either that or extra step with 'perf data merge' or somthing like that
>

Any update about "perf stat record" patch set? That will help a lot, if
we finally choose the 'perf data merge' way. Right?  
 
> SNIP
> 
> > > but if we go this way I think we should keep/allow all the options
> > > perf stat
> >
> > Do you mean something like "perf record stat"?
> > That's not the way I designed. I don't want to run perf record and
> > perf stat together in one command.
> >
> > I just want to do similar thing like what sample read did. Sample read
> > can read counters on each sample. While the counter read can read
> > counters in a fix
> 
> yea, but sample read stores data into ring buffer in the kernel while you
> read the data like in perf stat
> 
> > frequency (set by --counter-read-interval). So it's an extension of perf
> record.
> > It applies all possible options of perf record, like -C -a -g...
> > I introduce a new option --counter-read-interval is because that there
> > is no  interval options in perf record.
> 
> the way I see it you implemented 'perf stat' logic within record command
> you create counter (non sampling) and read it via read syscall
> 
> I think it's good idea, but I think we should follow the way we do in perf
> stat command and reuse the interface (and code)
> 
> like having the 'stat' keyword separating the non-sampling config:
> 
>   $ perf record -e cycles stat -e 'uncore_imc_1/cas_count_read/' -I 10000 -
>  ./tchain_edit
>

Another thing is that there is limitation for --interval-print in perf stat.
The interval must >= 100ms. However, we need the interval >=10ms.

Any idea about where 100ms is from? Print limit?  
 
If we choose this way, I think we need to introduce a new option for perf
stat to break up the limitation. 

> just an idea.. but I dont think the :C modifier is a good way
>
> SNIP
> 
> > > I think we'll need special output/display for non sampling events,
> > > something like extra window in TUI and distinguished output in
> > > stdio, the above is hacked sampling output ;-)
> >
> > I think it depends on what way we finally use.
> >
> > If we use the way which merging perf.data from perf stat and record, I
> > think we need special output for the data from perf stat in TUI/stdio.
> >
> > But if we use the way counter read (:C), I think the best place to
> > show the counter read results is the header/title (just like the patch
> > did). Because the results are the aggregate counts during the whole
> sampling process.
> > Something like,
> > # Event count: 35937 of event 'uncore_imc_1/cas_count_read/C'
> 
> hum, how the --counter-read-interval data displayed then? it's not single
> number right?
>
No matter which way we choose, I think the output should be similar.

As my original design, perf only output every --counter-read-interval data
in perf report -D.
For tui and stdio, it only output the aggregate number. So, yes, single number.

I think it should be enough. In tui/stdio, perf gives user a roughly image by
the total number during the whole sampling process. If they want details,
they can check by report -D.
Considering the interval is only 10ms, if perf output everything in tui/stdio,
the output is too huge.

Thanks,
Kan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists