[rfc 0/4] x86 - apic dummy ops with cleanups

[rfc 0/4] x86 - apic dummy ops with cleanups

[Cyrill Gorcunov]

Hi,

here is a series of patches with some tiny
cleanup and one bugfix I believe (and most
probably why fixing one issue new one could
be introduced as well :-)

Please review and test if possible. _Any_ kind
of feedback is highly appreciated!

Cyrill

[rfc 1/4] x86: irq.c - tiny cleanup

[Cyrill Gorcunov]

[-- Attachment #1: x86-irq-cleanup --]
[-- Type: text/plain, Size: 1350 bytes --]

Impact: cleanup

1) guard ack_bad_irq with printk_ratelimit since there is no
   guarantee we will not be flooded one day
2) use pr_emerg helper

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/kernel/irq.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Index: linux-2.6.git/arch/x86/kernel/irq.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/irq.c
+++ linux-2.6.git/arch/x86/kernel/irq.c
@@ -24,7 +24,8 @@ void (*generic_interrupt_extension)(void
  */
 void ack_bad_irq(unsigned int irq)
 {
-	printk(KERN_ERR "unexpected IRQ trap at vector %02x\n", irq);
+	if (printk_ratelimit())
+		pr_err("unexpected IRQ trap at vector %02x\n", irq);
 
 #ifdef CONFIG_X86_LOCAL_APIC
 	/*
@@ -188,7 +189,7 @@ u64 arch_irq_stat_cpu(unsigned int cpu)
 	sum += irq_stats(cpu)->irq_thermal_count;
 # ifdef CONFIG_X86_64
 	sum += irq_stats(cpu)->irq_threshold_count;
-#endif
+# endif
 #endif
 	return sum;
 }
@@ -229,8 +230,8 @@ unsigned int __irq_entry do_IRQ(struct p
 #endif
 
 		if (printk_ratelimit())
-			printk(KERN_EMERG "%s: %d.%d No irq handler for vector (irq %d)\n",
-			       __func__, smp_processor_id(), vector, irq);
+			pr_emerg("%s: %d.%d No irq handler for vector (irq %d)\n",
+				__func__, smp_processor_id(), vector, irq);
 	}
 
 	irq_exit();

Re: [rfc 1/4] x86: irq.c - tiny cleanup

[Ingo Molnar]

* Cyrill Gorcunov <gorcunov@openvz.org> wrote:

> Impact: cleanup
> 
> 1) guard ack_bad_irq with printk_ratelimit since there is no
>    guarantee we will not be flooded one day
> 2) use pr_emerg helper
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> ---
>  arch/x86/kernel/irq.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> Index: linux-2.6.git/arch/x86/kernel/irq.c
> =====================================================================
> --- linux-2.6.git.orig/arch/x86/kernel/irq.c
> +++ linux-2.6.git/arch/x86/kernel/irq.c
> @@ -24,7 +24,8 @@ void (*generic_interrupt_extension)(void
>   */
>  void ack_bad_irq(unsigned int irq)
>  {
> -	printk(KERN_ERR "unexpected IRQ trap at vector %02x\n", irq);
> +	if (printk_ratelimit())
> +		pr_err("unexpected IRQ trap at vector %02x\n", irq);

Btw, this is not so theoretical - in the past i've had bugs where a 
flood of these messages made the system unusable. (granted, it would 
have been unusable even with rate-limiting - but it's still nice to 
have a throttle in place for something like this.)

	Ingo

Re: [rfc 1/4] x86: irq.c - tiny cleanup

[Cyrill Gorcunov]

[Ingo Molnar - Sun, Apr 12, 2009 at 07:25:36PM +0200]
| 
| * Cyrill Gorcunov <gorcunov@openvz.org> wrote:
| 
| > Impact: cleanup
| > 
| > 1) guard ack_bad_irq with printk_ratelimit since there is no
| >    guarantee we will not be flooded one day
| > 2) use pr_emerg helper
| > 
... 
| Btw, this is not so theoretical - in the past i've had bugs where a 
| flood of these messages made the system unusable. (granted, it would 
| have been unusable even with rate-limiting - but it's still nice to 
| have a throttle in place for something like this.)
| 
| 	Ingo
| 

I bet the only benefit is that a user will have
some time to pull out camera and snap screen
before it's scrolled with bad irq messages :)

        Cyrill

[tip:x86/apic] x86: irq.c - tiny cleanup

[tip-bot for Cyrill Gorcunov]

Commit-ID:  edea7148a87c099e5d5d4838285cc27e459588b7
Gitweb:     http://git.kernel.org/tip/edea7148a87c099e5d5d4838285cc27e459588b7
Author:     Cyrill Gorcunov <gorcunov@openvz.org>
AuthorDate: Sun, 12 Apr 2009 20:47:39 +0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 12 Apr 2009 19:23:52 +0200

x86: irq.c - tiny cleanup

Impact: cleanup, robustization

 1) guard ack_bad_irq with printk_ratelimit since there is no
    guarantee we will not be flooded one day

 2) use pr_emerg() helper

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
LKML-Reference: <20090412165058.277579847@openvz.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/kernel/irq.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index 3aaf7b9..6603492 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -24,7 +24,8 @@ void (*generic_interrupt_extension)(void) = NULL;
  */
 void ack_bad_irq(unsigned int irq)
 {
-	printk(KERN_ERR "unexpected IRQ trap at vector %02x\n", irq);
+	if (printk_ratelimit())
+		pr_err("unexpected IRQ trap at vector %02x\n", irq);
 
 #ifdef CONFIG_X86_LOCAL_APIC
 	/*
@@ -178,7 +179,7 @@ u64 arch_irq_stat_cpu(unsigned int cpu)
 	sum += irq_stats(cpu)->irq_thermal_count;
 # ifdef CONFIG_X86_64
 	sum += irq_stats(cpu)->irq_threshold_count;
-#endif
+# endif
 #endif
 	return sum;
 }
@@ -219,8 +220,8 @@ unsigned int __irq_entry do_IRQ(struct pt_regs *regs)
 #endif
 
 		if (printk_ratelimit())
-			printk(KERN_EMERG "%s: %d.%d No irq handler for vector (irq %d)\n",
-			       __func__, smp_processor_id(), vector, irq);
+			pr_emerg("%s: %d.%d No irq handler for vector (irq %d)\n",
+				__func__, smp_processor_id(), vector, irq);
 	}
 
 	irq_exit();

[rfc 2/4] x86: apic - introduce imcr_ helpers

[Cyrill Gorcunov]

[-- Attachment #1: x86-apic-imcr --]
[-- Type: text/plain, Size: 1801 bytes --]

Impact: cleanup

Distinguish port writting magic into helpers with comments.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/kernel/apic/apic.c |   29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

Index: linux-2.6.git/arch/x86/kernel/apic/apic.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/apic.c
+++ linux-2.6.git/arch/x86/kernel/apic/apic.c
@@ -99,6 +99,29 @@ early_param("lapic", parse_lapic);
 /* Local APIC was disabled by the BIOS and enabled by the kernel */
 static int enabled_via_apicbase;
 
+/*
+ * Handle interrupt mode configuration register (IMCR).
+ * This register controls whether the interrupt signals
+ * that reach the BSP come from the master PIC or from the
+ * local APIC. Before entering Symmetric I/O Mode, either
+ * the BIOS or the operating system must switch out of
+ * PIC Mode by changing the IMCR.
+ */
+static inline imcr_pic_to_apic(void)
+{
+	/* select IMCR register */
+	outb(0x70, 0x22);
+	/* NMI and 8259 INTR go through APIC */
+	outb(0x01, 0x23);
+}
+
+static inline imcr_apic_to_pic(void)
+{
+	/* select IMCR register */
+	outb(0x70, 0x22);
+	/* NMI and 8259 INTR go directly to BSP */
+	outb(0x00, 0x23);
+}
 #endif
 
 #ifdef CONFIG_X86_64
@@ -1737,8 +1760,7 @@ void __init connect_bsp_APIC(void)
 		 */
 		apic_printk(APIC_VERBOSE, "leaving PIC mode, "
 				"enabling APIC mode.\n");
-		outb(0x70, 0x22);
-		outb(0x01, 0x23);
+		imcr_pic_to_apic();
 	}
 #endif
 	if (apic->enable_apic_mode)
