Re: [tip:perfcounters/core] perf_counter: Rework the perf counter disable/enable

Paul Mackerras
2009-05-15T11:06:20Z

tip-bot for Peter Zijlstra writes:

> x86 NMI/INT throttling has non-nested use of this, breaking things. Therefore
> provide a reference counter disable/enable interface, where the first disable
> disables the hardware, and the last enable enables the hardware again.

It looks to me like what you've done for powerpc enables the hardware
again on the first enable, not the last one:

> @@ -436,7 +435,7 @@ u64 hw_perf_save_disable(void)
> * If we were previously disabled and counters were added, then
> * put the new config on the PMU.
> */
> -void hw_perf_restore(u64 disable)
> +void hw_perf_enable(void)
> {
> struct perf_counter *counter;
> struct cpu_hw_counters *cpuhw;
> @@ -448,9 +447,12 @@ void hw_perf_restore(u64 disable)
> int n_lim;
> int idx;
>
> - if (disable)
> - return;
> local_irq_save(flags);
> + if (!cpuhw->disabled) {
> + local_irq_restore(flags);
> + return;
> + }
> +
> cpuhw = &__get_cpu_var(cpu_hw_counters);
> cpuhw->disabled = 0;

I do rely on nesting the disable/enable calls and only having the
hardware re-enabled on the last enable. I can't see anything here
that detects the last enable. Have I missed it somewhere?

Paul.