thr3ads.net - Xen devel - [Xen-devel] [PATCH] x86: clean up physid_mask

If this information is useful, please help other people find it:
Share via:

Jan Beulich

2011-Oct-14 08:57 UTC

[Xen-devel] [PATCH] x86: clean up physid_mask_t handling

This eliminates passing and returning by value of this type (making it
unnecessary for the compiler to create temporary variables for doing so
on the stack), thus dramatically reducing the stack frame sizes of a
couple of functions (was in one case over 12k for a 4095-CPU build).

In one case a local variable gets converted to a static one, possible
because the function gets called at most once (during early boot).

Some accessors get eliminated altogether as being unused or as being
equally well open coded at the place of use.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -1464,7 +1464,8 @@ int __init APIC_init_uniprocessor (void)
 #ifdef CONFIG_CRASH_DUMP
     boot_cpu_physical_apicid = get_apic_id();
 #endif
-    phys_cpu_present_map = physid_mask_of_physid(boot_cpu_physical_apicid);
+    physids_clear(phys_cpu_present_map);
+    physid_set(boot_cpu_physical_apicid, phys_cpu_present_map);
 
     setup_local_APIC();
 
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -1463,7 +1463,7 @@ void disable_IO_APIC(void)
 static void __init setup_ioapic_ids_from_mpc(void)
 {
     union IO_APIC_reg_00 reg_00;
-    physid_mask_t phys_id_present_map;
+    static physid_mask_t __initdata phys_id_present_map;
     int apic;
     int i;
     unsigned char old_id;
@@ -1481,7 +1481,7 @@ static void __init setup_ioapic_ids_from
      * This is broken; anything with a real cpu count has to
      * circumvent this idiocy regardless.
      */
-    phys_id_present_map = ioapic_phys_id_map(phys_cpu_present_map);
+    ioapic_phys_id_map(&phys_id_present_map);
 
     /*
      * Set the IOAPIC ID to the value stored in the MPC table.
@@ -1508,7 +1508,7 @@ static void __init setup_ioapic_ids_from
          * system must have a unique ID or we get lots of nice
          * ''stuck on smp_invalidate_needed IPI wait''
messages.
          */
-        if (check_apicid_used(phys_id_present_map,
+        if (check_apicid_used(&phys_id_present_map,
                               mp_ioapics[apic].mpc_apicid)) {
             printk(KERN_ERR "BIOS bug, IO-APIC#%d ID %d is already
used!...\n",
                    apic, mp_ioapics[apic].mpc_apicid);
@@ -1519,17 +1519,13 @@ static void __init setup_ioapic_ids_from
                 panic("Max APIC ID exceeded!\n");
             printk(KERN_ERR "... fixing up to %d. (tell your hw
vendor)\n",
                    i);
-            physid_set(i, phys_id_present_map);
             mp_ioapics[apic].mpc_apicid = i;
         } else {
-            physid_mask_t tmp;
-            tmp = apicid_to_cpu_present(mp_ioapics[apic].mpc_apicid);
             apic_printk(APIC_VERBOSE, "Setting %d in the "
                         "phys_id_present_map\n",
                         mp_ioapics[apic].mpc_apicid);
-            physids_or(phys_id_present_map, phys_id_present_map, tmp);
         }
-
+        set_apicid(mp_ioapics[apic].mpc_apicid, &phys_id_present_map);
 
         /*
          * We need to adjust the IRQ routing table
@@ -2195,7 +2191,6 @@ int __init io_apic_get_unique_id (int io
 {
     union IO_APIC_reg_00 reg_00;
     static physid_mask_t __initdata apic_id_map = PHYSID_MASK_NONE;
-    physid_mask_t tmp;
     unsigned long flags;
     int i = 0;
 
@@ -2209,7 +2204,7 @@ int __init io_apic_get_unique_id (int io
      */
 
     if (physids_empty(apic_id_map))
-        apic_id_map = ioapic_phys_id_map(phys_cpu_present_map);
+        ioapic_phys_id_map(&apic_id_map);
 
     spin_lock_irqsave(&ioapic_lock, flags);
     reg_00.raw = io_apic_read(ioapic, 0);
@@ -2225,10 +2220,10 @@ int __init io_apic_get_unique_id (int io
      * Every APIC in a system must have a unique ID or we get lots of nice 
      * ''stuck on smp_invalidate_needed IPI wait'' messages.
      */
-    if (check_apicid_used(apic_id_map, apic_id)) {
+    if (check_apicid_used(&apic_id_map, apic_id)) {
 
         for (i = 0; i < get_physical_broadcast(); i++) {
-            if (!check_apicid_used(apic_id_map, i))
+            if (!check_apicid_used(&apic_id_map, i))
                 break;
         }
 
@@ -2241,8 +2236,7 @@ int __init io_apic_get_unique_id (int io
         apic_id = i;
     } 
 
-    tmp = apicid_to_cpu_present(apic_id);
-    physids_or(apic_id_map, apic_id_map, tmp);
+    set_apicid(apic_id, &apic_id_map);
 
     if (reg_00.bits.ID != apic_id) {
         reg_00.bits.ID = apic_id;
--- a/xen/arch/x86/mpparse.c
+++ b/xen/arch/x86/mpparse.c
@@ -89,7 +89,6 @@ static int __devinit MP_processor_info_x
 					 u32 apicidx, bool_t hotplug)
 {
  	int ver, apicid, cpu = 0;
-	physid_mask_t phys_cpu;
  	
 	if (!(m->mpc_cpuflag & CPU_ENABLED))
 		return -EINVAL;
@@ -114,8 +113,7 @@ static int __devinit MP_processor_info_x
 	}
 	apic_version[apicid] = ver;
 
-	phys_cpu = apicid_to_cpu_present(apicid);
-	physids_or(phys_cpu_present_map, phys_cpu_present_map, phys_cpu);
+	set_apicid(apicid, &phys_cpu_present_map);
 
 	if (num_processors >= NR_CPUS) {
 		printk(KERN_WARNING "WARNING: NR_CPUS limit of %i reached."
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -752,7 +752,8 @@ void __init smp_prepare_cpus(unsigned in
     {
         printk(KERN_NOTICE "SMP motherboard not detected.\n");
     init_uniprocessor:
-        phys_cpu_present_map = physid_mask_of_physid(0);
+        physids_clear(phys_cpu_present_map);
+        physid_set(0, phys_cpu_present_map);
         if (APIC_init_uniprocessor())
             printk(KERN_NOTICE "Local APIC not detected."
                    " Using dummy APIC emulation.\n");
@@ -767,7 +768,7 @@ void __init smp_prepare_cpus(unsigned in
      * CPU too, but we do it for the sake of robustness anyway.
      * Makes no sense to do this check in clustered apic mode, so skip it
      */
-    if ( !check_phys_apicid_present(boot_cpu_physical_apicid) )
+    if ( !check_apicid_present(boot_cpu_physical_apicid) )
     {
         printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
                boot_cpu_physical_apicid);
--- a/xen/include/asm-x86/mach-generic/mach_apic.h
+++ b/xen/include/asm-x86/mach-generic/mach_apic.h
@@ -58,29 +58,24 @@ static inline int apic_id_registered(voi
 			    phys_cpu_present_map);
 }
 
-static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
+static inline void ioapic_phys_id_map(physid_mask_t *map)
 {
-	return phys_map;
+	*map = phys_cpu_present_map;
 }
 
-static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
+static inline int check_apicid_used(const physid_mask_t *map, int apicid)
 {
-	return physid_isset(apicid, bitmap);
+	return physid_isset(apicid, *map);
 }
 
-static inline unsigned long check_apicid_present(int apicid)
+static inline int check_apicid_present(int apicid)
 {
 	return physid_isset(apicid, phys_cpu_present_map);
 }
 
-static inline int check_phys_apicid_present(int boot_cpu_physical_apicid)
+static inline void set_apicid(int phys_apicid, physid_mask_t *map)
 {
-	return physid_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
-}
-
-static inline physid_mask_t apicid_to_cpu_present(int phys_apicid)
-{
-	return physid_mask_of_physid(phys_apicid);
+	physid_set(phys_apicid, *map);
 }
 
 #endif /* __ASM_MACH_APIC_H */
--- a/xen/include/asm-x86/mpspec.h
+++ b/xen/include/asm-x86/mpspec.h
@@ -50,23 +50,6 @@ typedef struct physid_mask physid_mask_t
 #define physids_empty(map)			bitmap_empty((map).mask, MAX_APICS)
 #define physids_equal(map1, map2)		bitmap_equal((map1).mask, (map2).mask,
MAX_APICS)
 #define physids_weight(map)			bitmap_weight((map).mask, MAX_APICS)
-#define physids_shift_right(d, s, n)		bitmap_shift_right((d).mask, (s).mask, n,
MAX_APICS)
-#define physids_shift_left(d, s, n)		bitmap_shift_left((d).mask, (s).mask, n,
MAX_APICS)
-#define physids_coerce(map)			((map).mask[0])
-
-#define physids_promote(physids)						\
-	({									\
-		physid_mask_t __physid_mask = PHYSID_MASK_NONE;			\
-		__physid_mask.mask[0] = physids;				\
-		__physid_mask;							\
-	})
-
-#define physid_mask_of_physid(physid)						\
-	({									\
-		physid_mask_t __physid_mask = PHYSID_MASK_NONE;			\
-		physid_set(physid, __physid_mask);				\
-		__physid_mask;							\
-	})
 
 #define PHYSID_MASK_ALL		{ {[0 ... PHYSID_ARRAY_SIZE-1] = ~0UL} }
 #define PHYSID_MASK_NONE	{ {[0 ... PHYSID_ARRAY_SIZE-1] = 0UL} }



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

Keir Fraser

2011-Oct-14 09:14 UTC

head link

Re: [Xen-devel] [PATCH] x86: clean up physid_mask_t handling

On 14/10/2011 09:57, "Jan Beulich" <JBeulich@suse.com> wrote:
> This eliminates passing and returning by value of this type (making it
> unnecessary for the compiler to create temporary variables for doing so
> on the stack), thus dramatically reducing the stack frame sizes of a
> couple of functions (was in one case over 12k for a 4095-CPU build).
> 
> In one case a local variable gets converted to a static one, possible
> because the function gets called at most once (during early boot).
> 
> Some accessors get eliminated altogether as being unused or as being
> equally well open coded at the place of use.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -1464,7 +1464,8 @@ int __init APIC_init_uniprocessor (void)
>  #ifdef CONFIG_CRASH_DUMP
>      boot_cpu_physical_apicid = get_apic_id();
>  #endif
> -    phys_cpu_present_map =
physid_mask_of_physid(boot_cpu_physical_apicid);
> +    physids_clear(phys_cpu_present_map);
> +    physid_set(boot_cpu_physical_apicid, phys_cpu_present_map);
>  
>      setup_local_APIC();
>  
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -1463,7 +1463,7 @@ void disable_IO_APIC(void)
>  static void __init setup_ioapic_ids_from_mpc(void)
>  {
>      union IO_APIC_reg_00 reg_00;
> -    physid_mask_t phys_id_present_map;
> +    static physid_mask_t __initdata phys_id_present_map;
>      int apic;
>      int i;
>      unsigned char old_id;
> @@ -1481,7 +1481,7 @@ static void __init setup_ioapic_ids_from
>       * This is broken; anything with a real cpu count has to
>       * circumvent this idiocy regardless.
>       */
> -    phys_id_present_map = ioapic_phys_id_map(phys_cpu_present_map);
> +    ioapic_phys_id_map(&phys_id_present_map);
>  
>      /*
>       * Set the IOAPIC ID to the value stored in the MPC table.
> @@ -1508,7 +1508,7 @@ static void __init setup_ioapic_ids_from
>           * system must have a unique ID or we get lots of nice
>           * ''stuck on smp_invalidate_needed IPI wait''
messages.
>           */
> -        if (check_apicid_used(phys_id_present_map,
> +        if (check_apicid_used(&phys_id_present_map,
>                                mp_ioapics[apic].mpc_apicid)) {
>              printk(KERN_ERR "BIOS bug, IO-APIC#%d ID %d is already
> used!...\n",
>                     apic, mp_ioapics[apic].mpc_apicid);
> @@ -1519,17 +1519,13 @@ static void __init setup_ioapic_ids_from
>                  panic("Max APIC ID exceeded!\n");
>              printk(KERN_ERR "... fixing up to %d. (tell your hw
vendor)\n",
>                     i);
> -            physid_set(i, phys_id_present_map);
>              mp_ioapics[apic].mpc_apicid = i;
>          } else {
> -            physid_mask_t tmp;
> -            tmp = apicid_to_cpu_present(mp_ioapics[apic].mpc_apicid);
>              apic_printk(APIC_VERBOSE, "Setting %d in the "
>                          "phys_id_present_map\n",
>                          mp_ioapics[apic].mpc_apicid);
> -            physids_or(phys_id_present_map, phys_id_present_map, tmp);
>          }
> -
> +        set_apicid(mp_ioapics[apic].mpc_apicid, &phys_id_present_map);
>  
>          /*
>           * We need to adjust the IRQ routing table
> @@ -2195,7 +2191,6 @@ int __init io_apic_get_unique_id (int io
>  {
>      union IO_APIC_reg_00 reg_00;
>      static physid_mask_t __initdata apic_id_map = PHYSID_MASK_NONE;
> -    physid_mask_t tmp;
>      unsigned long flags;
>      int i = 0;
>  
> @@ -2209,7 +2204,7 @@ int __init io_apic_get_unique_id (int io
>       */
>  
>      if (physids_empty(apic_id_map))
> -        apic_id_map = ioapic_phys_id_map(phys_cpu_present_map);
> +        ioapic_phys_id_map(&apic_id_map);
>  
>      spin_lock_irqsave(&ioapic_lock, flags);
>      reg_00.raw = io_apic_read(ioapic, 0);
> @@ -2225,10 +2220,10 @@ int __init io_apic_get_unique_id (int io
>       * Every APIC in a system must have a unique ID or we get lots of nice
>       * ''stuck on smp_invalidate_needed IPI wait''
messages.
>       */
> -    if (check_apicid_used(apic_id_map, apic_id)) {
> +    if (check_apicid_used(&apic_id_map, apic_id)) {
>  
>          for (i = 0; i < get_physical_broadcast(); i++) {
> -            if (!check_apicid_used(apic_id_map, i))
> +            if (!check_apicid_used(&apic_id_map, i))
>                  break;
>          }
>  
> @@ -2241,8 +2236,7 @@ int __init io_apic_get_unique_id (int io
>          apic_id = i;
>      } 
>  
> -    tmp = apicid_to_cpu_present(apic_id);
> -    physids_or(apic_id_map, apic_id_map, tmp);
> +    set_apicid(apic_id, &apic_id_map);
>  
>      if (reg_00.bits.ID != apic_id) {
>          reg_00.bits.ID = apic_id;
> --- a/xen/arch/x86/mpparse.c
> +++ b/xen/arch/x86/mpparse.c
> @@ -89,7 +89,6 @@ static int __devinit MP_processor_info_x
> u32 apicidx, bool_t hotplug)
>  {
> int ver, apicid, cpu = 0;
> - physid_mask_t phys_cpu;
> 
> if (!(m->mpc_cpuflag & CPU_ENABLED))
> return -EINVAL;
> @@ -114,8 +113,7 @@ static int __devinit MP_processor_info_x
> }
> apic_version[apicid] = ver;
>  
> - phys_cpu = apicid_to_cpu_present(apicid);
> - physids_or(phys_cpu_present_map, phys_cpu_present_map, phys_cpu);
> + set_apicid(apicid, &phys_cpu_present_map);
>  
> if (num_processors >= NR_CPUS) {
> printk(KERN_WARNING "WARNING: NR_CPUS limit of %i reached."
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -752,7 +752,8 @@ void __init smp_prepare_cpus(unsigned in
>      {
>          printk(KERN_NOTICE "SMP motherboard not detected.\n");
>      init_uniprocessor:
> -        phys_cpu_present_map = physid_mask_of_physid(0);
> +        physids_clear(phys_cpu_present_map);
> +        physid_set(0, phys_cpu_present_map);
>          if (APIC_init_uniprocessor())
>              printk(KERN_NOTICE "Local APIC not detected."
>                     " Using dummy APIC emulation.\n");
> @@ -767,7 +768,7 @@ void __init smp_prepare_cpus(unsigned in
>       * CPU too, but we do it for the sake of robustness anyway.
>       * Makes no sense to do this check in clustered apic mode, so skip it
>       */
> -    if ( !check_phys_apicid_present(boot_cpu_physical_apicid) )
> +    if ( !check_apicid_present(boot_cpu_physical_apicid) )
>      {
>          printk("weird, boot CPU (#%d) not listed by the
BIOS.\n",
>                 boot_cpu_physical_apicid);
> --- a/xen/include/asm-x86/mach-generic/mach_apic.h
> +++ b/xen/include/asm-x86/mach-generic/mach_apic.h
> @@ -58,29 +58,24 @@ static inline int apic_id_registered(voi
>    phys_cpu_present_map);
>  }
>  
> -static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
> +static inline void ioapic_phys_id_map(physid_mask_t *map)
>  {
> - return phys_map;
> + *map = phys_cpu_present_map;
>  }
>  
> -static inline unsigned long check_apicid_used(physid_mask_t bitmap, int
> apicid)
> +static inline int check_apicid_used(const physid_mask_t *map, int apicid)
>  {
> - return physid_isset(apicid, bitmap);
> + return physid_isset(apicid, *map);
>  }
>  
> -static inline unsigned long check_apicid_present(int apicid)
> +static inline int check_apicid_present(int apicid)
>  {
> return physid_isset(apicid, phys_cpu_present_map);
>  }
>  
> -static inline int check_phys_apicid_present(int boot_cpu_physical_apicid)
> +static inline void set_apicid(int phys_apicid, physid_mask_t *map)
>  {
> - return physid_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
> -}
> -
> -static inline physid_mask_t apicid_to_cpu_present(int phys_apicid)
> -{
> - return physid_mask_of_physid(phys_apicid);
> + physid_set(phys_apicid, *map);
>  }
>  
>  #endif /* __ASM_MACH_APIC_H */
> --- a/xen/include/asm-x86/mpspec.h
> +++ b/xen/include/asm-x86/mpspec.h
> @@ -50,23 +50,6 @@ typedef struct physid_mask physid_mask_t
>  #define physids_empty(map)   bitmap_empty((map).mask, MAX_APICS)
>  #define physids_equal(map1, map2)  bitmap_equal((map1).mask, (map2).mask,
> MAX_APICS)
>  #define physids_weight(map)   bitmap_weight((map).mask, MAX_APICS)
> -#define physids_shift_right(d, s, n)  bitmap_shift_right((d).mask,
(s).mask,
> n, MAX_APICS)
> -#define physids_shift_left(d, s, n)  bitmap_shift_left((d).mask, (s).mask,
n,
> MAX_APICS)
> -#define physids_coerce(map)   ((map).mask[0])
> -
> -#define physids_promote(physids)      \
> - ({         \
> -  physid_mask_t __physid_mask = PHYSID_MASK_NONE;   \
> -  __physid_mask.mask[0] = physids;    \
> -  __physid_mask;       \
> - })
> -
> -#define physid_mask_of_physid(physid)      \
> - ({         \
> -  physid_mask_t __physid_mask = PHYSID_MASK_NONE;   \
> -  physid_set(physid, __physid_mask);    \
> -  __physid_mask;       \
> - })
>  
>  #define PHYSID_MASK_ALL  { {[0 ... PHYSID_ARRAY_SIZE-1] = ~0UL} }
>  #define PHYSID_MASK_NONE { {[0 ... PHYSID_ARRAY_SIZE-1] = 0UL} }
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


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

Xen devel - Oct 2011 - [PATCH] x86: clean up physid_mask_t handling

[Xen-devel] [PATCH] x86: clean up physid_mask_t handling

Re: [Xen-devel] [PATCH] x86: clean up physid_mask_t handling