@@ -1766,8 +1788,7 @@ void disconnect_bsp_APIC(int virt_wire_s
 		 */
 		apic_printk(APIC_VERBOSE, "disabling APIC mode, "
 				"entering PIC mode.\n");
-		outb(0x70, 0x22);
-		outb(0x00, 0x23);
+		imcr_apic_to_pic();
 		return;
 	}
 #endif

[tip:x86/apic] x86: apic - introduce imcr_ helpers

[tip-bot for Cyrill Gorcunov]

Commit-ID:  c0eaa4536f08b98fbcfa7fce5b7b0de1bebcb0e1
Gitweb:     http://git.kernel.org/tip/c0eaa4536f08b98fbcfa7fce5b7b0de1bebcb0e1
Author:     Cyrill Gorcunov <gorcunov@openvz.org>
AuthorDate: Sun, 12 Apr 2009 20:47:40 +0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 12 Apr 2009 19:23:53 +0200

x86: apic - introduce imcr_ helpers

Impact: cleanup

Distinguish port writting magic into helpers with comments.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
LKML-Reference: <20090412165058.535921550@openvz.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/kernel/apic/apic.c |   29 +++++++++++++++++++++++++----
 1 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 098ec84..c3be10f 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -98,6 +98,29 @@ early_param("lapic", parse_lapic);
 /* Local APIC was disabled by the BIOS and enabled by the kernel */
 static int enabled_via_apicbase;
 
