On Mon, Aug 26, 2019 at 07:47:35AM -0700, kan.li...@linux.intel.com wrote:
> Groups
> ======
>
> To avoid reading the METRICS register multiple times, the metrics and
> slots value can only be updated by the first slots/metrics event in a
> group. All active slots and metrics events will be updated one time.
Can't we require SLOTS to be the group leader for any Metric group?
Is there ever a case where we want to add other events to a metric
group?
> Reset
> ======
>
> The PERF_METRICS and Fixed counter 3 have to be reset for each read,
> because:
> - The 8bit metrics ratio values lose precision when the measurement
> period gets longer.
So it musn't be too hot,
> - The PERF_METRICS may report wrong value if its delta was less than
> 1/255 of SLOTS (Fixed counter 3).
it also musn't be too cold. But that leaves it unspecified what exactly
is the right range.
IOW, you want a Goldilocks number of SLOTS.
> Also, for counting, the -max_period is the initial value of the SLOTS.
> The huge initial value will definitely trigger the issue mentioned
> above. Force initial value as 0 for topdown and slots event counting.
But you just told us that 0 is wrong too (too cold).
I'm still confused by all this; when exactly does:
> NMI
> ======
>
> The METRICS register may be overflow. The bit 48 of STATUS register
> will be set. If so, update all active slots and metrics events.
that happen? It would be useful to get that METRIC_OVF (can we please
start naming them; 62,55,48 is past silly) at the exact point
where PERF_METRICS is saturated.
If this is so; then we can use this to update/reset PERF_METRICS and
nothing else.
That is; leave the SLOTS programming alone; use -max_period as usual.
Then on METRIC_OVF, read PERF_METRICS and clear it, and update all the
metric events by adding slots_delta * frac / 256 -- where slots_delta is
the SLOTS count since the last METRIC_OVF.
On read; read PERF_METRICS -- BUT DO NOT RESET -- and compute an
intermediate delta and add that to whatever stable count we had.
Maybe something like:
do {
count1 = local64_read(&event->count);
barrier();
metrics = read_perf_metrics();
barrier();
count2 = local64_read(event->count);
} while (count1 != count2);
/* no METRIC_OVF happened and {count,metrics} is consistent */
return count1 + (slots_delta * frac / 256);
> The update_topdown_event() has to read two registers separately. The
> values may be modify by a NMI. PMU has to be disabled before calling the
> function.
Then there is no mucking about with that odd counter/metrics msr pair
reset nonsense. Becuase that really stinks.
