Messages in this thread | | | Subject | Re: [RFC][PATCH 00/11] perf pmu interface -v2 | From | Peter Zijlstra <> | Date | Fri, 02 Jul 2010 11:52:03 +0200 |
| |
On Fri, 2010-07-02 at 11:57 +0900, Paul Mundt wrote: > At the moment it's not an issue since we have big enough counters that > overflows don't really happen, especially if we're primarily using them > for one-shot measuring. > > SH-4A style counters behave in such a fashion that we have 2 general > purpose counters, and 2 counters for measuring bus transactions. These > bus counters can optionally be disabled and used in a chained mode to > provide the general purpose counters a 64-bit counter (the actual > validity in the upper half of the chained counter varies depending on the > CPUs, but all of them can do at least 48-bits when chained).
Right, so I was reading some of that code and I couldn't actually find where you keep consistency between the hardware counter value and the stored prev_count value.
That is, suppose I'm counting, the hardware starts at 0, hwc->prev_count = 0 and event->count = 0.
At some point, x we context switch this task away, so we ->disable(), which disables the counter and updates the values, so at that time hwc->prev = x and event->count = x, right?
Now suppose we schedule the task back in, so we do ->enable(), then what happens? sh_pmu_enable() finds an unused index, (disables it for some reason.. it should already be cleared if its not used, but I guess a few extra hardware writes dont hurt) and calls sh4a_pmu_enable() on it.
sh4a_pmu_enable() does 3 writes:
PPC_PMCAT -- does this clear the counter value? PPC_CCBR -- writes the ->config bits PPC_CCBR (adds CCBR_DUC, couldn't this be done in the previous write to this reg?)
Now assuming that enable does indeed clear the hardware counter value, shouldn't you also set hwc->prev_count to 0 again? Otherwise the next update will see a massive jump?
Alternatively you could write the hwc->prev_count value back to the register.
If you eventually want to drop the chained counter support I guess it would make sense to have sh_perf_event_update() read and clear the counter so that you're always 0 based and then enforce an update from the arch tick hander so you never overflow.
| |