+/*
+ * Handle interrupt mode configuration register (IMCR).
+ * This register controls whether the interrupt signals
+ * that reach the BSP come from the master PIC or from the
+ * local APIC. Before entering Symmetric I/O Mode, either
+ * the BIOS or the operating system must switch out of
+ * PIC Mode by changing the IMCR.
+ */
+static inline imcr_pic_to_apic(void)
+{
+	/* select IMCR register */
+	outb(0x70, 0x22);
+	/* NMI and 8259 INTR go through APIC */
+	outb(0x01, 0x23);
+}
+
+static inline imcr_apic_to_pic(void)
+{
+	/* select IMCR register */
+	outb(0x70, 0x22);
+	/* NMI and 8259 INTR go directly to BSP */
+	outb(0x00, 0x23);
+}
 #endif
 
 #ifdef CONFIG_X86_64
@@ -1727,8 +1750,7 @@ void __init connect_bsp_APIC(void)
 		 */
 		apic_printk(APIC_VERBOSE, "leaving PIC mode, "
 				"enabling APIC mode.\n");
-		outb(0x70, 0x22);
-		outb(0x01, 0x23);
+		imcr_pic_to_apic();
 	}
 #endif
 	if (apic->enable_apic_mode)
@@ -1756,8 +1778,7 @@ void disconnect_bsp_APIC(int virt_wire_setup)
 		 */
 		apic_printk(APIC_VERBOSE, "disabling APIC mode, "
 				"entering PIC mode.\n");
-		outb(0x70, 0x22);
-		outb(0x00, 0x23);
+		imcr_apic_to_pic();
 		return;
 	}
 #endif

[rfc 3/4] x86: apic - introduce dummy apic operations

[Cyrill Gorcunov]

[-- Attachment #1: x86-apic-write-nop --]
[-- Type: text/plain, Size: 3633 bytes --]

In case if apic was disabled by kernel option
or hardware limits we could use dummy operations
such as apic->write at moment.

Same time the patch fixes the missed EOI in
do_IRQ function (which has place if kernel
is compiled as X86-32 and interrupt without
handler happens where apic was not asked to
be disabled via kernel option).

Note that native_apic_write_dummy consists of
WARN_ON_ONCE to catch any unpredictable
writes on enabled APICs. Could be removed
after long time of testing.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/include/asm/apic.h |    3 ++-
 arch/x86/kernel/apic/apic.c |   24 ++++++++++++++++++++++++
 arch/x86/kernel/irq.c       |   10 ++--------
 3 files changed, 28 insertions(+), 9 deletions(-)

Index: linux-2.6.git/arch/x86/include/asm/apic.h
=====================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/apic.h
+++ linux-2.6.git/arch/x86/include/asm/apic.h
@@ -212,6 +212,7 @@ static inline void ack_x2APIC_irq(void)
 }
 #endif
 
