Messages in this thread | | | Date | Mon, 20 Feb 2012 11:19:02 +0800 | Subject | Re: oprofile and ARM A9 hardware counter | From | Ming Lei <> |
| |
Hi,
On Fri, Feb 17, 2012 at 6:26 PM, Will Deacon <will.deacon@arm.com> wrote: > On Fri, Feb 17, 2012 at 05:24:02AM +0000, Ming Lei wrote: >> On Fri, Feb 17, 2012 at 2:08 AM, Will Deacon <will.deacon@arm.com> wrote: >> >> > >> > The more I think about this, the more I think that the overflow parameter to >> > armpmu_event_update needs to go. It was introduced to prevent massive event >> > loss in non-sampling mode, but I think we can get around that by changing >> > the default sample_period to be half of the max_period, therefore giving >> > ourselves a much better chance of handling the interrupt before new wraps >> > around past prev. >> > >> > Ming Lei - can you try the following please? If it works for you, then I'll >> > do it properly and kill the overflow parameter altogether. >> >> Of course, it does work for the problem reported by Stephane since >> it changes the delta computation basically as I did, but I am afraid that >> it may be not good enough for the issue fixed in a737823d ("ARM: 6835/1: >> perf: ensure overflows aren't missed due to IRQ latency"). > > I think it does solve that problem. > >> > >> > if (!hwc->sample_period) { >> > - hwc->sample_period = armpmu->max_period; >> > + hwc->sample_period = armpmu->max_period >> 1; >> >> If you assume that the issue addressed by a737823d can only happen in >> non-sample situation, Peter's idea of u32 cast is OK and maybe simpler. > > I don't want to assume that the counters are 32-bit in this code as we may > want to plug in some other PMU with 16-bit counters, for example. That's why > we have max_period defined for each PMU. Furthermore, it still doesn't help us > in the stat case where prev will typically be quite small after we've had an > overflow and new may well overtake it.
In fact, I suggested to keep the overflow information to handle the case by reading hw counter overflow flag, but looks it is a bit frail to sync overflow flag with the counter value in non-interrupt situation, so I agree with you to remove 'overflow' parameter from armpmu_event_update.
> >> But I am afraid that the issue still can be triggered in sample-based situation, >> especially in very high frequency case: suppose the sample freq is 10000, >> 100us IRQ delay may trigger the issue. > > Not sure I follow. If the frequency is 10000, then we write 0xffffd8f0 to > the counter. That means we have a 0xffffd8f0 event window to read the
The frequency I described is the 'freq' from '-F freq'. On OMAP4, when the 'freq' is 10000 and the interval for one samle is 100us, the observed counter ('left' variable in armpmu_event_set_period) is about 90000, so the written value to the hw counter is 0xFFFEA06F, looks the window is wide enough, and we may not consider the issue in a737823d for sample-based profiling.
> counter after it overflows before new overtakes prev and we get confused. > If this passed in 100us then either your clock speed is 4.3*10^12Hz or you > have a seriously wide issue :)
On OMAP4, I can observe that about 19800 sample events can be generated with the below command:
'perf record -e cycles -F 10000 ./noploop 2&& perf report -D | tail -20'
So the above result looks not bad, :-)
> >> So we may use the overflow information to make perf more robust, IMO. > > There's a trade off between allowing the counter to wrap back around past > its previous value or being able to handle overflow on a non-interrupt path.
I agree, it is not easy to read overflow flag and counter accurately from hardware directly on a non-interrupt path.
So do you need me to prepare a new patch, or you will do it by yourself?
thanks, -- Ming Lei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |