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

Peter Zijlstra
2009-05-15T11:24:18Z

On Fri, 2009-05-15 at 21:05 +1000, Paul Mackerras wrote:
> 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?

+void perf_disable(void)
+{
+ __perf_disable();
+ hw_perf_disable();
+}

+void perf_enable(void)
+{
+ if (__perf_enable())
+ hw_perf_enable();
+}