WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

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