+extern void apic_disable(void);
 extern int lapic_get_maxlvt(void);
 extern void clear_local_APIC(void);
 extern void connect_bsp_APIC(void);
@@ -252,7 +253,7 @@ static inline void lapic_shutdown(void) 
 #define local_apic_timer_c2_ok		1
 static inline void init_apic_mappings(void) { }
 static inline void disable_local_APIC(void) { }
-
+static inline void apic_disable(void) { }
 #endif /* !CONFIG_X86_LOCAL_APIC */
 
 #ifdef CONFIG_X86_64
Index: linux-2.6.git/arch/x86/kernel/apic/apic.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/apic.c
+++ linux-2.6.git/arch/x86/kernel/apic/apic.c
@@ -233,6 +233,24 @@ static int modern_apic(void)
 	return lapic_get_version() >= 0x14;
 }
 
+/*
+ * bare function to substitute write operation
+ * and it's _that_ fast :)
+ */
+void native_apic_write_dummy(u32 reg, u32 v)
+{
+	WARN_ON_ONCE((cpu_has_apic || !disable_apic));
+}
+
+/*
+ * right after this call apic->write doesn't do anything
+ * note that there is no restore operation it works one way
+ */
+void apic_disable(void)
+{
+	apic->write = native_apic_write_dummy;
+}
+
 void native_apic_wait_icr_idle(void)
 {
 	while (apic_read(APIC_ICR) & APIC_ICR_BUSY)
@@ -1592,6 +1610,12 @@ void __init init_apic_mappings(void)
 	 */
 	if (boot_cpu_physical_apicid == -1U)
 		boot_cpu_physical_apicid = read_apic_id();
+
+	/* lets check if we may to NOP'ify apic operations */
+	if (!cpu_has_apic) {
+		pr_info("APIC: disable apic facility\n");
+		apic_disable();
+	}
 }
 
 /*
Index: linux-2.6.git/arch/x86/kernel/irq.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/irq.c
+++ linux-2.6.git/arch/x86/kernel/irq.c
@@ -27,7 +27,6 @@ void ack_bad_irq(unsigned int irq)
 	if (printk_ratelimit())
 		pr_err("unexpected IRQ trap at vector %02x\n", irq);
 
-#ifdef CONFIG_X86_LOCAL_APIC
 	/*
 	 * Currently unexpected vectors happen only on SMP and APIC.
 	 * We _must_ ack these because every local APIC has only N
@@ -37,9 +36,7 @@ void ack_bad_irq(unsigned int irq)
 	 * completely.
 	 * But only ack when the APIC is enabled -AK
 	 */
-	if (cpu_has_apic)
-		ack_APIC_irq();
-#endif
+	ack_APIC_irq();
 }
 
 #define irq_stats(x)		(&per_cpu(irq_stat, x))
