RE: [Xen-devel] [PATCH] X86: Prefer TSC-deadline timer in Xen

["Wei, Gang" <gang.wei@xxxxxxxxx>]

> Tim Deegan [mailto:Tim.Deegan@xxxxxxxxxx], Thursday, October 28, 2010 10:23 PM
...
> > > >> > +static int tdt_enabled;
> > > >> > +static int tdt_disable;
> > > >> > +boolean_param("tdt_off", tdt_disable);
> > > >>
> > > >> It would be more natural to call the parameter just "tdt", and
> > > >> use a non-zero initialized variable that gets set to zero when
> > > >> the user passes "tdt=off" (or another of the boolean false
> > > >> indicators). Perhaps you could even get away with just the
> > > >> single "tdt_enabled" variable then.
> > > >
> > > > Rename the parameter should be ok. But I prefer to keep two variable
> there
> > > > to avoid check both tdt_enabled &
> > > boot_cpu_has(X86_FEATURE_TSC_DEADLINE)
> > > > everywhere.
> > >
> > > Why? Just clear tdt_enabled when you find
> > > !boot_cpu_has(X86_FEATURE_TSC_DEADLINE) during initialization.
> > >
> > > And btw., this (or if you really want to keep them separate, both)
> > > variable(s) are pretty reasonable candidates for __read_mostly.
> >
> > I still want to keep them because __setup_APIC_LVTT() will be called
> > multiple times - the first call with tdt_enabled == false, and the
> > following calls with tdt_enabled == true.
> 
> Is that important?  If so, please add explanatory comments in the
> appropriate places, because it's not obvious that it's happening, or why.

Think it again, I should remove tdt_enable and just keep tdt_enabled, and skip 
calibrate_APIC_clock() while tdt_enabled & 
boot_cpu_has(X86_FEATURE_TSC_DEADLINE). Thus things become simpler.

Jimmy

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel