[patch 0/8] Updated patches for x86_64

[patch 0/8] Updated patches for x86_64

[Ashok Raj]

Hi Andrew,

This patch series contains some misc fixes related to CPU hotplug for X86_64.

Some are new, some are due to regressions from introduction of physflat mode
genapic schemes. Please consider for -mm. These are on 2.6.13-rc4-mm1

do_clustered_apic_check
	Needed for x86_64, removed recently from shared code.
create-sysfs-onlyfor-present-cpus
	Create sysfs entries only for present cpus. New cpus will have them
	created by ACPI code when notification for the same is processed.
fix-enforce-max-cpu
	Dont enforce this when CPU hotplug is enabled. This doesnt permit
	booting with maxcpus=1 and then testing logical hot-add of cpu.
fix-cluster-allbutself-ipi
	Cluster mode also needs to prevent preempt when excluding self from
	online map. Propagating the fix i added for genapic_flat to cluster
	genapic code as well.
fix-flat-mode-nobroadcast-again
	Recent physflat broke this for hotplug. Re-introducing it again.
fix-physflat-dmode
	Removed un-necessary code from physflat settings.
use-common-physflat-cluster
	Used common code for genapic-physflat and cluster since they share 
	a lot of code was duplicated instead of sharing them.
choose-physflat-onlyfor-gt8cpus-amd
	Choose physflat only when >8 cpus. We could still use flat mode without 
	broadcast shortcut. The mask version of IPI is still effective and 
	more performant than the unicast version thats required when we use
	physical mode.

Cheers,
ashok

[patch 1/8] x86_64: Reintroduce clustered_apic_check() for x86_64.

[Ashok Raj]

