On Tue, 29 Mar 2011, Benjamin Herrenschmidt wrote:
> On Fri, 2011-03-25 at 09:28 -0400, Eric B Munson wrote:
> > It is possible on POWER7 for some perf events to have values decrease. This
> > causes a problem with the way the kernel counters are updated. Deltas are
> > computed and then stored in a 64 bit value while the registers are 32 bits
> > wide so if new value is smaller than previous value, the delta is a very
> > large positive value. As a work around this patch skips updating the kernel
> > counter in when the new value is smaller than the previous. This can lead
> > to
> > a lack of precision in the coutner values, but from my testing the value is
> > typcially fewer than 10 samples at a time.
>
> Unfortunately the patch isn't 100% correct I believe:
>
> I think you don't deal with the rollover of the counters. The new value
> could be smaller than the previous one simply because the counter just
> rolled over.
>
> In cases like this:
>
> > @@ -449,8 +458,10 @@ static void freeze_limited_counters(struct
> > cpu_hw_events *cpuhw,
> > val = (event->hw.idx == 5) ? pmc5 : pmc6;
> > prev = local64_read(&event->hw.prev_count);
> > event->hw.idx = 0;
> > - delta = (val - prev) & 0xfffffffful;
> > - local64_add(delta, &event->count);
> > + if (val >= prev) {
> > + delta = (val - prev) & 0xfffffffful;
> > + local64_add(delta, &event->count);
> > + }
> > }
> > }
>
> I wonder if it isn't easier to just define delta to be a s32, get rid
> of the mask and test if delta is positive, something like:
>
> delta = val - prev;
> if (delta > 0)
> local64_add(delta, &event->count);
>
> Wouldn't that be simpler ? Or do I miss a reason why it wouldn't work ?Here I made the assumption that the hardware would never remove more events in a speculative roll back than it had added. This is not a situation I encoutered in my limited testing, so I didn't think underflow was possible. I will send out a V2 using the signed 32 bit delta and remeber to CC stable this time. Eric
signature.asc
Description: Digital signature
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
