Messages in this thread | | | Date | Mon, 6 Aug 2018 23:50:06 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 2/3] x86, perf: Add a separate Arch Perfmon v4 PMI handler |
| |
On Mon, Aug 06, 2018 at 02:33:23PM -0700, Andi Kleen wrote: > On Mon, Aug 06, 2018 at 08:35:15PM +0200, Peter Zijlstra wrote: > > > +static bool disable_counter_freezing; > > > +module_param(disable_counter_freezing, bool, 0444); > > > +MODULE_PARM_DESC(disable_counter_freezing, "Disable counter freezing feature." > > > + "The PMI handler will fall back to generic handler." > > > + "Default is false (enable counter freezing feature)."); > > > > Why? > > See the description. Counter freezing took some time to stabilize, > so it seemed better to have a knob to ask users to try in case > there are more problems.
But it is not a module.. did you want early_param() or __setup()?
> > > + /* > > > + * Ack the PMU late after the APIC. This avoids bogus > > > > > + * freezing on Skylake CPUs. The acking unfreezes the PMU > > > + */ > > That doesn't make sense. PMU and APIC do not have order.> > > It makes a difference for the hardware.
I still have no clue what it wants to say.
> > > + /* > > > + * For arch perfmon 4 use counter freezing to avoid > > > + * several MSR accesses in the PMI. > > > + */ > > > + if (x86_pmu.counter_freezing) { > > > + x86_pmu.handle_irq = intel_pmu_handle_irq_v4; > > > + pr_cont("counter freezing, "); > > > + } > > > > Lets not print the counter freezing, we already print v4, right? > > I find it useful to see that the kernel has the support, otherwise > you would need to look at the version number, but it gets difficult > with backports. This is another paranoia bit, in case there > are problems.
That line will get ver long if we keep adding every dinky bit to it.
> > > @@ -561,6 +566,7 @@ struct x86_pmu { > > > struct x86_pmu_quirk *quirks; > > > int perfctr_second_write; > > > bool late_ack; > > > + bool counter_freezing; > > > > Please make the both of them int or something. > > That would make them bigger for no reason?
Then use u8 or something, I just don't much like _Bool in composite types.
| |