[PATCH 1/2] x86_32: summit_32, use BAD_APICID

[PATCH 1/2] x86_32: summit_32, use BAD_APICID

[jirislaby]

From: Jiri Slaby <jirislaby@gmail.com>

Use BAD_APICID instead of 0xFF constants in summit_cpu_mask_to_apicid.

Also remove bogus comments about what we actually return.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/summit_32.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/summit_32.c b/arch/x86/kernel/summit_32.c
index 1e733ef..b10133a 100644
--- a/arch/x86/kernel/summit_32.c
+++ b/arch/x86/kernel/summit_32.c
@@ -309,12 +309,10 @@ static inline unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
 	int cpu;
 
 	num_bits_set = cpus_weight(*cpumask);
-	/* Return id to all */
 	if (num_bits_set >= nr_cpu_ids)
-		return 0xFF;
+		return BAD_APICID;
 	/*
-	 * The cpus in the mask must all be on the apic cluster.  If are not
-	 * on the same apicid cluster return default value of target_cpus():
+	 * The cpus in the mask must all be on the apic cluster.
 	 */
 	cpu = first_cpu(*cpumask);
 	apicid = summit_cpu_to_logical_apicid(cpu);
@@ -325,9 +323,9 @@ static inline unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
 
 			if (apicid_cluster(apicid) !=
 					apicid_cluster(new_apicid)) {
-				printk ("%s: Not a valid mask!\n", __func__);
+				printk("%s: Not a valid mask!\n", __func__);
 
-				return 0xFF;
+				return BAD_APICID;
 			}
 			apicid = apicid | new_apicid;
 			cpus_found++;
-- 
1.6.1.3

[PATCH 2/2] x86_32: summit_32, de-inline functions

[jirislaby]

From: Jiri Slaby <jirislaby@gmail.com>

The ones which go only into struct genapic are de-inlined
by compiler anyway, so remove the inline specifier from them.

Afterwards, remove summit_setup_portio_remap completely as it
is unused.

Remove inline also from summit_cpu_mask_to_apicid, since it's
not worth it (it is used in struct genapic too).

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/summit_32.c |   47 ++++++++++++++++++------------------------
 1 files changed, 20 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kernel/summit_32.c b/arch/x86/kernel/summit_32.c
index b10133a..1d3fe85 100644
--- a/arch/x86/kernel/summit_32.c
+++ b/arch/x86/kernel/summit_32.c
@@ -50,7 +50,7 @@
 #include <linux/gfp.h>
 #include <linux/smp.h>
 
-static inline unsigned summit_get_apic_id(unsigned long x)
+static unsigned summit_get_apic_id(unsigned long x)
 {
 	return (x >> 24) & 0xFF;
 }
@@ -60,7 +60,7 @@ static inline void summit_send_IPI_mask(const cpumask_t *mask, int vector)
 	default_send_IPI_mask_sequence_logical(mask, vector);
 }
 
-static inline void summit_send_IPI_allbutself(int vector)
+static void summit_send_IPI_allbutself(int vector)
 {
 	cpumask_t mask = cpu_online_map;
 	cpu_clear(smp_processor_id(), mask);
@@ -69,7 +69,7 @@ static inline void summit_send_IPI_allbutself(int vector)
 		summit_send_IPI_mask(&mask, vector);
 }
 
-static inline void summit_send_IPI_all(int vector)
+static void summit_send_IPI_all(int vector)
 {
 	summit_send_IPI_mask(&cpu_online_map, vector);
 }
@@ -84,8 +84,8 @@ extern void setup_summit(void);
 #define setup_summit()	{}
 #endif
 
