2010/9/22 Matt Fleming <matt@xxxxxxxxxxxxxxxxx>: > I'm probably just being stupid but I can't figure out what this snippet > of code is doing. You're checking to see if the counter has overflowed, > and if it has then you just clear the overflow bit? I would have > expected you to reset the counter to 0. [DC]: Maybe my original code comment for the member msbs of the struct cpu_hw_events is too simple. And here is more: Unlike the perf counters in some other architectures, a 32bit MIPS perf counter, for example, will generate an interrupt after 0x7fffffff. But we want the operation to look like: 0x0 -> 0xffffffff -> interrupt. So there's a "pseudo" signal halfway. Please also note that the counter value will be brought back to 0 soon after it reaches 0x80000000 *each time*. > Shouldn't you also clear the overflow bit in ->msbs here? [DC]: See my comment above. > Having conditional code like this is a pretty sure sign that you haven't > separated support for the various performance hardware properly. Have > you had a look at how SH uses a registration interface to register > sh_pmus? Ideally all the internals for each type of perfcounter > hardware should be in their own file. [DC]: It does look ugly. It should be easy to put conditional code into perf_event_[mipsxx|loongson2|rm9000].c. I'll post a patch to fix it when the whole thing is accepted. > SH also has this problem that it doesn't have any sort of performance > counter interrupt and so can't check for overflow. This lack of > interrupt really needs to be solved generically in perf as it's a > problem that effects quite a few architectures. I suspect that using a > hrtimer instead of piggy-backing the timer interrupt would make the core > perf guys happier. Writing support for this is on my ever-growing list > of things todo. I've already started on some patches for the perf tool > so that it's possible to sample counters even though there is no > periodic interrupt (but that's a different problem). [DC]: Please add me into your post list when your patches are ready :-) Finally, thanks for your time to review the code. Deng-Cheng
