Re: [PATCH 4/9] perf stat: Remove transaction_run from shadow update/print code

From: Arnaldo Carvalho de Melo
Date: Wed Jun 03 2015 - 11:56:07 EST


Em Wed, Jun 03, 2015 at 05:14:50PM +0200, Jiri Olsa escreveu:
> On Wed, Jun 03, 2015 at 12:10:38PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Jun 03, 2015 at 04:25:54PM +0200, Jiri Olsa escreveu:
> > > It's no longer needed, because we use nameid to recognize
> > > transaction events.
> > >
> > > Keeping it only in stat code to initialize transaction events.
> >
> > Lemme see if I understand this correctly, this update_shadow_stats() is
> > called only for transaction runs?
> >
> > It doesn't seem so, so now we will update those stats all the time?
>
> update_shadow_stats is called on currently read/processed
> counter and based on its 'type' we update various other
> counters - shadow counters - which are the base for things
> like IPC in the stat output


So:

- else if (transaction_run && perf_stat_evsel__is(counter, CYCLES_IN_TX))
+ else if (perf_stat_evsel__is(counter, CYCLES_IN_TX))
update_stats(&runtime_transaction_stats[ctx][cpu], count[0]);


Before we were going to check this _only_ when transaction_run was set,
now we are doing it always, or are you saying that
perf_stat_evsel__is(counter, CYCLES_IN_TX) will only return true for
transaction 'perf stat' usage?

Let me take a look:

Yes, in all cases we, start doing:

perf_evlist__alloc_stats()
perf_evsel__alloc_stat_priv()
perf_evsel__reset_stat_priv()
perf_stat_evsel_id_init();
struct perf_stat *ps = evsel->priv;
ps->id = one of the entries below:

static const char *id_str[PERF_STAT_EVSEL_ID__MAX] = {
ID(NONE, x),
ID(CYCLES_IN_TX, cpu/cycles-t/),
ID(TRANSACTION_START, cpu/tx-start/),
ID(ELISION_START, cpu/el-start/),
ID(CYCLES_IN_TX_CP, cpu/cycles-ct/),
};


Ok, guess I understood now, all the events above are transaction events,
so if one of them is present, then, yes, this is a transaction run, and
we don't need to look at the 'transaction_run' variable to update the
shadow stats, got it right?

I.e. we _never_ needed to look at 'transaction_id', just noticing that
ps->id is not zero, ok.

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