@@ -224,10 +221,7 @@ unsigned int __irq_entry do_IRQ(struct p
 	irq = __get_cpu_var(vector_irq)[vector];
 
 	if (!handle_irq(irq, regs)) {
-#ifdef CONFIG_X86_64
-		if (!disable_apic)
-			ack_APIC_irq();
-#endif
+		ack_APIC_irq();
 
 		if (printk_ratelimit())
 			pr_emerg("%s: %d.%d No irq handler for vector (irq %d)\n",

[tip:x86/apic] x86: apic - introduce dummy apic operations

[tip-bot for Cyrill Gorcunov]

Commit-ID:  08306ce61d6848e6fbf74fa4cc693c3fb29e943f
Gitweb:     http://git.kernel.org/tip/08306ce61d6848e6fbf74fa4cc693c3fb29e943f
Author:     Cyrill Gorcunov <gorcunov@openvz.org>
AuthorDate: Sun, 12 Apr 2009 20:47:41 +0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 12 Apr 2009 19:23:53 +0200

x86: apic - introduce dummy apic operations

Impact: refactor, speed up and robustize code

In case if apic was disabled by kernel option
or by hardware limits we can use dummy operations
in apic->write to simplify the ack_APIC_irq() code.

At the lame time the patch fixes the missed EOI in
do_IRQ function (which has place if kernel is compiled
as X86-32 and interrupt without handler happens where
apic was not asked to be disabled via kernel option).

Note that native_apic_write_dummy() consists of
WARN_ON_ONCE to catch any buggy writes on enabled
APICs. Could be removed after some time of testing.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
LKML-Reference: <20090412165058.724788431@openvz.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/include/asm/apic.h |    3 ++-
 arch/x86/kernel/apic/apic.c |   24 ++++++++++++++++++++++++
 arch/x86/kernel/irq.c       |   10 ++--------
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 42f2f83..2bd5a46 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -212,6 +212,7 @@ static inline void ack_x2APIC_irq(void)
 }
 #endif
 
+extern void apic_disable(void);
 extern int lapic_get_maxlvt(void);
 extern void clear_local_APIC(void);
 extern void connect_bsp_APIC(void);
@@ -252,7 +253,7 @@ static inline void lapic_shutdown(void) { }
 #define local_apic_timer_c2_ok		1
 static inline void init_apic_mappings(void) { }
 static inline void disable_local_APIC(void) { }
-
+static inline void apic_disable(void) { }
 #endif /* !CONFIG_X86_LOCAL_APIC */
 
 #ifdef CONFIG_X86_64
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index c3be10f..9b849d4 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -232,6 +232,24 @@ static int modern_apic(void)
 	return lapic_get_version() >= 0x14;
 }
 
+/*
+ * bare function to substitute write operation
+ * and it's _that_ fast :)
+ */
+void native_apic_write_dummy(u32 reg, u32 v)
+{
+	WARN_ON_ONCE((cpu_has_apic || !disable_apic));
+}
+
+/*
+ * right after this call apic->write doesn't do anything
+ * note that there is no restore operation it works one way
+ */
+void apic_disable(void)
+{
+	apic->write = native_apic_write_dummy;
+}
+
 void native_apic_wait_icr_idle(void)
 {
 	while (apic_read(APIC_ICR) & APIC_ICR_BUSY)
@@ -1582,6 +1600,12 @@ void __init init_apic_mappings(void)
 	 */
 	if (boot_cpu_physical_apicid == -1U)
 		boot_cpu_physical_apicid = read_apic_id();
+
+	/* lets check if we may to NOP'ify apic operations */
+	if (!cpu_has_apic) {
+		pr_info("APIC: disable apic facility\n");
+		apic_disable();
+	}
 }
 
 /*
diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index 6603492..fd57bf3 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -27,7 +27,6 @@ void ack_bad_irq(unsigned int irq)
 	if (printk_ratelimit())
 		pr_err("unexpected IRQ trap at vector %02x\n", irq);
 
-#ifdef CONFIG_X86_LOCAL_APIC
 	/*
 	 * Currently unexpected vectors happen only on SMP and APIC.
 	 * We _must_ ack these because every local APIC has only N
@@ -37,9 +36,7 @@ void ack_bad_irq(unsigned int irq)
 	 * completely.
 	 * But only ack when the APIC is enabled -AK
 	 */
-	if (cpu_has_apic)
-		ack_APIC_irq();
-#endif
+	ack_APIC_irq();
 }
 
 #define irq_stats(x)		(&per_cpu(irq_stat, x))