[-- Attachment #1: do_clustered_apic_check --]
[-- Type: text/plain, Size: 821 bytes --]

Auto selection of bigsmp patch removed this check from a shared common file
in arch/i386/kernel/acpi/boot.c. We still need to call this to determine 
the right genapic code for x86_64. 

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
-----------------------------------------------------------
 arch/x86_64/kernel/setup.c |    1 +
 1 files changed, 1 insertion(+)

Index: linux-2.6.13-rc4-mm1/arch/x86_64/kernel/setup.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/x86_64/kernel/setup.c
+++ linux-2.6.13-rc4-mm1/arch/x86_64/kernel/setup.c
@@ -663,6 +663,7 @@ void __init setup_arch(char **cmdline_p)
 	 * Read APIC and some other early information from ACPI tables.
 	 */
 	acpi_boot_init();
+	clustered_apic_check();
 #endif
 
 #ifdef CONFIG_X86_LOCAL_APIC

--

[patch 2/8] x86_64: create sysfs entries for cpu only for present cpus

[Ashok Raj]

[-- Attachment #1: create-sysfs-onlyfor-present-cpus --]
[-- Type: text/plain, Size: 970 bytes --]

Need to create sysfs only for cpus that are present. Without which we see
NR_CPUS entries created when we have CONFIG_HOTPLUG and CONFIG_HOTPLUG_CPU
enabled.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
--------------------------------------------------------------
 arch/i386/mach-default/topology.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6.13-rc3-ak/arch/i386/mach-default/topology.c
===================================================================
--- linux-2.6.13-rc3-ak.orig/arch/i386/mach-default/topology.c
+++ linux-2.6.13-rc3-ak/arch/i386/mach-default/topology.c
@@ -76,7 +76,7 @@ static int __init topology_init(void)
 	for_each_online_node(i)
 		arch_register_node(i);
 
-	for_each_cpu(i)
+	for_each_present_cpu(i)
 		arch_register_cpu(i);
 	return 0;
 }
@@ -87,7 +87,7 @@ static int __init topology_init(void)
 {
 	int i;
 
-	for_each_cpu(i)
+	for_each_present_cpu(i)
 		arch_register_cpu(i);
 	return 0;
 }

--

[patch 3/8] x86_64:Dont call enforce_max_cpus when hotplug is enabled

[Ashok Raj]

[-- Attachment #1: fix-enforce-max-cpu --]
[-- Type: text/plain, Size: 1627 bytes --]

No need to enforce_max_cpus when hotplug code is enabled. This
nukes out cpu_present_map and cpu_possible_map making it impossible to add
new cpus in the system.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
------------------------------------------------
 arch/x86_64/kernel/smpboot.c |   40 +++++++++++++++++++++++-----------------
 1 files changed, 23 insertions(+), 17 deletions(-)

Index: linux-2.6.13-rc4-mm1/arch/x86_64/kernel/smpboot.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/x86_64/kernel/smpboot.c
+++ linux-2.6.13-rc4-mm1/arch/x86_64/kernel/smpboot.c
@@ -893,23 +893,6 @@ static __init void disable_smp(void)
 	cpu_set(0, cpu_core_map[0]);
 }
 
-/*
- * Handle user cpus=... parameter.
- */
-static __init void enforce_max_cpus(unsigned max_cpus)
-{
-	int i, k;
-	k = 0;
-	for (i = 0; i < NR_CPUS; i++) {
-		if (!cpu_possible(i))
-			continue;
-		if (++k > max_cpus) {
-			cpu_clear(i, cpu_possible_map);
-			cpu_clear(i, cpu_present_map);
-		}
-	}
-}
-
 #ifdef CONFIG_HOTPLUG_CPU
 /*
  * cpu_possible_map should be static, it cannot change as cpu's
@@ -929,6 +912,29 @@ static void prefill_possible_map(void)
 	for (i = 0; i < NR_CPUS; i++)
 		cpu_set(i, cpu_possible_map);
 }
+
+/*
+ * Dont need this when we have hotplug enabled
+ */
+#define enforce_max_cpus(x)
+
+#else
+/*
+ * Handle user cpus=... parameter.
+ */
+static __init void enforce_max_cpus(unsigned max_cpus)
+{
+	int i, k;
+	k = 0;
+
+	for_each_cpu(i) {
+		if (++k > max_cpus) {
+			cpu_clear(i, cpu_possible_map);
+			cpu_clear(i, cpu_present_map);
+		}
+	}
+}
+
 #endif
 
 /*

--

[patch 4/8] x86_64:Fix cluster mode send_IPI_allbutself to use get_cpu()/put_cpu()

[Ashok Raj]

[-- Attachment #1: fix-cluster-allbutself-ipi --]
[-- Type: text/plain, Size: 1002 bytes --]

Need to ensure we dont get prempted when we clear ourself from mask when using
clustered mode genapic code.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
---------------------------------------------------------
 arch/x86_64/kernel/genapic_cluster.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletion(-)

Index: linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic_cluster.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/x86_64/kernel/genapic_cluster.c
+++ linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic_cluster.c
@@ -72,10 +72,14 @@ static void cluster_send_IPI_mask(cpumas
 static void cluster_send_IPI_allbutself(int vector)
 {
 	cpumask_t mask = cpu_online_map;
-	cpu_clear(smp_processor_id(), mask);
+	int me = get_cpu(); /* Ensure we are not preempted when we clear */
+
+	cpu_clear(me, mask);
 
 	if (!cpus_empty(mask))
 		cluster_send_IPI_mask(mask, vector);
+
+	put_cpu();
 }
 
 static void cluster_send_IPI_all(int vector)

--

[patch 5/8] x86_64:Dont do broadcast IPIs when hotplug is enabled in flat mode.

[Ashok Raj]