-static inline int
-summit_mps_oem_check(struct mpc_table *mpc, char *oem, char *productid)
+static int summit_mps_oem_check(struct mpc_table *mpc, char *oem,
+		char *productid)
 {
 	if (!strncmp(oem, "IBM ENSW", 8) &&
 			(!strncmp(productid, "VIGIL SMP", 9)
@@ -100,7 +100,7 @@ summit_mps_oem_check(struct mpc_table *mpc, char *oem, char *productid)
 }
 
 /* Hook from generic ACPI tables.c */
-static inline int summit_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
+static int summit_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 {
 	if (!strncmp(oem_id, "IBM", 3) &&
 	    (!strncmp(oem_table_id, "SERVIGIL", 8)
@@ -188,7 +188,7 @@ static inline int is_WPEG(struct rio_detail *rio){
 
 #define SUMMIT_APIC_DFR_VALUE	(APIC_DFR_CLUSTER)
 
-static inline const cpumask_t *summit_target_cpus(void)
+static const cpumask_t *summit_target_cpus(void)
 {
 	/* CPU_MASK_ALL (0xff) has undefined behaviour with
 	 * dest_LowestPrio mode logical clustered apic interrupt routing
@@ -197,14 +197,13 @@ static inline const cpumask_t *summit_target_cpus(void)
 	return &cpumask_of_cpu(0);
 }
 
-static inline unsigned long
-summit_check_apicid_used(physid_mask_t bitmap, int apicid)
+static unsigned long summit_check_apicid_used(physid_mask_t bitmap, int apicid)
 {
 	return 0;
 }
 
 /* we don't use the phys_cpu_present_map to indicate apicid presence */
-static inline unsigned long summit_check_apicid_present(int bit)
+static unsigned long summit_check_apicid_present(int bit)
 {
 	return 1;
 }
@@ -213,7 +212,7 @@ static inline unsigned long summit_check_apicid_present(int bit)
 
 extern u8 cpu_2_logical_apicid[];
 
-static inline void summit_init_apic_ldr(void)
+static void summit_init_apic_ldr(void)
 {
 	unsigned long val, id;
 	int count = 0;
@@ -240,18 +239,18 @@ static inline void summit_init_apic_ldr(void)
 	apic_write(APIC_LDR, val);
 }
 
-static inline int summit_apic_id_registered(void)
+static int summit_apic_id_registered(void)
 {
 	return 1;
 }
 
-static inline void summit_setup_apic_routing(void)
+static void summit_setup_apic_routing(void)
 {
 	printk("Enabling APIC mode:  Summit.  Using %d I/O APICs\n",
 						nr_ioapics);
 }
 
-static inline int summit_apicid_to_node(int logical_apicid)
+static int summit_apicid_to_node(int logical_apicid)
 {
 #ifdef CONFIG_SMP
 	return apicid_2_node[hard_smp_processor_id()];
@@ -272,7 +271,7 @@ static inline int summit_cpu_to_logical_apicid(int cpu)
 #endif
 }
 
-static inline int summit_cpu_present_to_apicid(int mps_cpu)
+static int summit_cpu_present_to_apicid(int mps_cpu)
 {
 	if (mps_cpu < nr_cpu_ids)
 		return (int)per_cpu(x86_bios_cpu_apicid, mps_cpu);
@@ -280,28 +279,23 @@ static inline int summit_cpu_present_to_apicid(int mps_cpu)
 		return BAD_APICID;
 }
 
-static inline physid_mask_t
-summit_ioapic_phys_id_map(physid_mask_t phys_id_map)
+static physid_mask_t summit_ioapic_phys_id_map(physid_mask_t phys_id_map)
 {
 	/* For clustered we don't have a good way to do this yet - hack */
 	return physids_promote(0x0F);
 }
 
-static inline physid_mask_t summit_apicid_to_cpu_present(int apicid)
+static physid_mask_t summit_apicid_to_cpu_present(int apicid)
 {
 	return physid_mask_of_physid(0);
 }
 
-static inline void summit_setup_portio_remap(void)
-{
-}
-
-static inline int summit_check_phys_apicid_present(int boot_cpu_physical_apicid)
+static int summit_check_phys_apicid_present(int boot_cpu_physical_apicid)
 {
 	return 1;
 }
 
-static inline unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
+static unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
 {
 	int cpus_found = 0;
 	int num_bits_set;
@@ -335,8 +329,7 @@ static inline unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
 	return apicid;
 }
 
-static inline unsigned int
-summit_cpu_mask_to_apicid_and(const struct cpumask *inmask,
+static unsigned int summit_cpu_mask_to_apicid_and(const struct cpumask *inmask,
 			      const struct cpumask *andmask)
 {
 	int apicid = summit_cpu_to_logical_apicid(0);
@@ -361,7 +354,7 @@ summit_cpu_mask_to_apicid_and(const struct cpumask *inmask,
  *
  * See Intel's IA-32 SW Dev's Manual Vol2 under CPUID.
  */
-static inline int summit_phys_pkg_id(int cpuid_apic, int index_msb)
+static int summit_phys_pkg_id(int cpuid_apic, int index_msb)
 {
 	return hard_smp_processor_id() >> index_msb;
 }
-- 
1.6.1.3

Re: [PATCH 1/2] x86_32: summit_32, use BAD_APICID

[Ingo Molnar]

* jirislaby@gmail.com <jirislaby@gmail.com> wrote:

> From: Jiri Slaby <jirislaby@gmail.com>
> 
> Use BAD_APICID instead of 0xFF constants in summit_cpu_mask_to_apicid.
> 
> Also remove bogus comments about what we actually return.

Hm, please send x86 patches against the latest x86 tree:

  http://people.redhat.com/mingo/tip.git/README

the Summit code moved to another position in the tree so your 
patches dont apply anymore.

	Ingo

[PATCH v2 1/2] x86_32: summit_32, use BAD_APICID

[Jiri Slaby]

Use BAD_APICID instead of 0xFF constants in summit_cpu_mask_to_apicid.

Also remove bogus comments about what we actually return.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/apic/summit_32.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/apic/summit_32.c b/arch/x86/kernel/apic/summit_32.c
index cfe7b09..d02f438 100644
--- a/arch/x86/kernel/apic/summit_32.c
+++ b/arch/x86/kernel/apic/summit_32.c
@@ -303,12 +303,10 @@ static inline unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
 	int cpu;
 
 	num_bits_set = cpus_weight(*cpumask);
-	/* Return id to all */
 	if (num_bits_set >= nr_cpu_ids)
-		return 0xFF;
+		return BAD_APICID;
 	/*
-	 * The cpus in the mask must all be on the apic cluster.  If are not
-	 * on the same apicid cluster return default value of target_cpus():
+	 * The cpus in the mask must all be on the apic cluster.
 	 */
 	cpu = first_cpu(*cpumask);
 	apicid = summit_cpu_to_logical_apicid(cpu);
@@ -318,9 +316,9 @@ static inline unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
 			int new_apicid = summit_cpu_to_logical_apicid(cpu);
 
 			if (APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
-				printk ("%s: Not a valid mask!\n", __func__);
+				printk("%s: Not a valid mask!\n", __func__);
 
-				return 0xFF;
+				return BAD_APICID;
 			}
 			apicid = apicid | new_apicid;
 			cpus_found++;
-- 
1.6.1.3

[PATCH v2 2/2] x86_32: summit_32, de-inline functions

[Jiri Slaby]

The ones which go only into struct genapic are de-inlined
by compiler anyway, so remove the inline specifier from them.

Afterwards, remove summit_setup_portio_remap completely as it
is unused.

Remove inline also from summit_cpu_mask_to_apicid, since it's
not worth it (it is used in struct genapic too).

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/apic/summit_32.c |   47 ++++++++++++++++---------------------
 1 files changed, 20 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kernel/apic/summit_32.c b/arch/x86/kernel/apic/summit_32.c
index d02f438..32838b5 100644
--- a/arch/x86/kernel/apic/summit_32.c
+++ b/arch/x86/kernel/apic/summit_32.c
@@ -48,7 +48,7 @@
 #include <linux/gfp.h>
 #include <linux/smp.h>
 
-static inline unsigned summit_get_apic_id(unsigned long x)
+static unsigned summit_get_apic_id(unsigned long x)
 {
 	return (x >> 24) & 0xFF;
 }
@@ -58,7 +58,7 @@ static inline void summit_send_IPI_mask(const cpumask_t *mask, int vector)
 	default_send_IPI_mask_sequence_logical(mask, vector);
 }
 
-static inline void summit_send_IPI_allbutself(int vector)
+static void summit_send_IPI_allbutself(int vector)
 {
 	cpumask_t mask = cpu_online_map;
 	cpu_clear(smp_processor_id(), mask);
@@ -67,7 +67,7 @@ static inline void summit_send_IPI_allbutself(int vector)
 		summit_send_IPI_mask(&mask, vector);
 }
 
-static inline void summit_send_IPI_all(int vector)
+static void summit_send_IPI_all(int vector)
 {
 	summit_send_IPI_mask(&cpu_online_map, vector);
 }
@@ -82,8 +82,8 @@ extern void setup_summit(void);
 #define setup_summit()	{}
 #endif
 
-static inline int
-summit_mps_oem_check(struct mpc_table *mpc, char *oem, char *productid)
+static int summit_mps_oem_check(struct mpc_table *mpc, char *oem,
+		char *productid)
 {
 	if (!strncmp(oem, "IBM ENSW", 8) &&
 			(!strncmp(productid, "VIGIL SMP", 9)
@@ -98,7 +98,7 @@ summit_mps_oem_check(struct mpc_table *mpc, char *oem, char *productid)
 }
 
 /* Hook from generic ACPI tables.c */
-static inline int summit_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
+static int summit_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 {
 	if (!strncmp(oem_id, "IBM", 3) &&
 	    (!strncmp(oem_table_id, "SERVIGIL", 8)
@@ -186,7 +186,7 @@ static inline int is_WPEG(struct rio_detail *rio){
 
 #define SUMMIT_APIC_DFR_VALUE	(APIC_DFR_CLUSTER)
 
-static inline const cpumask_t *summit_target_cpus(void)
+static const cpumask_t *summit_target_cpus(void)
 {
 	/* CPU_MASK_ALL (0xff) has undefined behaviour with
 	 * dest_LowestPrio mode logical clustered apic interrupt routing
@@ -195,19 +195,18 @@ static inline const cpumask_t *summit_target_cpus(void)
 	return &cpumask_of_cpu(0);
 }
 
-static inline unsigned long
-summit_check_apicid_used(physid_mask_t bitmap, int apicid)
+static unsigned long summit_check_apicid_used(physid_mask_t bitmap, int apicid)
 {
 	return 0;
 }
 
 /* we don't use the phys_cpu_present_map to indicate apicid presence */
-static inline unsigned long summit_check_apicid_present(int bit)
+static unsigned long summit_check_apicid_present(int bit)
 {
 	return 1;
 }
 
-static inline void summit_init_apic_ldr(void)
+static void summit_init_apic_ldr(void)
 {
 	unsigned long val, id;
 	int count = 0;
@@ -234,18 +233,18 @@ static inline void summit_init_apic_ldr(void)
 	apic_write(APIC_LDR, val);
 }
 
-static inline int summit_apic_id_registered(void)
+static int summit_apic_id_registered(void)
 {
 	return 1;
 }
 
-static inline void summit_setup_apic_routing(void)
+static void summit_setup_apic_routing(void)
 {
 	printk("Enabling APIC mode:  Summit.  Using %d I/O APICs\n",
 						nr_ioapics);
 }
 
-static inline int summit_apicid_to_node(int logical_apicid)
+static int summit_apicid_to_node(int logical_apicid)
 {
 #ifdef CONFIG_SMP
 	return apicid_2_node[hard_smp_processor_id()];
@@ -266,7 +265,7 @@ static inline int summit_cpu_to_logical_apicid(int cpu)
 #endif
 }
 
-static inline int summit_cpu_present_to_apicid(int mps_cpu)
+static int summit_cpu_present_to_apicid(int mps_cpu)
 {
 	if (mps_cpu < nr_cpu_ids)
 		return (int)per_cpu(x86_bios_cpu_apicid, mps_cpu);
@@ -274,28 +273,23 @@ static inline int summit_cpu_present_to_apicid(int mps_cpu)
 		return BAD_APICID;
 }
 
-static inline physid_mask_t
-summit_ioapic_phys_id_map(physid_mask_t phys_id_map)
+static physid_mask_t summit_ioapic_phys_id_map(physid_mask_t phys_id_map)
 {
 	/* For clustered we don't have a good way to do this yet - hack */
 	return physids_promote(0x0F);
 }
 
-static inline physid_mask_t summit_apicid_to_cpu_present(int apicid)
+static physid_mask_t summit_apicid_to_cpu_present(int apicid)
 {
 	return physid_mask_of_physid(0);
 }
 
-static inline void summit_setup_portio_remap(void)
-{
-}
-
-static inline int summit_check_phys_apicid_present(int boot_cpu_physical_apicid)
+static int summit_check_phys_apicid_present(int boot_cpu_physical_apicid)
 {
 	return 1;
 }
 
-static inline unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
+static unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
 {
 	int cpus_found = 0;
 	int num_bits_set;
@@ -328,8 +322,7 @@ static inline unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
 	return apicid;
 }
 
-static inline unsigned int
-summit_cpu_mask_to_apicid_and(const struct cpumask *inmask,
+static unsigned int summit_cpu_mask_to_apicid_and(const struct cpumask *inmask,
 			      const struct cpumask *andmask)
 {
 	int apicid = summit_cpu_to_logical_apicid(0);
@@ -354,7 +347,7 @@ summit_cpu_mask_to_apicid_and(const struct cpumask *inmask,
  *
  * See Intel's IA-32 SW Dev's Manual Vol2 under CPUID.
  */
-static inline int summit_phys_pkg_id(int cpuid_apic, int index_msb)
+static int summit_phys_pkg_id(int cpuid_apic, int index_msb)
 {
 	return hard_smp_processor_id() >> index_msb;
 }
-- 
1.6.1.3

Re: [PATCH v2 1/2] x86_32: summit_32, use BAD_APICID

[Jiri Slaby]

On 24.2.2009 21:41, Jiri Slaby wrote:
> --- a/arch/x86/kernel/apic/summit_32.c
> +++ b/arch/x86/kernel/apic/summit_32.c
> @@ -303,12 +303,10 @@ static inline unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
>   	int cpu;
>
>   	num_bits_set = cpus_weight(*cpumask);
> -	/* Return id to all */
>   	if (num_bits_set>= nr_cpu_ids)
> -		return 0xFF;
> +		return BAD_APICID;

Actually, wasn't this intended to return all available rather than BAD 
(to which would correspond the comment), but performed in a wrong 
manner? This is old code, any ideas who would know this?

In my opinion the check should be removed completely to allow the code 
go through same-cluster check.

In that case, the callers code is buggy, since it passes online_cpu 
masks even on machines, where apics are not on the same clusters.

Re: [PATCH v2 1/2] x86_32: summit_32, use BAD_APICID

[Ingo Molnar]

* Jiri Slaby <jirislaby@gmail.com> wrote:

> On 24.2.2009 21:41, Jiri Slaby wrote:
>> --- a/arch/x86/kernel/apic/summit_32.c
>> +++ b/arch/x86/kernel/apic/summit_32.c
>> @@ -303,12 +303,10 @@ static inline unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
>>   	int cpu;
>>
>>   	num_bits_set = cpus_weight(*cpumask);
>> -	/* Return id to all */
>>   	if (num_bits_set>= nr_cpu_ids)
>> -		return 0xFF;
>> +		return BAD_APICID;
>
> Actually, wasn't this intended to return all available rather 
> than BAD (to which would correspond the comment), but 
> performed in a wrong manner? This is old code, any ideas who 
> would know this?
>
> In my opinion the check should be removed completely to allow 
> the code go through same-cluster check.
>
> In that case, the callers code is buggy, since it passes 
> online_cpu masks even on machines, where apics are not on the 
> same clusters.

It's most likely confusion in the old code. This used to be 
copy&paste-ed versions of different snapshots of the 
mach-default-code, hacked to make work on weird platforms. 
Mainline fixes/updates werent merged in consistently.

So could you please send a patch that fixes this?

	Ingo

[PATCH 1/4] x86_32: apic/bigsmp_32, de-inline functions

[Jiri Slaby]

The ones which go only into struct apic are de-inlined
by compiler anyway, so remove the inline specifier from them.

Afterwards, remove bigsmp_setup_portio_remap completely as it
is unused.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/apic/bigsmp_32.c |   40 ++++++++++++++++---------------------
 1 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/arch/x86/kernel/apic/bigsmp_32.c b/arch/x86/kernel/apic/bigsmp_32.c
index 0b10933..d14e635 100644
--- a/arch/x86/kernel/apic/bigsmp_32.c
+++ b/arch/x86/kernel/apic/bigsmp_32.c
@@ -16,17 +16,17 @@
 #include <asm/apic.h>
 #include <asm/ipi.h>
 
-static inline unsigned bigsmp_get_apic_id(unsigned long x)
+static unsigned bigsmp_get_apic_id(unsigned long x)
 {
 	return (x >> 24) & 0xFF;
 }
 
-static inline int bigsmp_apic_id_registered(void)
+static int bigsmp_apic_id_registered(void)
 {
 	return 1;
 }
 
-static inline const cpumask_t *bigsmp_target_cpus(void)
+static const cpumask_t *bigsmp_target_cpus(void)
 {
 #ifdef CONFIG_SMP
 	return &cpu_online_map;
@@ -35,13 +35,12 @@ static inline const cpumask_t *bigsmp_target_cpus(void)
 #endif
 }
 
-static inline unsigned long
-bigsmp_check_apicid_used(physid_mask_t bitmap, int apicid)
+static unsigned long bigsmp_check_apicid_used(physid_mask_t bitmap, int apicid)
 {
 	return 0;
 }
 
-static inline unsigned long bigsmp_check_apicid_present(int bit)
+static unsigned long bigsmp_check_apicid_present(int bit)
 {
 	return 1;
 }
@@ -64,7 +63,7 @@ static inline unsigned long calculate_ldr(int cpu)
  * an APIC.  See e.g. "AP-388 82489DX User's Manual" (Intel
  * document number 292116).  So here it goes...
  */
-static inline void bigsmp_init_apic_ldr(void)
+static void bigsmp_init_apic_ldr(void)
 {
 	unsigned long val;
 	int cpu = smp_processor_id();
@@ -74,19 +73,19 @@ static inline void bigsmp_init_apic_ldr(void)
 	apic_write(APIC_LDR, val);
 }
 
-static inline void bigsmp_setup_apic_routing(void)
+static void bigsmp_setup_apic_routing(void)
 {
 	printk(KERN_INFO
 		"Enabling APIC mode:  Physflat.  Using %d I/O APICs\n",
 		nr_ioapics);
 }
 
-static inline int bigsmp_apicid_to_node(int logical_apicid)
+static int bigsmp_apicid_to_node(int logical_apicid)
 {
 	return apicid_2_node[hard_smp_processor_id()];
 }
 
-static inline int bigsmp_cpu_present_to_apicid(int mps_cpu)
+static int bigsmp_cpu_present_to_apicid(int mps_cpu)
 {
 	if (mps_cpu < nr_cpu_ids)
 		return (int) per_cpu(x86_bios_cpu_apicid, mps_cpu);
@@ -94,7 +93,7 @@ static inline int bigsmp_cpu_present_to_apicid(int mps_cpu)
 	return BAD_APICID;
 }
 
-static inline physid_mask_t bigsmp_apicid_to_cpu_present(int phys_apicid)
+static physid_mask_t bigsmp_apicid_to_cpu_present(int phys_apicid)
 {
 	return physid_mask_of_physid(phys_apicid);
 }
@@ -107,29 +106,24 @@ static inline int bigsmp_cpu_to_logical_apicid(int cpu)
 	return cpu_physical_id(cpu);
 }
 
-static inline physid_mask_t bigsmp_ioapic_phys_id_map(physid_mask_t phys_map)
+static physid_mask_t bigsmp_ioapic_phys_id_map(physid_mask_t phys_map)
 {
 	/* For clustered we don't have a good way to do this yet - hack */
 	return physids_promote(0xFFL);
 }
 
-static inline void bigsmp_setup_portio_remap(void)
-{
-}
-
-static inline int bigsmp_check_phys_apicid_present(int boot_cpu_physical_apicid)
+static int bigsmp_check_phys_apicid_present(int boot_cpu_physical_apicid)
 {
 	return 1;
 }
 
 /* As we are using single CPU as destination, pick only one CPU here */
-static inline unsigned int bigsmp_cpu_mask_to_apicid(const cpumask_t *cpumask)
+static unsigned int bigsmp_cpu_mask_to_apicid(const cpumask_t *cpumask)
 {
 	return bigsmp_cpu_to_logical_apicid(first_cpu(*cpumask));
 }
 
-static inline unsigned int
-bigsmp_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
+static unsigned int bigsmp_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
 			      const struct cpumask *andmask)
 {
 	int cpu;
@@ -148,7 +142,7 @@ bigsmp_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
 	return BAD_APICID;
 }
 
-static inline int bigsmp_phys_pkg_id(int cpuid_apic, int index_msb)
+static int bigsmp_phys_pkg_id(int cpuid_apic, int index_msb)
 {
 	return cpuid_apic >> index_msb;
 }
@@ -158,12 +152,12 @@ static inline void bigsmp_send_IPI_mask(const struct cpumask *mask, int vector)
 	default_send_IPI_mask_sequence_phys(mask, vector);
 }
 
-static inline void bigsmp_send_IPI_allbutself(int vector)
+static void bigsmp_send_IPI_allbutself(int vector)
 {
 	default_send_IPI_mask_allbutself_phys(cpu_online_mask, vector);
 }
 
-static inline void bigsmp_send_IPI_all(int vector)
+static void bigsmp_send_IPI_all(int vector)
 {
 	bigsmp_send_IPI_mask(cpu_online_mask, vector);
 }
-- 
1.6.1.3

[PATCH 2/4] x86_32: apic/es7000_32, cpu_mask_to_apicid cleanup

[Jiri Slaby]

Remove es7000_cpu_mask_to_apicid_cluster completely, because it's
almost the same as es7000_cpu_mask_to_apicid except 2 code paths.
One of them is about to be removed soon, the another should be
BAD_APICID (it's a fail path).

The _cluster one was not invoked on apic->cpu_mask_to_apicid_and
anyway, since there was no _cluster_and variant.

Also use newer cpumask functions.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/apic/es7000_32.c |   46 ++-----------------------------------
 1 files changed, 3 insertions(+), 43 deletions(-)

diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index 320f2d2..6c15126 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -565,25 +565,21 @@ static int es7000_check_phys_apicid_present(int cpu_physical_apicid)
 	return 1;
 }
 
-static unsigned int
-es7000_cpu_mask_to_apicid_cluster(const struct cpumask *cpumask)
+static unsigned int es7000_cpu_mask_to_apicid(const cpumask_t *cpumask)
 {
-	int cpus_found = 0;
-	int num_bits_set;
+	unsigned int cpu, num_bits_set, cpus_found = 0;
 	int apicid;
-	int cpu;
 
 	num_bits_set = cpumask_weight(cpumask);
 	/* Return id to all */
 	if (num_bits_set == nr_cpu_ids)
-		return 0xFF;
+		return es7000_cpu_to_logical_apicid(0);
 	/*
 	 * The cpus in the mask must all be on the apic cluster.  If are not
 	 * on the same apicid cluster return default value of target_cpus():
 	 */
 	cpu = cpumask_first(cpumask);
 	apicid = es7000_cpu_to_logical_apicid(cpu);
-
 	while (cpus_found < num_bits_set) {
 		if (cpumask_test_cpu(cpu, cpumask)) {
 			int new_apicid = es7000_cpu_to_logical_apicid(cpu);
@@ -591,40 +587,6 @@ es7000_cpu_mask_to_apicid_cluster(const struct cpumask *cpumask)
 			if (APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
 				WARN(1, "Not a valid mask!");
 
-				return 0xFF;
-			}
-			apicid = new_apicid;
-			cpus_found++;
-		}
-		cpu++;
-	}
-	return apicid;
-}
-
-static unsigned int es7000_cpu_mask_to_apicid(const cpumask_t *cpumask)
-{
-	int cpus_found = 0;
-	int num_bits_set;
-	int apicid;
-	int cpu;
-
-	num_bits_set = cpus_weight(*cpumask);
-	/* Return id to all */
-	if (num_bits_set == nr_cpu_ids)
-		return es7000_cpu_to_logical_apicid(0);
-	/*
-	 * The cpus in the mask must all be on the apic cluster.  If are not
-	 * on the same apicid cluster return default value of target_cpus():
-	 */
-	cpu = first_cpu(*cpumask);
-	apicid = es7000_cpu_to_logical_apicid(cpu);
-	while (cpus_found < num_bits_set) {
-		if (cpu_isset(cpu, *cpumask)) {
-			int new_apicid = es7000_cpu_to_logical_apicid(cpu);
-
-			if (APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
-				printk("%s: Not a valid mask!\n", __func__);
-
 				return es7000_cpu_to_logical_apicid(0);
 			}
 			apicid = new_apicid;
@@ -667,8 +629,6 @@ void __init es7000_update_apic_to_cluster(void)
 	apic->irq_dest_mode = 1;
 
 	apic->init_apic_ldr = es7000_init_apic_ldr_cluster;
-
-	apic->cpu_mask_to_apicid = es7000_cpu_mask_to_apicid_cluster;
 }
 
 static int probe_es7000(void)
-- 
1.6.1.3

[PATCH 3/4] x86_32: apic/es7000_32, fix cpu_mask_to_apicid

[Jiri Slaby]

Perform same-cluster checking even for masks with all (nr_cpu_ids)
bits set and report BAD_APICID on failure.

While at it, convert it to for_each_cpu.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/apic/es7000_32.c |   31 +++++++++++--------------------
 1 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index 6c15126..bbf85b6 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -567,32 +567,23 @@ static int es7000_check_phys_apicid_present(int cpu_physical_apicid)
 
 static unsigned int es7000_cpu_mask_to_apicid(const cpumask_t *cpumask)
 {
-	unsigned int cpu, num_bits_set, cpus_found = 0;
-	int apicid;
+	unsigned int round = 0;
+	int cpu, apicid;
 
-	num_bits_set = cpumask_weight(cpumask);
-	/* Return id to all */
-	if (num_bits_set == nr_cpu_ids)
-		return es7000_cpu_to_logical_apicid(0);
 	/*
-	 * The cpus in the mask must all be on the apic cluster.  If are not
-	 * on the same apicid cluster return default value of target_cpus():
+	 * The cpus in the mask must all be on the apic cluster.
 	 */
-	cpu = cpumask_first(cpumask);
-	apicid = es7000_cpu_to_logical_apicid(cpu);
-	while (cpus_found < num_bits_set) {
-		if (cpumask_test_cpu(cpu, cpumask)) {
-			int new_apicid = es7000_cpu_to_logical_apicid(cpu);
+	apicid = es7000_cpu_to_logical_apicid(cpumask_first(cpumask));
+	for_each_cpu(cpu, cpumask) {
+		int new_apicid = es7000_cpu_to_logical_apicid(cpu);
 
-			if (APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
-				WARN(1, "Not a valid mask!");
+		if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
+			WARN(1, "Not a valid mask!");
 
-				return es7000_cpu_to_logical_apicid(0);
-			}
-			apicid = new_apicid;
-			cpus_found++;
+			return BAD_APICID;
 		}
-		cpu++;
+		apicid = new_apicid;
+		round++;
 	}
 	return apicid;
 }
-- 
1.6.1.3

[PATCH 4/4] x86_32: apic/summit_32, fix cpu_mask_to_apicid

[Jiri Slaby]

Perform same-cluster checking even for masks with all (nr_cpu_ids)
bits set and report correct apicid on success instead.

While at it, convert it to for_each_cpu and newer cpumask api.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/apic/summit_32.c |   30 +++++++++---------------------
 1 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kernel/apic/summit_32.c b/arch/x86/kernel/apic/summit_32.c
index 32838b5..95916dc 100644
--- a/arch/x86/kernel/apic/summit_32.c
+++ b/arch/x86/kernel/apic/summit_32.c
@@ -291,33 +291,21 @@ static int summit_check_phys_apicid_present(int boot_cpu_physical_apicid)
 
 static unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
 {
-	int cpus_found = 0;
-	int num_bits_set;
-	int apicid;
-	int cpu;
+	unsigned int round = 0;
+	int cpu, apicid = 0;
 
-	num_bits_set = cpus_weight(*cpumask);
-	if (num_bits_set >= nr_cpu_ids)
-		return BAD_APICID;
 	/*
 	 * The cpus in the mask must all be on the apic cluster.
 	 */
-	cpu = first_cpu(*cpumask);
-	apicid = summit_cpu_to_logical_apicid(cpu);
-
-	while (cpus_found < num_bits_set) {
-		if (cpu_isset(cpu, *cpumask)) {
-			int new_apicid = summit_cpu_to_logical_apicid(cpu);
+	for_each_cpu(cpu, cpumask) {
+		int new_apicid = summit_cpu_to_logical_apicid(cpu);
 
-			if (APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
-				printk("%s: Not a valid mask!\n", __func__);
-
-				return BAD_APICID;
-			}
-			apicid = apicid | new_apicid;
-			cpus_found++;
+		if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
+			printk("%s: Not a valid mask!\n", __func__);
+			return BAD_APICID;
 		}
-		cpu++;
+		apicid |= new_apicid;
+		round++;
 	}
 	return apicid;
 }
-- 
1.6.1.3

[PATCH v2 3/4] x86_32: apic/es7000_32, fix cpu_mask_to_apicid

[Jiri Slaby]

Perform same-cluster checking even for masks with all (nr_cpu_ids)
bits set and report BAD_APICID on failure.

While at it, convert it to for_each_cpu.

[v2]
- remove omitted apicid initial assignment

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/apic/es7000_32.c |   30 ++++++++++--------------------
 1 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index 6c15126..d2ab096 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -567,32 +567,22 @@ static int es7000_check_phys_apicid_present(int cpu_physical_apicid)
 
 static unsigned int es7000_cpu_mask_to_apicid(const cpumask_t *cpumask)
 {
-	unsigned int cpu, num_bits_set, cpus_found = 0;
-	int apicid;
+	unsigned int round = 0;
+	int cpu, uninitialized_var(apicid);
 
-	num_bits_set = cpumask_weight(cpumask);
-	/* Return id to all */
-	if (num_bits_set == nr_cpu_ids)
-		return es7000_cpu_to_logical_apicid(0);
 	/*
-	 * The cpus in the mask must all be on the apic cluster.  If are not
-	 * on the same apicid cluster return default value of target_cpus():
+	 * The cpus in the mask must all be on the apic cluster.
 	 */
-	cpu = cpumask_first(cpumask);
-	apicid = es7000_cpu_to_logical_apicid(cpu);
-	while (cpus_found < num_bits_set) {
-		if (cpumask_test_cpu(cpu, cpumask)) {
-			int new_apicid = es7000_cpu_to_logical_apicid(cpu);
+	for_each_cpu(cpu, cpumask) {
+		int new_apicid = es7000_cpu_to_logical_apicid(cpu);
 
-			if (APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
-				WARN(1, "Not a valid mask!");
+		if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
+			WARN(1, "Not a valid mask!");
 
-				return es7000_cpu_to_logical_apicid(0);
-			}
-			apicid = new_apicid;
-			cpus_found++;
+			return BAD_APICID;
 		}
-		cpu++;
+		apicid = new_apicid;
+		round++;
 	}
 	return apicid;
 }
-- 
1.6.1.3

Re: [PATCH v2 1/2] x86_32: summit_32, use BAD_APICID

[Jiri Slaby]

On 25.2.2009 12:10, Ingo Molnar wrote:
> * Jiri Slaby<jirislaby@gmail.com>  wrote:
>> In that case, the callers code is buggy, since it passes
>> online_cpu masks even on machines, where apics are not on the
>> same clusters.
>
> It's most likely confusion in the old code. This used to be
> copy&paste-ed versions of different snapshots of the
> mach-default-code, hacked to make work on weird platforms.
> Mainline fixes/updates werent merged in consistently.
>
> So could you please send a patch that fixes this?

I've sent 4 more patches, but there are still issues:
* es7000 + summit: I haven't solved calling with all bits set (only all 
online is sufficient to trigger this). Some of the processors needn't be 
on the same apic cluster. It will scream now (again -- it did before 
adding the "optimisation"). Actually I don't know how to solve this. How 
the caller would know the correct mask, ANDing with a apic->target_cpus 
retval?

* es7000: target_cpus_cluster returns CPU_MASK_ALL and hence it will 
choke itself, because es7000_cpu_mask_to_apicid doesn't count with that. 
Invoked by setup_timer_IRQ0_pin this way.

* set_desc_affinity doesn't expect apic->cpu_mask_to_apicid_and to 
return BAD_APICID and silently sets desc->affinity. Again, I see no 
straightforward solution (rollback of assign_irq_vector and 
set_extra_move_desc needed).

Re: [PATCH v2 1/2] x86_32: summit_32, use BAD_APICID

[Ingo Molnar]

* Jiri Slaby <jirislaby@gmail.com> wrote:

> On 25.2.2009 12:10, Ingo Molnar wrote:
>> * Jiri Slaby<jirislaby@gmail.com>  wrote:
>>> In that case, the callers code is buggy, since it passes
>>> online_cpu masks even on machines, where apics are not on the
>>> same clusters.
>>
>> It's most likely confusion in the old code. This used to be
>> copy&paste-ed versions of different snapshots of the
>> mach-default-code, hacked to make work on weird platforms.
>> Mainline fixes/updates werent merged in consistently.
>>
>> So could you please send a patch that fixes this?
>
> I've sent 4 more patches, but there are still issues:
> * es7000 + summit: I haven't solved calling with all bits set (only all  
> online is sufficient to trigger this). Some of the processors needn't be  
> on the same apic cluster. It will scream now (again -- it did before  
> adding the "optimisation"). Actually I don't know how to solve this. How  
> the caller would know the correct mask, ANDing with a apic->target_cpus  
> retval?
>
> * es7000: target_cpus_cluster returns CPU_MASK_ALL and hence it will  
> choke itself, because es7000_cpu_mask_to_apicid doesn't count with that.  
> Invoked by setup_timer_IRQ0_pin this way.
>
> * set_desc_affinity doesn't expect apic->cpu_mask_to_apicid_and to  
> return BAD_APICID and silently sets desc->affinity. Again, I see no  
> straightforward solution (rollback of assign_irq_vector and  
> set_extra_move_desc needed).

Ok, they look good. I got a conflict in 2/4, due to an 
interacting cleanup from Yinghai - mind resending them against 
latest tip:master?

Thanks,

	Ingo

[PATCH 1/4] x86_32: apic/bigsmp_32, de-inline functions

[Jiri Slaby]

The ones which go only into struct apic are de-inlined
by compiler anyway, so remove the inline specifier from them.

Afterwards, remove bigsmp_setup_portio_remap completely as it
is unused.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/apic/bigsmp_32.c |   40 ++++++++++++++++---------------------
 1 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/arch/x86/kernel/apic/bigsmp_32.c b/arch/x86/kernel/apic/bigsmp_32.c
index 69c512e..d806eca 100644
--- a/arch/x86/kernel/apic/bigsmp_32.c
+++ b/arch/x86/kernel/apic/bigsmp_32.c
@@ -16,17 +16,17 @@
 #include <asm/apic.h>
 #include <asm/ipi.h>
 
-static inline unsigned bigsmp_get_apic_id(unsigned long x)
+static unsigned bigsmp_get_apic_id(unsigned long x)
 {
 	return (x >> 24) & 0xFF;
 }
 
-static inline int bigsmp_apic_id_registered(void)
+static int bigsmp_apic_id_registered(void)
 {
 	return 1;
 }
 
-static inline const cpumask_t *bigsmp_target_cpus(void)
+static const cpumask_t *bigsmp_target_cpus(void)
 {
 #ifdef CONFIG_SMP
 	return &cpu_online_map;
@@ -35,13 +35,12 @@ static inline const cpumask_t *bigsmp_target_cpus(void)
 #endif
 }
 
-static inline unsigned long
-bigsmp_check_apicid_used(physid_mask_t bitmap, int apicid)
+static unsigned long bigsmp_check_apicid_used(physid_mask_t bitmap, int apicid)
 {
 	return 0;
 }
 
-static inline unsigned long bigsmp_check_apicid_present(int bit)
+static unsigned long bigsmp_check_apicid_present(int bit)
 {
 	return 1;
 }
@@ -64,7 +63,7 @@ static inline unsigned long calculate_ldr(int cpu)
  * an APIC.  See e.g. "AP-388 82489DX User's Manual" (Intel
  * document number 292116).  So here it goes...
  */
-static inline void bigsmp_init_apic_ldr(void)
+static void bigsmp_init_apic_ldr(void)
 {
 	unsigned long val;
 	int cpu = smp_processor_id();
@@ -74,19 +73,19 @@ static inline void bigsmp_init_apic_ldr(void)
 	apic_write(APIC_LDR, val);
 }
 
-static inline void bigsmp_setup_apic_routing(void)
+static void bigsmp_setup_apic_routing(void)
 {
 	printk(KERN_INFO
 		"Enabling APIC mode:  Physflat.  Using %d I/O APICs\n",
 		nr_ioapics);
 }
 
-static inline int bigsmp_apicid_to_node(int logical_apicid)
+static int bigsmp_apicid_to_node(int logical_apicid)
 {
 	return apicid_2_node[hard_smp_processor_id()];
 }
 
-static inline int bigsmp_cpu_present_to_apicid(int mps_cpu)
+static int bigsmp_cpu_present_to_apicid(int mps_cpu)
 {
 	if (mps_cpu < nr_cpu_ids)
 		return (int) per_cpu(x86_bios_cpu_apicid, mps_cpu);
@@ -94,7 +93,7 @@ static inline int bigsmp_cpu_present_to_apicid(int mps_cpu)
 	return BAD_APICID;
 }
 
-static inline physid_mask_t bigsmp_apicid_to_cpu_present(int phys_apicid)
+static physid_mask_t bigsmp_apicid_to_cpu_present(int phys_apicid)
 {
 	return physid_mask_of_physid(phys_apicid);
 }
@@ -107,29 +106,24 @@ static inline int bigsmp_cpu_to_logical_apicid(int cpu)
 	return cpu_physical_id(cpu);
 }
 
-static inline physid_mask_t bigsmp_ioapic_phys_id_map(physid_mask_t phys_map)
+static physid_mask_t bigsmp_ioapic_phys_id_map(physid_mask_t phys_map)
 {
 	/* For clustered we don't have a good way to do this yet - hack */
 	return physids_promote(0xFFL);
 }
 
-static inline void bigsmp_setup_portio_remap(void)
-{
-}
-
-static inline int bigsmp_check_phys_apicid_present(int boot_cpu_physical_apicid)
+static int bigsmp_check_phys_apicid_present(int boot_cpu_physical_apicid)
 {
 	return 1;
 }
 
 /* As we are using single CPU as destination, pick only one CPU here */
-static inline unsigned int bigsmp_cpu_mask_to_apicid(const cpumask_t *cpumask)
+static unsigned int bigsmp_cpu_mask_to_apicid(const cpumask_t *cpumask)
 {
 	return bigsmp_cpu_to_logical_apicid(first_cpu(*cpumask));
 }
 
-static inline unsigned int
-bigsmp_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
+static unsigned int bigsmp_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
 			      const struct cpumask *andmask)
 {
 	int cpu;
@@ -148,7 +142,7 @@ bigsmp_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
 	return BAD_APICID;
 }
 
-static inline int bigsmp_phys_pkg_id(int cpuid_apic, int index_msb)
+static int bigsmp_phys_pkg_id(int cpuid_apic, int index_msb)
 {
 	return cpuid_apic >> index_msb;
 }
@@ -158,12 +152,12 @@ static inline void bigsmp_send_IPI_mask(const struct cpumask *mask, int vector)
 	default_send_IPI_mask_sequence_phys(mask, vector);
 }
 
-static inline void bigsmp_send_IPI_allbutself(int vector)
+static void bigsmp_send_IPI_allbutself(int vector)
 {
 	default_send_IPI_mask_allbutself_phys(cpu_online_mask, vector);
 }
 
-static inline void bigsmp_send_IPI_all(int vector)
+static void bigsmp_send_IPI_all(int vector)
 {
 	bigsmp_send_IPI_mask(cpu_online_mask, vector);
 }
-- 
1.6.1.3

[PATCH 2/4] x86_32: apic/es7000_32, cpu_mask_to_apicid cleanup

[Jiri Slaby]

Remove es7000_cpu_mask_to_apicid_cluster completely, because it's
almost the same as es7000_cpu_mask_to_apicid except 2 code paths.
One of them is about to be removed soon, the another should be
BAD_APICID (it's a fail path).

The _cluster one was not invoked on apic->cpu_mask_to_apicid_and
anyway, since there was no _cluster_and variant.

Also use newer cpumask functions.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/apic/es7000_32.c |   46 +++----------------------------------
 1 files changed, 4 insertions(+), 42 deletions(-)

diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index da37e2c..9b9e86f 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -575,25 +575,21 @@ static int es7000_check_phys_apicid_present(int cpu_physical_apicid)
 	return 1;
 }
 
-static unsigned int
-es7000_cpu_mask_to_apicid_cluster(const struct cpumask *cpumask)
+static unsigned int es7000_cpu_mask_to_apicid(const cpumask_t *cpumask)
 {
-	int cpus_found = 0;
-	int num_bits_set;
+	unsigned int cpu, num_bits_set, cpus_found = 0;
 	int apicid;
-	int cpu;
 
 	num_bits_set = cpumask_weight(cpumask);
 	/* Return id to all */
 	if (num_bits_set == nr_cpu_ids)
-		return 0xFF;
+		return es7000_cpu_to_logical_apicid(0);
 	/*
 	 * The cpus in the mask must all be on the apic cluster.  If are not
 	 * on the same apicid cluster return default value of target_cpus():
 	 */
 	cpu = cpumask_first(cpumask);
 	apicid = es7000_cpu_to_logical_apicid(cpu);
-
 	while (cpus_found < num_bits_set) {
 		if (cpumask_test_cpu(cpu, cpumask)) {
 			int new_apicid = es7000_cpu_to_logical_apicid(cpu);
@@ -601,40 +597,6 @@ es7000_cpu_mask_to_apicid_cluster(const struct cpumask *cpumask)
 			if (APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
 				WARN(1, "Not a valid mask!");
 
-				return 0xFF;
-			}
-			apicid = new_apicid;
-			cpus_found++;
-		}
-		cpu++;
-	}
-	return apicid;
-}
-
-static unsigned int es7000_cpu_mask_to_apicid(const cpumask_t *cpumask)
-{
-	int cpus_found = 0;
-	int num_bits_set;
-	int apicid;
-	int cpu;
-
-	num_bits_set = cpus_weight(*cpumask);
-	/* Return id to all */
-	if (num_bits_set == nr_cpu_ids)
-		return es7000_cpu_to_logical_apicid(0);
-	/*
-	 * The cpus in the mask must all be on the apic cluster.  If are not
-	 * on the same apicid cluster return default value of target_cpus():
-	 */
-	cpu = first_cpu(*cpumask);
-	apicid = es7000_cpu_to_logical_apicid(cpu);
-	while (cpus_found < num_bits_set) {
-		if (cpu_isset(cpu, *cpumask)) {
-			int new_apicid = es7000_cpu_to_logical_apicid(cpu);
-
-			if (APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
-				printk("%s: Not a valid mask!\n", __func__);
-
 				return es7000_cpu_to_logical_apicid(0);
 			}
 			apicid = new_apicid;
@@ -739,7 +701,7 @@ struct apic apic_es7000_cluster = {
 	.set_apic_id			= NULL,
 	.apic_id_mask			= 0xFF << 24,
 
-	.cpu_mask_to_apicid		= es7000_cpu_mask_to_apicid_cluster,
+	.cpu_mask_to_apicid		= es7000_cpu_mask_to_apicid,
 	.cpu_mask_to_apicid_and		= es7000_cpu_mask_to_apicid_and,
 
 	.send_IPI_mask			= es7000_send_IPI_mask,
-- 
1.6.1.3

[PATCH 3/4] x86_32: apic/es7000_32, fix cpu_mask_to_apicid

[Jiri Slaby]

Perform same-cluster checking even for masks with all (nr_cpu_ids)
bits set and report BAD_APICID on failure.

While at it, convert it to for_each_cpu.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/apic/es7000_32.c |   30 ++++++++++--------------------
 1 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index 9b9e86f..4d8830f 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -577,32 +577,22 @@ static int es7000_check_phys_apicid_present(int cpu_physical_apicid)
 
 static unsigned int es7000_cpu_mask_to_apicid(const cpumask_t *cpumask)
 {
-	unsigned int cpu, num_bits_set, cpus_found = 0;
-	int apicid;
+	unsigned int round = 0;
+	int cpu, uninitialized_var(apicid);
 
-	num_bits_set = cpumask_weight(cpumask);
-	/* Return id to all */
-	if (num_bits_set == nr_cpu_ids)
-		return es7000_cpu_to_logical_apicid(0);
 	/*
-	 * The cpus in the mask must all be on the apic cluster.  If are not
-	 * on the same apicid cluster return default value of target_cpus():
+	 * The cpus in the mask must all be on the apic cluster.
 	 */
-	cpu = cpumask_first(cpumask);
-	apicid = es7000_cpu_to_logical_apicid(cpu);
-	while (cpus_found < num_bits_set) {
-		if (cpumask_test_cpu(cpu, cpumask)) {
-			int new_apicid = es7000_cpu_to_logical_apicid(cpu);
+	for_each_cpu(cpu, cpumask) {
+		int new_apicid = es7000_cpu_to_logical_apicid(cpu);
 
-			if (APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
-				WARN(1, "Not a valid mask!");
+		if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
+			WARN(1, "Not a valid mask!");
 
-				return es7000_cpu_to_logical_apicid(0);
-			}
-			apicid = new_apicid;
-			cpus_found++;
+			return BAD_APICID;
 		}
-		cpu++;
+		apicid = new_apicid;
+		round++;
 	}
 	return apicid;
 }
-- 
1.6.1.3

[PATCH 4/4] x86_32: apic/summit_32, fix cpu_mask_to_apicid

[Jiri Slaby]

Perform same-cluster checking even for masks with all (nr_cpu_ids)
bits set and report correct apicid on success instead.

While at it, convert it to for_each_cpu and newer cpumask api.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/apic/summit_32.c |   30 +++++++++---------------------
 1 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kernel/apic/summit_32.c b/arch/x86/kernel/apic/summit_32.c
index 0a1135c..beda620 100644
--- a/arch/x86/kernel/apic/summit_32.c
+++ b/arch/x86/kernel/apic/summit_32.c
@@ -291,33 +291,21 @@ static int summit_check_phys_apicid_present(int boot_cpu_physical_apicid)
 
 static unsigned int summit_cpu_mask_to_apicid(const cpumask_t *cpumask)
 {
-	int cpus_found = 0;
-	int num_bits_set;
-	int apicid;
-	int cpu;
+	unsigned int round = 0;
+	int cpu, apicid = 0;
 
-	num_bits_set = cpus_weight(*cpumask);
-	if (num_bits_set >= nr_cpu_ids)
-		return BAD_APICID;
 	/*
 	 * The cpus in the mask must all be on the apic cluster.
 	 */
-	cpu = first_cpu(*cpumask);
-	apicid = summit_cpu_to_logical_apicid(cpu);
-
-	while (cpus_found < num_bits_set) {
-		if (cpu_isset(cpu, *cpumask)) {
-			int new_apicid = summit_cpu_to_logical_apicid(cpu);
+	for_each_cpu(cpu, cpumask) {
+		int new_apicid = summit_cpu_to_logical_apicid(cpu);
 
-			if (APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
-				printk("%s: Not a valid mask!\n", __func__);
-
-				return BAD_APICID;
-			}
-			apicid = apicid | new_apicid;
-			cpus_found++;
+		if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
+			printk("%s: Not a valid mask!\n", __func__);
+			return BAD_APICID;
 		}
-		cpu++;
+		apicid |= new_apicid;
+		round++;
 	}
 	return apicid;
 }
-- 
1.6.1.3

Re: [PATCH v2 1/2] x86_32: summit_32, use BAD_APICID

[Jiri Slaby]

On 28.2.2009 09:21, Ingo Molnar wrote:
>> I've sent 4 more patches, but there are still issues:
>> * es7000 + summit: I haven't solved calling with all bits set (only all
>> online is sufficient to trigger this). Some of the processors needn't be
>> on the same apic cluster. It will scream now (again -- it did before
>> adding the "optimisation"). Actually I don't know how to solve this. How
>> the caller would know the correct mask, ANDing with a apic->target_cpus
>> retval?

Anybody, please? This is not a theoretical issue, this is a real problem 
we hit...

cpu_mask_to_apicid: Not a valid mask! [was: x86_32: summit_32, use BAD_APICID]

[Jiri Slaby]

Adding Yinghai and Andi to CCs, maybe they can help? And changing subject.

On 8.3.2009 18:00, Jiri Slaby wrote:
> On 28.2.2009 09:21, Ingo Molnar wrote:
>>> I've sent 4 more patches, but there are still issues:
>>> * es7000 + summit: I haven't solved calling with all bits set (only all
>>> online is sufficient to trigger this). Some of the processors needn't be
>>> on the same apic cluster. It will scream now (again -- it did before
>>> adding the "optimisation"). Actually I don't know how to solve this. How
>>> the caller would know the correct mask, ANDing with a apic->target_cpus
>>> retval?
>
> Anybody, please? This is not a theoretical issue, this is a real problem
> we hit...