@@ -214,10 +211,7 @@ unsigned int __irq_entry do_IRQ(struct pt_regs *regs)
 	irq = __get_cpu_var(vector_irq)[vector];
 
 	if (!handle_irq(irq, regs)) {
-#ifdef CONFIG_X86_64
-		if (!disable_apic)
-			ack_APIC_irq();
-#endif
+		ack_APIC_irq();
 
 		if (printk_ratelimit())
 			pr_emerg("%s: %d.%d No irq handler for vector (irq %d)\n",

[rfc 4/4] x86: smp.c - align smp_ops assignments

[Cyrill Gorcunov]

[-- Attachment #1: x86-smp-ops --]
[-- Type: text/plain, Size: 1473 bytes --]

Impact: cleanup

It's a bit hard to parse by eyes without
them being aligned.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/kernel/smp.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Index: linux-2.6.git/arch/x86/kernel/smp.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/smp.c
+++ linux-2.6.git/arch/x86/kernel/smp.c
@@ -193,19 +193,19 @@ void smp_call_function_single_interrupt(
 }
 
 struct smp_ops smp_ops = {
-	.smp_prepare_boot_cpu = native_smp_prepare_boot_cpu,
-	.smp_prepare_cpus = native_smp_prepare_cpus,
-	.smp_cpus_done = native_smp_cpus_done,
+	.smp_prepare_boot_cpu	= native_smp_prepare_boot_cpu,
+	.smp_prepare_cpus	= native_smp_prepare_cpus,
+	.smp_cpus_done		= native_smp_cpus_done,
 
-	.smp_send_stop = native_smp_send_stop,
-	.smp_send_reschedule = native_smp_send_reschedule,
+	.smp_send_stop		= native_smp_send_stop,
+	.smp_send_reschedule	= native_smp_send_reschedule,
 
-	.cpu_up = native_cpu_up,
-	.cpu_die = native_cpu_die,
-	.cpu_disable = native_cpu_disable,
-	.play_dead = native_play_dead,
+	.cpu_up			= native_cpu_up,
+	.cpu_die		= native_cpu_die,
+	.cpu_disable		= native_cpu_disable,
+	.play_dead		= native_play_dead,
 
-	.send_call_func_ipi = native_send_call_func_ipi,
+	.send_call_func_ipi	= native_send_call_func_ipi,
 	.send_call_func_single_ipi = native_send_call_func_single_ipi,
 };
 EXPORT_SYMBOL_GPL(smp_ops);

[tip:x86/apic] x86: smp.c - align smp_ops assignments

[tip-bot for Cyrill Gorcunov]

Commit-ID:  b9b34f24b23ba9e79e07c0980e7fff16af2a67d1
Gitweb:     http://git.kernel.org/tip/b9b34f24b23ba9e79e07c0980e7fff16af2a67d1
Author:     Cyrill Gorcunov <gorcunov@openvz.org>
AuthorDate: Sun, 12 Apr 2009 20:47:42 +0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 12 Apr 2009 19:23:54 +0200

x86: smp.c - align smp_ops assignments

Impact: cleanup

It's a bit hard to parse by eyes without
them being aligned.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
LKML-Reference: <20090412165058.924175574@openvz.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/kernel/smp.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
index 13f33ea..f6db48c 100644
--- a/arch/x86/kernel/smp.c
+++ b/arch/x86/kernel/smp.c
@@ -193,19 +193,19 @@ void smp_call_function_single_interrupt(struct pt_regs *regs)
 }
 
 struct smp_ops smp_ops = {
-	.smp_prepare_boot_cpu = native_smp_prepare_boot_cpu,
-	.smp_prepare_cpus = native_smp_prepare_cpus,
-	.smp_cpus_done = native_smp_cpus_done,
+	.smp_prepare_boot_cpu	= native_smp_prepare_boot_cpu,
+	.smp_prepare_cpus	= native_smp_prepare_cpus,
+	.smp_cpus_done		= native_smp_cpus_done,
 
-	.smp_send_stop = native_smp_send_stop,
-	.smp_send_reschedule = native_smp_send_reschedule,
+	.smp_send_stop		= native_smp_send_stop,
+	.smp_send_reschedule	= native_smp_send_reschedule,
 
-	.cpu_up = native_cpu_up,
-	.cpu_die = native_cpu_die,
-	.cpu_disable = native_cpu_disable,
-	.play_dead = native_play_dead,
+	.cpu_up			= native_cpu_up,
+	.cpu_die		= native_cpu_die,
+	.cpu_disable		= native_cpu_disable,
+	.play_dead		= native_play_dead,
 
-	.send_call_func_ipi = native_send_call_func_ipi,
+	.send_call_func_ipi	= native_send_call_func_ipi,
 	.send_call_func_single_ipi = native_send_call_func_single_ipi,
 };
 EXPORT_SYMBOL_GPL(smp_ops);

Re: [rfc 0/4] x86 - apic dummy ops with cleanups

[Ingo Molnar]

* Cyrill Gorcunov <gorcunov@openvz.org> wrote:

> Hi,
> 
> here is a series of patches with some tiny
> cleanup and one bugfix I believe (and most
> probably why fixing one issue new one could
> be introduced as well :-)
> 
> Please review and test if possible. _Any_ kind
> of feedback is highly appreciated!

This series looks very good and is well-structured,
i've applied it to tip:x86/apic and started testing
it, thanks Cyrill!

	Ingo

Re: [rfc 0/4] x86 - apic dummy ops with cleanups

[Cyrill Gorcunov]

[Ingo Molnar - Sun, Apr 12, 2009 at 07:24:31PM +0200]
| 
| * Cyrill Gorcunov <gorcunov@openvz.org> wrote:
| 
| > Hi,
| > 
| > here is a series of patches with some tiny
| > cleanup and one bugfix I believe (and most
| > probably why fixing one issue new one could
| > be introduced as well :-)
| > 
| > Please review and test if possible. _Any_ kind
| > of feedback is highly appreciated!
| 
| This series looks very good and is well-structured,
| i've applied it to tip:x86/apic and started testing
| it, thanks Cyrill!
| 
| 	Ingo
| 

Thanks, Ingo!

Actually I found one a bit suspicious thing (not sure if it was
initially designed to  behave like that).

Suppose we have SMP compiled kernel and even supported on HW
level but then we disable apic thru kernel option. And then
when system startup it should be working fine since we go
to uniprocessor mode. Then the machine going for reboot and we have:

Starting from arch/x86/kernel/reboot.c

native_machine_shutdown
	smp_send_stop
		smp_ops.smp_send_stop
			native_smp_send_stop
				disable_local_APIC
					apic_write(APIC_SPIV, value);

Since we have reserved APIC memory mapped area (in _anycase_)
it doesn't cause any problems but was it initially supposed?

Most probably I miss something.

        Cyrill

Re: [rfc 0/4] x86 - apic dummy ops with cleanups

[Ingo Molnar]

* Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> Thanks, Ingo!
> 
> Actually I found one a bit suspicious thing (not sure if it was 
> initially designed to behave like that).
> 
> Suppose we have SMP compiled kernel and even supported on HW level 
> but then we disable apic thru kernel option. And then when system 
> startup it should be working fine since we go to uniprocessor 
> mode. Then the machine going for reboot and we have:
> 
> Starting from arch/x86/kernel/reboot.c
> 
> native_machine_shutdown
> 	smp_send_stop
> 		smp_ops.smp_send_stop
> 			native_smp_send_stop
> 				disable_local_APIC
> 					apic_write(APIC_SPIV, value);
> 
> Since we have reserved APIC memory mapped area (in _anycase_) it 
> doesn't cause any problems but was it initially supposed?

i dont think that was supposed so. If the APIC wasnt even enabled 
then we shouldnt try to disable it. Note that there are a few 
unusual paths to APIC enablement: for example by twiddling the 
nmi_watchdog knob in /proc/sys - or in theory through a CPU hotplug 
event.

	Ingo

Re: [rfc 0/4] x86 - apic dummy ops with cleanups

[Cyrill Gorcunov]

[Ingo Molnar - Sun, Apr 12, 2009 at 08:02:39PM +0200]
| 
| * Cyrill Gorcunov <gorcunov@gmail.com> wrote:
| 
| > Thanks, Ingo!
| > 
| > Actually I found one a bit suspicious thing (not sure if it was 
| > initially designed to behave like that).
| > 
| > Suppose we have SMP compiled kernel and even supported on HW level 
| > but then we disable apic thru kernel option. And then when system 
| > startup it should be working fine since we go to uniprocessor 
| > mode. Then the machine going for reboot and we have:
| > 
| > Starting from arch/x86/kernel/reboot.c
| > 
| > native_machine_shutdown
| > 	smp_send_stop
| > 		smp_ops.smp_send_stop
| > 			native_smp_send_stop
| > 				disable_local_APIC
| > 					apic_write(APIC_SPIV, value);
| > 
| > Since we have reserved APIC memory mapped area (in _anycase_) it 
| > doesn't cause any problems but was it initially supposed?
| 
| i dont think that was supposed so. If the APIC wasnt even enabled 
| then we shouldnt try to disable it. Note that there are a few 
| unusual paths to APIC enablement: for example by twiddling the 
| nmi_watchdog knob in /proc/sys - or in theory through a CPU hotplug 
| event.
| 
| 	Ingo
|

Ugh... I expected something like "No, look at that code! You've
missed this snippet!". Which would make me happy :-)

I'll take a look.

        Cyrill