At 14:54 +0100 on 28 Oct (1288277689), Wei, Gang wrote:
> > From: Jan Beulich [mailto:JBeulich@xxxxxxxxxx], October 28, 2010 7:43 PM
> > >>> On 28.10.10 at 11:48, "Wei, Gang" <gang.wei@xxxxxxxxx> wrote:
> > >> From: Jan Beulich [mailto:JBeulich@xxxxxxxxxx], Thursday, October 28,
> > 2010
> > > 3:46 PM
> > >> >>> On 28.10.10 at 07:45, "Wei, Gang" <gang.wei@xxxxxxxxx> wrote:
> > >> > --- a/xen/arch/x86/apic.c Wed Oct 20 17:26:51 2010 +0100
> > >> > +++ b/xen/arch/x86/apic.c Fri Oct 29 19:24:56 2010 +0800
> > >> > @@ -37,6 +37,15 @@
> > >> > #include <asm/asm_defns.h> /* for BUILD_SMP_INTERRUPT */
> > >> > #include <mach_apic.h>
> > >> > #include <io_ports.h>
> > >> > +
> > >> > +#define APIC_TIMER_MODE_ONESHOT (0 << 17)
> > >> > +#define APIC_TIMER_MODE_PERIODIC (1 << 17)
> > >> > +#define APIC_TIMER_MODE_TSC_DEADLINE (2 << 17)
> > >> > +#define APIC_TIMER_MODE_MASK (3 << 17)
> > >> > +
> > >> > +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.
Tim.
--
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home