mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Andi Kleen <ak@suse.de>, <linux-kernel@vger.kernel.org>,
	Neil Brown <neilb@suse.de>, "Rafael J. Wysocki" <rjw@sisk.pl>,
	Ingo Molnar <mingo@elte.hu>,
	Zwane Mwaikambo <zwane@linuxpower.ca>
Subject: [PATCH] x86: Document the hotplug code is incompatible with x86 irq handling
Date: Thu, 31 May 2007 07:32:05 -0600	[thread overview]
Message-ID: <m1tztt3yqy.fsf@ebiederm.dsl.xmission.com> (raw)


I just realized that except for doing the code review and noticing
that the current cpu hotplug code is fundamentally incompatible
with x86 I haven't done anything about it.  So here is my patch
to document what is wrong.

The current cpu hotplug code requires irqs to be migrated from a cpu
outside of irq context.  On x86 ioapics simply do not support this,
making the code unfixable without major redesign of the generic cpu
hotplug code.

So this patch makes CPU_HOTPLUG on x86 depend on CONFIG_BROKEN
and adds a WARN_ON so people that do enable it are not in doubt about
which part of the code is broken, even if it does work for them.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 arch/i386/Kconfig        |    2 +-
 arch/i386/kernel/irq.c   |   13 +++++++++++++
 arch/x86_64/Kconfig      |    2 +-
 arch/x86_64/kernel/irq.c |   13 +++++++++++++
 4 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/arch/i386/Kconfig b/arch/i386/Kconfig
index 8770a5d..74444c1 100644
--- a/arch/i386/Kconfig
+++ b/arch/i386/Kconfig
@@ -892,7 +892,7 @@ config PHYSICAL_ALIGN
 
 config HOTPLUG_CPU
 	bool "Support for suspend on SMP and hot-pluggable CPUs (EXPERIMENTAL)"
-	depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER
+	depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER && BROKEN
 	---help---
 	  Say Y here to experiment with turning CPUs off and on, and to
 	  enable suspend on SMP systems. CPUs can be controlled through
diff --git a/arch/i386/kernel/irq.c b/arch/i386/kernel/irq.c
index d2daf67..42aeccb 100644
--- a/arch/i386/kernel/irq.c
+++ b/arch/i386/kernel/irq.c
@@ -312,6 +312,19 @@ void fixup_irqs(cpumask_t map)
 	unsigned int irq;
 	static int warned;
 
+	/* 
+	 * Function is so wrong at so many levels.
+	 * - We migrate irqs that are directed at the cpu we are
+	 *   removing.
+	 * - We cannot safely migrate ioapic irqs on x86 except in
+	 *   side of irq context.
+	 * - We are enabling irqs when the interface requires irqs to
+	 *   be disabled.
+	 * Since someone probably finds this useful just warn very
+	 * loudly until cpu hotplug is redesigned.
+	 */
+	WARN_ON(1);
+
 	for (irq = 0; irq < NR_IRQS; irq++) {
 		cpumask_t mask;
 		if (irq == 2)
diff --git a/arch/x86_64/Kconfig b/arch/x86_64/Kconfig
index 5ce9443..a61c4f2 100644
--- a/arch/x86_64/Kconfig
+++ b/arch/x86_64/Kconfig
@@ -429,7 +429,7 @@ config NR_CPUS
 
 config HOTPLUG_CPU
 	bool "Support for suspend on SMP and hot-pluggable CPUs (EXPERIMENTAL)"
-	depends on SMP && HOTPLUG && EXPERIMENTAL
+	depends on SMP && HOTPLUG && EXPERIMENTAL && BROKEN
 	help
 		Say Y here to experiment with turning CPUs off and on.  CPUs
 		can be controlled through /sys/devices/system/cpu/cpu#.
diff --git a/arch/x86_64/kernel/irq.c b/arch/x86_64/kernel/irq.c
index 3eaceac..da6f282 100644
--- a/arch/x86_64/kernel/irq.c
+++ b/arch/x86_64/kernel/irq.c
@@ -142,6 +142,19 @@ void fixup_irqs(cpumask_t map)
 	unsigned int irq;
 	static int warned;
 
+	/* 
+	 * Function is so wrong at so many levels.
+	 * - We migrate irqs that are directed at the cpu we are
+	 *   removing.
+	 * - We cannot safely migrate ioapic irqs on x86 except in
+	 *   side of irq context.
+	 * - We are enabling irqs when the interface requires irqs to
+	 *   be disabled.
+	 * Since someone probably finds this useful just warn very
+	 * loudly until cpu hotplug is redesigned.
+	 */
+	WARN_ON(1);
+
 	for (irq = 0; irq < NR_IRQS; irq++) {
 		cpumask_t mask;
 		if (irq == 2)
-- 
1.5.1.1.181.g2de0


             reply	other threads:[~2007-05-31 13:33 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-31 13:32 Eric W. Biederman [this message]
2007-06-07 14:01 ` Pavel Machek
2007-06-12 18:19   ` Eric W. Biederman
2007-06-12 20:52     ` Rafael J. Wysocki
2007-06-12 21:56       ` Siddha, Suresh B
2007-06-12 22:16         ` Rafael J. Wysocki
2007-06-12 22:24           ` Siddha, Suresh B
2007-06-12 22:58             ` Rafael J. Wysocki
2007-06-22 17:27               ` Eric W. Biederman
     [not found] <fa.tUMR7tAB+jMgtfyl/LJ7U9QMgBs@ifi.uio.no>
2007-05-31 14:34 ` Robert Hancock
2007-05-31 15:47   ` Eric W. Biederman
2007-05-31 20:12     ` Rafael J. Wysocki
2007-06-01 19:48       ` Eric W. Biederman
2007-06-01 20:06         ` Rafael J. Wysocki
2007-06-01 20:29           ` Eric W. Biederman
2007-06-01 20:44             ` Rafael J. Wysocki
2007-06-01 20:34         ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m1tztt3yqy.fsf@ebiederm.dsl.xmission.com \
    --to=ebiederm@xmission.com \
    --cc=ak@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=neilb@suse.de \
    --cc=rjw@sisk.pl \
    --cc=zwane@linuxpower.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®