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: avoid LOCK prefix where possible

To: Jan Beulich <jbeulich@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] x86: avoid LOCK prefix where possible
From: Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>
Date: Mon, 10 Dec 2007 10:50:48 +0000
Delivery-date: Mon, 10 Dec 2007 02:52:13 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <475D141B.76E4.0078.0@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acg7GoXMxFORFKcNEdykEAAX8io7RQ==
Thread-topic: [Xen-devel] [PATCH] x86: avoid LOCK prefix where possible
User-agent: Microsoft-Entourage/11.3.6.070618
If these are changed in upstream then I'll pull the changes down. I don;t
think any of these extra LOCK prefixes are on critical paths.

 -- Keir

On 10/12/07 09:25, "Jan Beulich" <jbeulich@xxxxxxxxxx> wrote:

> The controversial part of this may be the change to the node mask
> handling: While all users of include/xen/nodemask.h are okay with
> non-atomic updates, users of include/xen/cpumask.h aren't.
> Nevertheless, the vast majority of the latter would be, so it might be
> desirable to have distinct accessors or designate the *_test_and_*
> ones to be atomic. If so, it'd probably be a good idea to make the
> node accessors follow the cpu ones in that respect.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
> 
> Index: 2007-12-10/xen/arch/x86/apic.c
> ===================================================================
> --- 2007-12-10.orig/xen/arch/x86/apic.c 2007-12-10 09:18:48.000000000 +0100
> +++ 2007-12-10/xen/arch/x86/apic.c 2007-12-10 09:19:12.000000000 +0100
> @@ -704,7 +704,7 @@ static void apic_pm_activate(void)
>  static void __init lapic_disable(char *str)
>  {
>      enable_local_apic = -1;
> -    clear_bit(X86_FEATURE_APIC, boot_cpu_data.x86_capability);
> +    __clear_bit(X86_FEATURE_APIC, boot_cpu_data.x86_capability);
>  }
>  custom_param("nolapic", lapic_disable);
>  
> @@ -784,7 +784,7 @@ static int __init detect_init_APIC (void
>          return -1;
>      }
>  
> -    set_bit(X86_FEATURE_APIC, boot_cpu_data.x86_capability);
> +    __set_bit(X86_FEATURE_APIC, boot_cpu_data.x86_capability);
>      mp_lapic_addr = APIC_DEFAULT_PHYS_BASE;
>  
>      /* The BIOS may have set up the APIC at some other address */
> @@ -1233,7 +1233,7 @@ fastcall void smp_error_interrupt(struct
>  int __init APIC_init_uniprocessor (void)
>  {
>      if (enable_local_apic < 0)
> -        clear_bit(X86_FEATURE_APIC, boot_cpu_data.x86_capability);
> +        __clear_bit(X86_FEATURE_APIC, boot_cpu_data.x86_capability);
>  
>      if (!smp_found_config && !cpu_has_apic)
>          return -1;
> @@ -1244,7 +1244,7 @@ int __init APIC_init_uniprocessor (void)
>      if (!cpu_has_apic &&
> APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid])) {
>          printk(KERN_ERR "BIOS bug, local APIC #%d not detected!...\n",
>                 boot_cpu_physical_apicid);
> -        clear_bit(X86_FEATURE_APIC, boot_cpu_data.x86_capability);
> +        __clear_bit(X86_FEATURE_APIC, boot_cpu_data.x86_capability);
>          return -1;
>      }
>  
> Index: 2007-12-10/xen/arch/x86/cpu/amd.c
> ===================================================================
> --- 2007-12-10.orig/xen/arch/x86/cpu/amd.c 2007-12-10 09:18:48.000000000 +0100
> +++ 2007-12-10/xen/arch/x86/cpu/amd.c 2007-12-10 09:19:12.000000000 +0100
> @@ -155,7 +155,7 @@ static void __init init_amd(struct cpuin
>  
> /* Bit 31 in normal CPUID used for nonstandard 3DNow ID;
>   3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */
> - clear_bit(0*32+31, c->x86_capability);
> + __clear_bit(0*32+31, c->x86_capability);
> 
> r = get_model_name(c);
>  
> @@ -181,8 +181,8 @@ static void __init init_amd(struct cpuin
> {
> /* Based on AMD doc 20734R - June 2000 */
> if ( c->x86_model == 0 ) {
> -     clear_bit(X86_FEATURE_APIC, c->x86_capability);
> -     set_bit(X86_FEATURE_PGE, c->x86_capability);
> +     __clear_bit(X86_FEATURE_APIC, c->x86_capability);
> +     __set_bit(X86_FEATURE_PGE, c->x86_capability);
> }
> break;
> }
> @@ -258,7 +258,7 @@ static void __init init_amd(struct cpuin
> /*  Set MTRR capability flag if appropriate */
> if (c->x86_model == 13 || c->x86_model == 9 ||
>   (c->x86_model == 8 && c->x86_mask >= 8))
> -     set_bit(X86_FEATURE_K6_MTRR, c->x86_capability);
> +     __set_bit(X86_FEATURE_K6_MTRR, c->x86_capability);
> break;
> }
>  
> @@ -280,7 +280,7 @@ static void __init init_amd(struct cpuin
> rdmsr(MSR_K7_HWCR, l, h);
> l &= ~0x00008000;
> wrmsr(MSR_K7_HWCR, l, h);
> -     set_bit(X86_FEATURE_XMM, c->x86_capability);
> +     __set_bit(X86_FEATURE_XMM, c->x86_capability);
> }
> }
>  
> @@ -304,13 +304,13 @@ static void __init init_amd(struct cpuin
> /* Use K8 tuning for Fam10h and Fam11h */
> case 0x10:
> case 0x11:
> -  set_bit(X86_FEATURE_K8, c->x86_capability);
> +  __set_bit(X86_FEATURE_K8, c->x86_capability);
> disable_c1e(NULL);
> if (acpi_smi_cmd && (acpi_enable_value | acpi_disable_value))
> pv_post_outb_hook = check_disable_c1e;
> break;
> case 6:
> -  set_bit(X86_FEATURE_K7, c->x86_capability);
> +  __set_bit(X86_FEATURE_K7, c->x86_capability);
> break;
> }
>  
> @@ -338,7 +338,7 @@ static void __init init_amd(struct cpuin
> if (cpuid_eax(0x80000000) >= 0x80000007) {
> c->x86_power = cpuid_edx(0x80000007);
> if (c->x86_power & (1<<8))
> -   set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> +   __set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> }
>  
>  #ifdef CONFIG_X86_HT
> @@ -363,15 +363,15 @@ static void __init init_amd(struct cpuin
>  
> /* Pointless to use MWAIT on Family10 as it does not deep sleep. */
> if (c->x86 == 0x10 && !force_mwait)
> -  clear_bit(X86_FEATURE_MWAIT, c->x86_capability);
> +  __clear_bit(X86_FEATURE_MWAIT, c->x86_capability);
>  
> /* K6s reports MCEs but don't actually have all the MSRs */
> if (c->x86 < 6)
> -  clear_bit(X86_FEATURE_MCE, c->x86_capability);
> +  __clear_bit(X86_FEATURE_MCE, c->x86_capability);
>  
>  #ifdef __x86_64__
> /* AMD CPUs do not support SYSENTER outside of legacy mode. */
> - clear_bit(X86_FEATURE_SEP, c->x86_capability);
> + __clear_bit(X86_FEATURE_SEP, c->x86_capability);
>  #endif
>  
> /* Prevent TSC drift in non single-processor, single-core platforms. */
> Index: 2007-12-10/xen/arch/x86/cpu/centaur.c
> ===================================================================
> --- 2007-12-10.orig/xen/arch/x86/cpu/centaur.c 2007-12-10 09:18:48.000000000
> +0100
> +++ 2007-12-10/xen/arch/x86/cpu/centaur.c 2007-12-10 09:19:12.000000000 +0100
> @@ -50,12 +50,12 @@ static void __init init_c3(struct cpuinf
> rdmsr (MSR_VIA_FCR, lo, hi);
> lo |= (1<<1 | 1<<7);
> wrmsr (MSR_VIA_FCR, lo, hi);
> -  set_bit(X86_FEATURE_CX8, c->x86_capability);
> +  __set_bit(X86_FEATURE_CX8, c->x86_capability);
> }
>  
> /* Before Nehemiah, the C3's had 3dNOW! */
> if (c->x86_model >=6 && c->x86_model <9)
> -  set_bit(X86_FEATURE_3DNOW, c->x86_capability);
> +  __set_bit(X86_FEATURE_3DNOW, c->x86_capability);
>  
> get_model_name(c);
> display_cacheinfo(c);
> @@ -65,7 +65,7 @@ static void __init init_centaur(struct c
>  {
> /* Bit 31 in normal CPUID used for nonstandard 3DNow ID;
>   3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */
> - clear_bit(0*32+31, c->x86_capability);
> + __clear_bit(0*32+31, c->x86_capability);
>  
> if (c->x86 == 6)
> init_c3(c);
> Index: 2007-12-10/xen/arch/x86/cpu/common.c
> ===================================================================
> --- 2007-12-10.orig/xen/arch/x86/cpu/common.c 2007-12-10 09:18:48.000000000
> +0100
> +++ 2007-12-10/xen/arch/x86/cpu/common.c 2007-12-10 09:19:12.000000000 +0100
> @@ -304,7 +304,7 @@ static void __cpuinit squash_the_stupid_
> lo |= 0x200000;
> wrmsr(MSR_IA32_BBL_CR_CTL,lo,hi);
> printk(KERN_NOTICE "CPU serial number disabled.\n");
> -  clear_bit(X86_FEATURE_PN, c->x86_capability);
> +  __clear_bit(X86_FEATURE_PN, c->x86_capability);
>  
> /* Disabling the serial number may affect the cpuid level */
> c->cpuid_level = cpuid_eax(0);
> @@ -384,16 +384,16 @@ void __cpuinit identify_cpu(struct cpuin
>  
> /* TSC disabled? */
> if ( tsc_disable )
> -  clear_bit(X86_FEATURE_TSC, c->x86_capability);
> +  __clear_bit(X86_FEATURE_TSC, c->x86_capability);
>  
> /* FXSR disabled? */
> if (disable_x86_fxsr) {
> -  clear_bit(X86_FEATURE_FXSR, c->x86_capability);
> -  clear_bit(X86_FEATURE_XMM, c->x86_capability);
> +  __clear_bit(X86_FEATURE_FXSR, c->x86_capability);
> +  __clear_bit(X86_FEATURE_XMM, c->x86_capability);
> }
>  
> if (disable_pse)
> -  clear_bit(X86_FEATURE_PSE, c->x86_capability);
> +  __clear_bit(X86_FEATURE_PSE, c->x86_capability);
>  
> /* If the model name is still unset, do table lookup. */
> if ( !c->x86_model_id[0] ) {
> Index: 2007-12-10/xen/arch/x86/cpu/cyrix.c
> ===================================================================
> --- 2007-12-10.orig/xen/arch/x86/cpu/cyrix.c 2007-12-10 09:18:48.000000000
> +0100
> +++ 2007-12-10/xen/arch/x86/cpu/cyrix.c 2007-12-10 09:19:12.000000000 +0100
> @@ -185,12 +185,12 @@ static void __init init_cyrix(struct cpu
>  
> /* Bit 31 in normal CPUID used for nonstandard 3DNow ID;
>   3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */
> - clear_bit(0*32+31, c->x86_capability);
> + __clear_bit(0*32+31, c->x86_capability);
>  
> /* Cyrix used bit 24 in extended (AMD) CPUID for Cyrix MMX extensions */
> if ( test_bit(1*32+24, c->x86_capability) ) {
> -  clear_bit(1*32+24, c->x86_capability);
> -  set_bit(X86_FEATURE_CXMMX, c->x86_capability);
> +  __clear_bit(1*32+24, c->x86_capability);
> +  __set_bit(X86_FEATURE_CXMMX, c->x86_capability);
> }
>  
> do_cyrix_devid(&dir0, &dir1);
> @@ -237,7 +237,7 @@ static void __init init_cyrix(struct cpu
> } else             /* 686 */
> p = Cx86_cb+1;
> /* Emulate MTRRs using Cyrix's ARRs. */
> -  set_bit(X86_FEATURE_CYRIX_ARR, c->x86_capability);
> +  __set_bit(X86_FEATURE_CYRIX_ARR, c->x86_capability);
> /* 6x86's contain this bug */
> /*c->coma_bug = 1;*/
> break;
> @@ -280,7 +280,7 @@ static void __init init_cyrix(struct cpu
> if (((dir1 & 0x0f) > 4) || ((dir1 & 0xf0) == 0x20))
> (c->x86_model)++;
> /* Emulate MTRRs using Cyrix's ARRs. */
> -  set_bit(X86_FEATURE_CYRIX_ARR, c->x86_capability);
> +  __set_bit(X86_FEATURE_CYRIX_ARR, c->x86_capability);
> break;
>  
> case 0xf:  /* Cyrix 486 without DEVID registers */
> Index: 2007-12-10/xen/arch/x86/cpu/intel.c
> ===================================================================
> --- 2007-12-10.orig/xen/arch/x86/cpu/intel.c 2007-12-10 09:18:48.000000000
> +0100
> +++ 2007-12-10/xen/arch/x86/cpu/intel.c 2007-12-10 09:19:12.000000000 +0100
> @@ -121,7 +121,7 @@ static void __devinit init_intel(struct
>  
> /* SEP CPUID bug: Pentium Pro reports SEP but doesn't have it until model 3
> mask 3 */
> if ((c->x86<<8 | c->x86_model<<4 | c->x86_mask) < 0x633)
> -  clear_bit(X86_FEATURE_SEP, c->x86_capability);
> +  __clear_bit(X86_FEATURE_SEP, c->x86_capability);
>  
> /* Names for the Pentium II/Celeron processors
>   detectable only by also checking the cache size.
> @@ -180,12 +180,12 @@ static void __devinit init_intel(struct
>  #endif
>  
> if (c->x86 == 15)
> -  set_bit(X86_FEATURE_P4, c->x86_capability);
> +  __set_bit(X86_FEATURE_P4, c->x86_capability);
> if (c->x86 == 6) 
> -  set_bit(X86_FEATURE_P3, c->x86_capability);
> +  __set_bit(X86_FEATURE_P3, c->x86_capability);
> if ((c->x86 == 0xf && c->x86_model >= 0x03) ||
> (c->x86 == 0x6 && c->x86_model >= 0x0e))
> -  set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> +  __set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
>  
> start_vmx();
>  }
> Index: 2007-12-10/xen/arch/x86/hvm/hvm.c
> ===================================================================
> --- 2007-12-10.orig/xen/arch/x86/hvm/hvm.c 2007-12-10 09:18:48.000000000 +0100
> +++ 2007-12-10/xen/arch/x86/hvm/hvm.c 2007-12-10 09:19:12.000000000 +0100
> @@ -81,7 +81,7 @@ void hvm_enable(struct hvm_function_tabl
>       * delays, but the vmexits simply slow things down).
>       */
>      memset(hvm_io_bitmap, ~0, sizeof(hvm_io_bitmap));
> -    clear_bit(0x80, hvm_io_bitmap);
> +    __clear_bit(0x80, hvm_io_bitmap);
>  
>      hvm_funcs   = *fns;
>      hvm_enabled = 1;
> Index: 2007-12-10/xen/arch/x86/hvm/vlapic.c
> ===================================================================
> --- 2007-12-10.orig/xen/arch/x86/hvm/vlapic.c 2007-12-10 09:18:48.000000000
> +0100
> +++ 2007-12-10/xen/arch/x86/hvm/vlapic.c 2007-12-10 09:19:12.000000000 +0100
> @@ -408,7 +408,7 @@ static void vlapic_ipi(struct vlapic *vl
>          if ( vlapic_match_dest(v, vlapic, short_hand, dest, dest_mode) )
>          {
>              if ( delivery_mode == APIC_DM_LOWEST )
> -                set_bit(v->vcpu_id, &lpr_map);
> +                __set_bit(v->vcpu_id, &lpr_map);
>              else
>                  vlapic_accept_irq(v, delivery_mode,
>                                    vector, level, trig_mode);
> Index: 2007-12-10/xen/arch/x86/traps.c
> ===================================================================
> --- 2007-12-10.orig/xen/arch/x86/traps.c 2007-12-10 09:18:48.000000000 +0100
> +++ 2007-12-10/xen/arch/x86/traps.c 2007-12-10 09:19:12.000000000 +0100
> @@ -678,25 +678,25 @@ static int emulate_forced_invalid_op(str
>      if ( regs->eax == 1 )
>      {
>          /* Modify Feature Information. */
> -        clear_bit(X86_FEATURE_VME, &d);
> -        clear_bit(X86_FEATURE_PSE, &d);
> -        clear_bit(X86_FEATURE_PGE, &d);
> +        __clear_bit(X86_FEATURE_VME, &d);
> +        __clear_bit(X86_FEATURE_PSE, &d);
> +        __clear_bit(X86_FEATURE_PGE, &d);
>          if ( !cpu_has_sep )
> -            clear_bit(X86_FEATURE_SEP, &d);
> +            __clear_bit(X86_FEATURE_SEP, &d);
>  #ifdef __i386__
>          if ( !supervisor_mode_kernel )
> -            clear_bit(X86_FEATURE_SEP, &d);
> +            __clear_bit(X86_FEATURE_SEP, &d);
>  #endif
>          if ( !IS_PRIV(current->domain) )
> -            clear_bit(X86_FEATURE_MTRR, &d);
> +            __clear_bit(X86_FEATURE_MTRR, &d);
>      }
>      else if ( regs->eax == 0x80000001 )
>      {
>          /* Modify Feature Information. */
>  #ifdef __i386__
> -        clear_bit(X86_FEATURE_SYSCALL % 32, &d);
> +        __clear_bit(X86_FEATURE_SYSCALL % 32, &d);
>  #endif
> -        clear_bit(X86_FEATURE_RDTSCP % 32, &d);
> +        __clear_bit(X86_FEATURE_RDTSCP % 32, &d);
>      }
>      else
>      {
> Index: 2007-12-10/xen/common/page_alloc.c
> ===================================================================
> --- 2007-12-10.orig/xen/common/page_alloc.c 2007-12-10 09:18:48.000000000
> +0100
> +++ 2007-12-10/xen/common/page_alloc.c 2007-12-10 09:19:12.000000000 +0100
> @@ -301,14 +301,15 @@ static void init_node_heap(int node)
>      /* First node to be discovered has its heap metadata statically alloced.
> */
>      static heap_by_zone_and_order_t _heap_static;
>      static unsigned long avail_static[NR_ZONES];
> -    static unsigned long first_node_initialised;
> +    static int first_node_initialised;
>  
>      int i, j;
>  
> -    if ( !test_and_set_bit(0, &first_node_initialised) )
> +    if ( !first_node_initialised )
>      {
>          _heap[node] = &_heap_static;
>          avail[node] = avail_static;
> +        first_node_initialised = 1;
>      }
>      else
>      {
> Index: 2007-12-10/xen/include/asm-x86/mpspec.h
> ===================================================================
> --- 2007-12-10.orig/xen/include/asm-x86/mpspec.h 2007-12-10 09:18:48.000000000
> +0100
> +++ 2007-12-10/xen/include/asm-x86/mpspec.h 2007-12-10 09:19:12.000000000
> +0100
> @@ -45,10 +45,10 @@ struct physid_mask
>  
>  typedef struct physid_mask physid_mask_t;
>  
> -#define physid_set(physid, map)   set_bit(physid, (map).mask)
> -#define physid_clear(physid, map)  clear_bit(physid, (map).mask)
> +#define physid_set(physid, map)   __set_bit(physid, (map).mask)
> +#define physid_clear(physid, map)  __clear_bit(physid, (map).mask)
>  #define physid_isset(physid, map)  test_bit(physid, (map).mask)
> -#define physid_test_and_set(physid, map) test_and_set_bit(physid, (map).mask)
> +#define physid_test_and_set(physid, map) __test_and_set_bit(physid,
> (map).mask)
>  
>  #define physids_and(dst, src1, src2)  bitmap_and((dst).mask, (src1).mask,
> (src2).mask, MAX_APICS)
>  #define physids_or(dst, src1, src2)  bitmap_or((dst).mask, (src1).mask,
> (src2).mask, MAX_APICS)
> Index: 2007-12-10/xen/include/xen/nodemask.h
> ===================================================================
> --- 2007-12-10.orig/xen/include/xen/nodemask.h 2007-12-10 09:18:48.000000000
> +0100
> +++ 2007-12-10/xen/include/xen/nodemask.h 2007-12-10 09:19:12.000000000 +0100
> @@ -80,15 +80,15 @@ typedef struct { DECLARE_BITMAP(bits, MA
>  extern nodemask_t _unused_nodemask_arg_;
>  
>  #define node_set(node, dst) __node_set((node), &(dst))
> -static inline void __node_set(int node, volatile nodemask_t *dstp)
> +static inline void __node_set(int node, nodemask_t *dstp)
>  {
> - set_bit(node, dstp->bits);
> + __set_bit(node, dstp->bits);
>  }
>  
>  #define node_clear(node, dst) __node_clear((node), &(dst))
> -static inline void __node_clear(int node, volatile nodemask_t *dstp)
> +static inline void __node_clear(int node, nodemask_t *dstp)
>  {
> - clear_bit(node, dstp->bits);
> + __clear_bit(node, dstp->bits);
>  }
>  
>  #define nodes_setall(dst) __nodes_setall(&(dst), MAX_NUMNODES)
> @@ -110,7 +110,7 @@ static inline void __nodes_clear(nodemas
> __node_test_and_set((node), &(nodemask))
>  static inline int __node_test_and_set(int node, nodemask_t *addr)
>  {
> - return test_and_set_bit(node, addr->bits);
> + return __test_and_set_bit(node, addr->bits);
>  }
>  
>  #define nodes_and(dst, src1, src2) \
> @@ -329,8 +329,8 @@ extern nodemask_t node_possible_map;
> node;     \
>  })
>  
> -#define node_set_online(node)    set_bit((node), node_online_map.bits)
> -#define node_set_offline(node)    clear_bit((node), node_online_map.bits)
> +#define node_set_online(node)    __set_bit((node), node_online_map.bits)
> +#define node_set_offline(node)    __clear_bit((node), node_online_map.bits)
>  
>  #define for_each_node(node)    for_each_node_mask((node), node_possible_map)
>  #define for_each_online_node(node) for_each_node_mask((node),
> node_online_map)
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



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

<Prev in Thread] Current Thread [Next in Thread>