[-- Attachment #1: fix-flat-mode-nobroadcast-again --]
[-- Type: text/plain, Size: 1351 bytes --]

the use of non-shortcut version of routines breaking CPU hotplug. The option
to select this via cmdline also is deleted with the physflat patch, hence
directly placing this code under CONFIG_HOTPLUG_CPU.

We dont want to use broadcast mode IPI's when hotplug is enabled. This causes
bad effects in send IPI to a cpu that is offline which can trip when the 
cpu is in the process of being kicked alive.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
-------------------------------------------------------
 arch/x86_64/kernel/genapic_flat.c |    8 ++++++++
 1 files changed, 8 insertions(+)

Index: linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic_flat.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/x86_64/kernel/genapic_flat.c
+++ linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic_flat.c
@@ -78,8 +78,16 @@ static void flat_send_IPI_mask(cpumask_t
 
 static void flat_send_IPI_allbutself(int vector)
 {
+#ifndef CONFIG_HOTPLUG_CPU
 	if (((num_online_cpus()) - 1) >= 1)
 		__send_IPI_shortcut(APIC_DEST_ALLBUT, vector,APIC_DEST_LOGICAL);
+#else
+	cpumask_t allbutme = cpu_online_map;
+	int me = get_cpu(); /* Ensure we are not preempted when we clear */
+	cpu_clear(me, allbutme);
+	flat_send_IPI_mask(allbutme, vector);
+	put_cpu();
+#endif
 }
 
 static void flat_send_IPI_all(int vector)

--

[patch 6/8] x86_64:Dont use Lowest Priority when using physical mode.

[Ashok Raj]

[-- Attachment #1: fix-physflat-dmode --]
[-- Type: text/plain, Size: 1047 bytes --]

Delivery mode should be APIC_DM_FIXED when using physical mode.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
----------------------------------------------------
 arch/x86_64/kernel/genapic_flat.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic_flat.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/x86_64/kernel/genapic_flat.c
+++ linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic_flat.c
@@ -175,9 +175,9 @@ static unsigned int physflat_cpu_mask_to
 
 struct genapic apic_physflat =  {
 	.name = "physical flat",
-	.int_delivery_mode = dest_LowestPrio,
+	.int_delivery_mode = dest_Fixed,
 	.int_dest_mode = (APIC_DEST_PHYSICAL != 0),
-	.int_delivery_dest = APIC_DEST_PHYSICAL | APIC_DM_LOWEST,
+	.int_delivery_dest = APIC_DEST_PHYSICAL | APIC_DM_FIXED,
 	.target_cpus = physflat_target_cpus,
 	.apic_id_registered = flat_apic_id_registered,
 	.init_apic_ldr = flat_init_apic_ldr,/*not needed, but shouldn't hurt*/

--

[patch 7/8] x86_64:Use common functions in cluster and physflat mode

[Ashok Raj]

[-- Attachment #1: use-common-physflat-cluster --]
[-- Type: text/plain, Size: 8156 bytes --]

Newly introduced physflat_* shares way too much with cluster with only
a very differences. So we introduce some common functions in that can be
reused in both cases.

In addition the following are also fixed.
- Use of non-existent CONFIG_CPU_HOTPLUG option renamed to actual one in use.
- Removed comment that ACPI would provide a way to select this dynamically
  since ACPI_CONFIG_HOTPLUG_CPU already exists that indicates platform support
  for hotplug via ACPI. In addition CONFIG_HOTPLUG_CPU only indicates logical 
  offline/online which is even used by Suspend/Resume folks where the same 
  support (for no-broadcast) is required.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
------------------------------------------------------------
 arch/x86_64/kernel/genapic.c         |   52 +++++++++++++++++++++++++++++----
 arch/x86_64/kernel/genapic_cluster.c |   55 +++--------------------------------
 arch/x86_64/kernel/genapic_flat.c    |   49 +++----------------------------
 include/asm-x86_64/ipi.h             |    5 +++
 4 files changed, 61 insertions(+), 100 deletions(-)

Index: linux-2.6.13-rc4-mm1/include/asm-x86_64/ipi.h
===================================================================
--- linux-2.6.13-rc4-mm1.orig/include/asm-x86_64/ipi.h
+++ linux-2.6.13-rc4-mm1/include/asm-x86_64/ipi.h
@@ -107,4 +107,9 @@ static inline void send_IPI_mask_sequenc
 	local_irq_restore(flags);
 }
 
+extern cpumask_t generic_target_cpus(void);
+extern void generic_send_IPI_mask(cpumask_t mask, int vector);
+extern void generic_send_IPI_allbutself(int vector);
+extern void generic_send_IPI_all(int vector);
+extern unsigned int generic_cpu_mask_to_apicid(cpumask_t cpumask);
 #endif /* __ASM_IPI_H */
Index: linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic_flat.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/x86_64/kernel/genapic_flat.c
+++ linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic_flat.c
@@ -134,56 +134,17 @@ struct genapic apic_flat =  {
  * overflows, so use physical mode.
  */
 
-static cpumask_t physflat_target_cpus(void)
-{
-	return cpumask_of_cpu(0);
-}
-
-static void physflat_send_IPI_mask(cpumask_t cpumask, int vector)
-{
-	send_IPI_mask_sequence(cpumask, vector);
-}
-
-static void physflat_send_IPI_allbutself(int vector)
-{
-	cpumask_t allbutme = cpu_online_map;
-	int me = get_cpu();
-	cpu_clear(me, allbutme);
-	physflat_send_IPI_mask(allbutme, vector);
-	put_cpu();
-}
-
-static void physflat_send_IPI_all(int vector)
-{
-	physflat_send_IPI_mask(cpu_online_map, vector);
-}
-
-static unsigned int physflat_cpu_mask_to_apicid(cpumask_t cpumask)
-{
-	int cpu;
-
-	/*
-	 * We're using fixed IRQ delivery, can only return one phys APIC ID.
-	 * May as well be the first.
-	 */
-	cpu = first_cpu(cpumask);
-	if ((unsigned)cpu < NR_CPUS)
-		return x86_cpu_to_apicid[cpu];
-	else
-		return BAD_APICID;
-}
-
 struct genapic apic_physflat =  {
 	.name = "physical flat",
 	.int_delivery_mode = dest_Fixed,
 	.int_dest_mode = (APIC_DEST_PHYSICAL != 0),
 	.int_delivery_dest = APIC_DEST_PHYSICAL | APIC_DM_FIXED,
-	.target_cpus = physflat_target_cpus,
+	.target_cpus = generic_target_cpus,
 	.apic_id_registered = flat_apic_id_registered,
 	.init_apic_ldr = flat_init_apic_ldr,/*not needed, but shouldn't hurt*/
-	.send_IPI_all = physflat_send_IPI_all,
-	.send_IPI_allbutself = physflat_send_IPI_allbutself,
-	.send_IPI_mask = physflat_send_IPI_mask,
-	.cpu_mask_to_apicid = physflat_cpu_mask_to_apicid,
+	.send_IPI_all = generic_send_IPI_all,
+	.send_IPI_allbutself = generic_send_IPI_allbutself,
+	.send_IPI_mask = generic_send_IPI_mask,
+	.cpu_mask_to_apicid = generic_cpu_mask_to_apicid,
 	.phys_pkg_id = phys_pkg_id,
 };
Index: linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic_cluster.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/x86_64/kernel/genapic_cluster.c
+++ linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic_cluster.c
@@ -57,56 +57,11 @@ static void cluster_init_apic_ldr(void)
 	apic_write_around(APIC_LDR, val);
 }
 
-/* Start with all IRQs pointing to boot CPU.  IRQ balancing will shift them. */
-
-static cpumask_t cluster_target_cpus(void)
-{
-	return cpumask_of_cpu(0);
-}
-
-static void cluster_send_IPI_mask(cpumask_t mask, int vector)
-{
-	send_IPI_mask_sequence(mask, vector);
-}
-
-static void cluster_send_IPI_allbutself(int vector)
-{
-	cpumask_t mask = cpu_online_map;
-	int me = get_cpu(); /* Ensure we are not preempted when we clear */
-
-	cpu_clear(me, mask);
-
-	if (!cpus_empty(mask))
-		cluster_send_IPI_mask(mask, vector);
-
-	put_cpu();
-}
-
-static void cluster_send_IPI_all(int vector)
-{
-	cluster_send_IPI_mask(cpu_online_map, vector);
-}
-
 static int cluster_apic_id_registered(void)
 {
 	return 1;
 }
 
-static unsigned int cluster_cpu_mask_to_apicid(cpumask_t cpumask)
-{
-	int cpu;
-
-	/*
-	 * We're using fixed IRQ delivery, can only return one phys APIC ID.
-	 * May as well be the first.
-	 */
-	cpu = first_cpu(cpumask);
-	if ((unsigned)cpu < NR_CPUS)
-		return x86_cpu_to_apicid[cpu];
-	else
-		return BAD_APICID;
-}
-
 /* cpuid returns the value latched in the HW at reset, not the APIC ID
  * register's value.  For any box whose BIOS changes APIC IDs, like
  * clustered APIC systems, we must use hard_smp_processor_id.
@@ -123,12 +78,12 @@ struct genapic apic_cluster = {
 	.int_delivery_mode = dest_Fixed,
 	.int_dest_mode = (APIC_DEST_PHYSICAL != 0),
 	.int_delivery_dest = APIC_DEST_PHYSICAL | APIC_DM_FIXED,
-	.target_cpus = cluster_target_cpus,
+	.target_cpus = generic_target_cpus,
 	.apic_id_registered = cluster_apic_id_registered,
 	.init_apic_ldr = cluster_init_apic_ldr,
-	.send_IPI_all = cluster_send_IPI_all,
-	.send_IPI_allbutself = cluster_send_IPI_allbutself,
-	.send_IPI_mask = cluster_send_IPI_mask,
-	.cpu_mask_to_apicid = cluster_cpu_mask_to_apicid,
+	.send_IPI_all = generic_send_IPI_all,
+	.send_IPI_allbutself = generic_send_IPI_allbutself,
+	.send_IPI_mask = generic_send_IPI_mask,
+	.cpu_mask_to_apicid = generic_cpu_mask_to_apicid,
 	.phys_pkg_id = phys_pkg_id,
 };
Index: linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/x86_64/kernel/genapic.c
+++ linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic.c
@@ -71,14 +71,10 @@ void __init clustered_apic_check(void)
 	/* Don't use clustered mode on AMD platforms. */
  	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
 		genapic = &apic_physflat;
-#ifndef CONFIG_CPU_HOTPLUG
 		/* In the CPU hotplug case we cannot use broadcast mode
 		   because that opens a race when a CPU is removed.
-		   Stay at physflat mode in this case.
-		   It is bad to do this unconditionally though. Once
-		   we have ACPI platform support for CPU hotplug
-		   we should detect hotplug capablity from ACPI tables and
-		   only do this when really needed. -AK */
+		   Stay at physflat mode in this case. - AK */
+#ifdef CONFIG_HOTPLUG_CPU
 		if (num_cpus <= 8)
 			genapic = &apic_flat;
 #endif
@@ -118,3 +114,47 @@ void send_IPI_self(int vector)
 {
 	__send_IPI_shortcut(APIC_DEST_SELF, vector, APIC_DEST_PHYSICAL);
 }
+
+/* Start with all IRQS pointing to current CPU. IRQ balancing will shift them */
+cpumask_t generic_target_cpus(void)
+{
+	int cpu = smp_processor_id();
+
+	return cpumask_of_cpu(cpu);
+}
+
+void generic_send_IPI_mask(cpumask_t mask, int vector)
+{
+	send_IPI_mask_sequence(mask, vector);
+}
+
+void generic_send_IPI_allbutself(int vector)
+{
+	cpumask_t mask = cpu_online_map;
+	int me=get_cpu(); /* Ensure we dont move to new cpu when clearing self */
+
+	cpu_clear(me, mask);
+	if (!cpus_empty(mask))
+		generic_send_IPI_mask(mask, vector);
+	put_cpu();
+}
+
+void generic_send_IPI_all(int vector)
+{
+	generic_send_IPI_mask(cpu_online_map, vector);
+}
+
+unsigned int generic_cpu_mask_to_apicid(cpumask_t cpumask)
+{
+	int cpu;
+
+	/*
+	 * We're using fixed IRQ delivery, can only return one phys APIC ID.
+	 * May as well be the first.
+	 */
+	cpu = first_cpu(cpumask);
+	if ((unsigned)cpu < NR_CPUS)
+		return x86_cpu_to_apicid[cpu];
+	else
+		return BAD_APICID;
+}

--

[patch 8/8] x86_64: Choose physflat for AMD systems only when >8 CPUS.

[Ashok Raj]

[-- Attachment #1: choose-physflat-onlyfor-gt8cpus-amd --]
[-- Type: text/plain, Size: 1338 bytes --]

It is not required to choose the physflat mode when CPU hotplug is enabled 
and CPUs <=8 case. Use of genapic_flat with the mask version is capable of 
doing the same, instead of doing the send_IPI_mask_sequence() where its a 
unicast.

This is another change that Andi introduced with the physflat mode. 

Andi: Do you think this is acceptable?

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
---------------------------------------------------------------
 arch/x86_64/kernel/genapic.c |    9 +--------
 1 files changed, 1 insertion(+), 8 deletions(-)

Index: linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/x86_64/kernel/genapic.c
+++ linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic.c
@@ -69,15 +69,8 @@ void __init clustered_apic_check(void)
 	}
 
 	/* Don't use clustered mode on AMD platforms. */
- 	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
+ 	if ((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) && (num_cpus > 8)) {
 		genapic = &apic_physflat;
-		/* In the CPU hotplug case we cannot use broadcast mode
-		   because that opens a race when a CPU is removed.
-		   Stay at physflat mode in this case. - AK */
-#ifdef CONFIG_HOTPLUG_CPU
-		if (num_cpus <= 8)
-			genapic = &apic_flat;
-#endif
  		goto print;
  	}
 

--

Re: [patch 1/8] x86_64: Reintroduce clustered_apic_check() for x86_64.

[Ashok Raj]

On Mon, Aug 01, 2005 at 01:20:18PM -0700, Ashok Raj wrote:
> Auto selection of bigsmp patch removed this check from a shared common file
> in arch/i386/kernel/acpi/boot.c. We still need to call this to determine 
> the right genapic code for x86_64. 
> 

Thanks venki,

missed the check for lapic and smp_found_config before the call.

Resending patch.

-- 
Cheers,
Ashok Raj
- Open Source Technology Center

Auto selection of bigsmp patch removed this check from a shared common file
in arch/i386/kernel/acpi/boot.c. We still need to call this to determine 
the right genapic code for x86_64. 

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
-----------------------------------------------------------
 arch/x86_64/kernel/setup.c |    2 ++
 1 files changed, 2 insertions(+)

Index: linux-2.6.13-rc4-mm1/arch/x86_64/kernel/setup.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/x86_64/kernel/setup.c
+++ linux-2.6.13-rc4-mm1/arch/x86_64/kernel/setup.c
@@ -663,6 +663,8 @@ void __init setup_arch(char **cmdline_p)
 	 * Read APIC and some other early information from ACPI tables.
 	 */
 	acpi_boot_init();
+	if (acpi_lapic && smp_found_config)
+		clustered_apic_check();
 #endif
 
 #ifdef CONFIG_X86_LOCAL_APIC

Re: [patch 2/8] x86_64: create sysfs entries for cpu only for present cpus

[Andi Kleen]

On Mon, Aug 01, 2005 at 01:20:19PM -0700, Ashok Raj wrote:
> Need to create sysfs only for cpus that are present. Without which we see
> NR_CPUS entries created when we have CONFIG_HOTPLUG and CONFIG_HOTPLUG_CPU
> enabled.

Thanks looks good.

-Andi

> 
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> --------------------------------------------------------------
>  arch/i386/mach-default/topology.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6.13-rc3-ak/arch/i386/mach-default/topology.c
> ===================================================================
> --- linux-2.6.13-rc3-ak.orig/arch/i386/mach-default/topology.c
> +++ linux-2.6.13-rc3-ak/arch/i386/mach-default/topology.c
> @@ -76,7 +76,7 @@ static int __init topology_init(void)
>  	for_each_online_node(i)
>  		arch_register_node(i);
>  
> -	for_each_cpu(i)
> +	for_each_present_cpu(i)
>  		arch_register_cpu(i);
>  	return 0;
>  }
> @@ -87,7 +87,7 @@ static int __init topology_init(void)
>  {
>  	int i;
>  
> -	for_each_cpu(i)
> +	for_each_present_cpu(i)
>  		arch_register_cpu(i);
>  	return 0;
>  }
> 
> --
> 

Re: [patch 3/8] x86_64:Dont call enforce_max_cpus when hotplug is enabled

[Andi Kleen]

On Mon, Aug 01, 2005 at 01:20:20PM -0700, Ashok Raj wrote:
> No need to enforce_max_cpus when hotplug code is enabled. This
> nukes out cpu_present_map and cpu_possible_map making it impossible to add
> new cpus in the system.

Hmm - i think there was some reason for this early zeroing,
but I cannot remember it right now.

It might be related to some checks later that check max possible cpus.

So it would be still good to have some way to limit max possible cpus.
Maybe with a new option?

-Andi

Re: [patch 4/8] x86_64:Fix cluster mode send_IPI_allbutself to use get_cpu()/put_cpu()

[Andi Kleen]

On Mon, Aug 01, 2005 at 01:20:21PM -0700, Ashok Raj wrote:
> Need to ensure we dont get prempted when we clear ourself from mask when using
> clustered mode genapic code.

It's not needed I think. If the caller wants to execute code
on the current CPU then it has to have disabled preemption
itself already to avoid races. And if not it doesn't care.

One could argue that this function should be always called
with preemption disabled though. Perhaps better a WARN_ON().

-Andi

Re: [patch 5/8] x86_64:Dont do broadcast IPIs when hotplug is enabled in flat mode.

[Andi Kleen]

>  static void flat_send_IPI_allbutself(int vector)
>  {
> +#ifndef CONFIG_HOTPLUG_CPU
>  	if (((num_online_cpus()) - 1) >= 1)
>  		__send_IPI_shortcut(APIC_DEST_ALLBUT, vector,APIC_DEST_LOGICAL);
> +#else
> +	cpumask_t allbutme = cpu_online_map;
> +	int me = get_cpu(); /* Ensure we are not preempted when we clear */
> +	cpu_clear(me, allbutme);
> +	flat_send_IPI_mask(allbutme, vector);
> +	put_cpu();

This still needs the num_online_cpus()s check.

-Andi

Re: [patch 8/8] x86_64: Choose physflat for AMD systems only when >8 CPUS.

[Andi Kleen]

On Mon, Aug 01, 2005 at 01:20:25PM -0700, Ashok Raj wrote:
> It is not required to choose the physflat mode when CPU hotplug is enabled 
> and CPUs <=8 case. Use of genapic_flat with the mask version is capable of 
> doing the same, instead of doing the send_IPI_mask_sequence() where its a 
> unicast.
> 
> This is another change that Andi introduced with the physflat mode. 
> 
> Andi: Do you think this is acceptable?

Yes it's ok.

-Andi

Re: [patch 3/8] x86_64:Dont call enforce_max_cpus when hotplug is enabled

[Ashok Raj]

On Thu, Aug 04, 2005 at 12:41:10PM +0200, Andi Kleen wrote:
> On Mon, Aug 01, 2005 at 01:20:20PM -0700, Ashok Raj wrote:
> > No need to enforce_max_cpus when hotplug code is enabled. This
> > nukes out cpu_present_map and cpu_possible_map making it impossible to add
> > new cpus in the system.
> 
> Hmm - i think there was some reason for this early zeroing,
> but I cannot remember it right now.
> 
> It might be related to some checks later that check max possible cpus.
> 
> So it would be still good to have some way to limit max possible cpus.
> Maybe with a new option?

The only useful thing with enfore_max() is that cpu_possible_map is trimmed
so some resource allocations that use for_each_cpu() for upfront allocation
wont allocate resources.

Currently i see max_cpus only limiting boot-time start, none trim cpu_possible
which is done in only x86_64. max_cpu is still honored, just that for initial
boot. I would think maybe remove enforce_max_cpus() altogether like other 
archs instead of adding one more just for x86_64. 

Maybe we should add only if there is a need, instead of adding and finding
no-one using it and finally removing it very soon.
> 
> -Andi

-- 
Cheers,
Ashok Raj
- Open Source Technology Center

Re: [patch 4/8] x86_64:Fix cluster mode send_IPI_allbutself to use get_cpu()/put_cpu()

[Ashok Raj]

On Thu, Aug 04, 2005 at 12:43:02PM +0200, Andi Kleen wrote:
> On Mon, Aug 01, 2005 at 01:20:21PM -0700, Ashok Raj wrote:
> > Need to ensure we dont get prempted when we clear ourself from mask when using
> > clustered mode genapic code.
> 
> It's not needed I think. If the caller wants to execute code
> on the current CPU then it has to have disabled preemption
> itself already to avoid races. And if not it doesn't care.
> 
> One could argue that this function should be always called
> with preemption disabled though. Perhaps better a WARN_ON().
> 

This is only required for smp_call_function(), since we do allbutself
by exclusing self, its the internal function that needs to do this.

allbutself shortcut takes care of it, since it doesnt matter which cpu
we write the shortcut, in the mask version and for cluster i think its required
to ensure in the low level function. Otherwise we would need each 
implementation of smp_call_function() and send_IPI_allbutself() callers would
need to do this, which would be lots of changes.
> -Andi

-- 
Cheers,
Ashok Raj
- Open Source Technology Center

Re: [patch 5/8] x86_64:Dont do broadcast IPIs when hotplug is enabled in flat mode.

[Ashok Raj]

On Thu, Aug 04, 2005 at 12:51:07PM +0200, Andi Kleen wrote:
> >  static void flat_send_IPI_allbutself(int vector)
> >  {
> > +#ifndef CONFIG_HOTPLUG_CPU
> >  	if (((num_online_cpus()) - 1) >= 1)
> >  		__send_IPI_shortcut(APIC_DEST_ALLBUT, vector,APIC_DEST_LOGICAL);
> > +#else
> > +	cpumask_t allbutme = cpu_online_map;
> > +	int me = get_cpu(); /* Ensure we are not preempted when we clear */
> > +	cpu_clear(me, allbutme);
> > +	flat_send_IPI_mask(allbutme, vector);
> > +	put_cpu();
> 
> This still needs the num_online_cpus()s check.

Opps missed that... Thanks for spotting it.

I will send an updated one to Andrew.
-- 
Cheers,
Ashok Raj
- Open Source Technology Center

Re: [patch 5/8] x86_64:Dont do broadcast IPIs when hotplug is enabled in flat mode.

[Ashok Raj]

On Thu, Aug 04, 2005 at 12:51:07PM +0200, Andi Kleen wrote:
> >  static void flat_send_IPI_allbutself(int vector)
> >  {
> > +#ifndef CONFIG_HOTPLUG_CPU
> >  	if (((num_online_cpus()) - 1) >= 1)
> >  		__send_IPI_shortcut(APIC_DEST_ALLBUT, vector,APIC_DEST_LOGICAL);
> > +#else
> > +	cpumask_t allbutme = cpu_online_map;
> > +	int me = get_cpu(); /* Ensure we are not preempted when we clear */
> > +	cpu_clear(me, allbutme);
> > +	flat_send_IPI_mask(allbutme, vector);
> > +	put_cpu();
> 
> This still needs the num_online_cpus()s check.
> 
> -Andi

Modified patch attached.

Andrew: the filename in your -mm queue is below, with the attached
patch.

x86_64dont-do-broadcast-ipis-when-hotplug-is-enabled-in-flat-mode.patch

-- 
Cheers,
Ashok Raj
- Open Source Technology Center


Note: Recent introduction of physflat mode for x86_64 inadvertently deleted 
the use of non-shortcut version of routines breaking CPU hotplug. The option
to select this via cmdline also is deleted with the physflat patch, hence
directly placing this code under CONFIG_HOTPLUG_CPU.

We dont want to use broadcast mode IPI's when hotplug is enabled. This causes
bad effects in send IPI to a cpu that is offline which can trip when the 
cpu is in the process of being kicked alive.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
-------------------------------------------------------
 arch/x86_64/kernel/genapic_flat.c |   10 ++++++++++
 1 files changed, 10 insertions(+)

Index: linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic_flat.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/x86_64/kernel/genapic_flat.c
+++ linux-2.6.13-rc4-mm1/arch/x86_64/kernel/genapic_flat.c
@@ -78,8 +78,18 @@ static void flat_send_IPI_mask(cpumask_t
 
 static void flat_send_IPI_allbutself(int vector)
 {
+#ifndef CONFIG_HOTPLUG_CPU
 	if (((num_online_cpus()) - 1) >= 1)
 		__send_IPI_shortcut(APIC_DEST_ALLBUT, vector,APIC_DEST_LOGICAL);
+#else
+	cpumask_t allbutme = cpu_online_map;
+	int me = get_cpu(); /* Ensure we are not preempted when we clear */
+	cpu_clear(me, allbutme);
+
+	if (!cpus_empty(allbutme))
+		flat_send_IPI_mask(allbutme, vector);
+	put_cpu();
+#endif
 }
 
 static void flat_send_IPI_all(int vector)