diff options
author | attilio <attilio@FreeBSD.org> | 2009-08-13 17:09:45 +0000 |
---|---|---|
committer | attilio <attilio@FreeBSD.org> | 2009-08-13 17:09:45 +0000 |
commit | e85ca71aadcc42a3f7cc90e6cff96c28f1c54290 (patch) | |
tree | fa01b2cb8792e972d05922a5507e082efa4d4c1d /sys/kern | |
parent | b1786d80d2f6e85141431d158b4cf3a47a2f71ec (diff) | |
download | FreeBSD-src-e85ca71aadcc42a3f7cc90e6cff96c28f1c54290.zip FreeBSD-src-e85ca71aadcc42a3f7cc90e6cff96c28f1c54290.tar.gz |
* Completely Remove the option STOP_NMI from the kernel. This option
has proven to have a good effect when entering KDB by using a NMI,
but it completely violates all the good rules about interrupts
disabled while holding a spinlock in other occasions. This can be the
cause of deadlocks on events where a normal IPI_STOP is expected.
* Adds an new IPI called IPI_STOP_HARD on all the supported architectures.
This IPI is responsible for sending a stop message among CPUs using a
privileged channel when disponible. In other cases it just does match a
normal IPI_STOP.
Right now the IPI_STOP_HARD functionality uses a NMI on ia32 and amd64
architectures, while on the other has a normal IPI_STOP effect. It is
responsibility of maintainers to eventually implement an hard stop
when necessary and possible.
* Use the new IPI facility in order to implement a new userend SMP kernel
function called stop_cpus_hard(). That is specular to stop_cpu() but
it does use the privileged channel for the stopping facility.
* Let KDB use the newly introduced function stop_cpus_hard() and leave
stop_cpus() for all the other cases
* Disable interrupts on CPU0 when starting the process of APs suspension.
* Style cleanup and comments adding
This patch should fix the reboot/shutdown deadlocks many users are
constantly reporting on mailing lists.
Please don't forget to update your config file with the STOP_NMI
option removal
Reviewed by: jhb
Tested by: pho, bz, rink
Approved by: re (kib)
Diffstat (limited to 'sys/kern')
-rw-r--r-- | sys/kern/kern_shutdown.c | 10 | ||||
-rw-r--r-- | sys/kern/subr_kdb.c | 7 | ||||
-rw-r--r-- | sys/kern/subr_smp.c | 25 |
3 files changed, 32 insertions, 10 deletions
diff --git a/sys/kern/kern_shutdown.c b/sys/kern/kern_shutdown.c index 80dda97..0f3a672 100644 --- a/sys/kern/kern_shutdown.c +++ b/sys/kern/kern_shutdown.c @@ -412,9 +412,6 @@ boot(int howto) */ EVENTHANDLER_INVOKE(shutdown_post_sync, howto); - /* XXX This doesn't disable interrupts any more. Reconsider? */ - splhigh(); - if ((howto & (RB_HALT|RB_DUMP)) == RB_DUMP && !cold && !dumping) doadump(); @@ -488,6 +485,13 @@ static void shutdown_reset(void *junk, int howto) { + /* + * Disable interrupts on CPU0 in order to avoid fast handlers + * to preempt the stopping process and to deadlock against other + * CPUs. + */ + spinlock_enter(); + printf("Rebooting...\n"); DELAY(1000000); /* wait 1 sec for printf's to complete and be read */ /* cpu_boot(howto); */ /* doesn't do anything at the moment */ diff --git a/sys/kern/subr_kdb.c b/sys/kern/subr_kdb.c index e6af53e..3e77db7 100644 --- a/sys/kern/subr_kdb.c +++ b/sys/kern/subr_kdb.c @@ -88,7 +88,8 @@ SYSCTL_PROC(_debug_kdb, OID_AUTO, trap_code, CTLTYPE_INT | CTLFLAG_RW, NULL, 0, * Flag indicating whether or not to IPI the other CPUs to stop them on * entering the debugger. Sometimes, this will result in a deadlock as * stop_cpus() waits for the other cpus to stop, so we allow it to be - * disabled. + * disabled. In order to maximize the chances of success, use a hard + * stop for that. */ #ifdef SMP static int kdb_stop_cpus = 1; @@ -226,7 +227,7 @@ kdb_panic(const char *msg) { #ifdef SMP - stop_cpus(PCPU_GET(other_cpus)); + stop_cpus_hard(PCPU_GET(other_cpus)); #endif printf("KDB: panic\n"); panic(msg); @@ -518,7 +519,7 @@ kdb_trap(int type, int code, struct trapframe *tf) #ifdef SMP if ((did_stop_cpus = kdb_stop_cpus) != 0) - stop_cpus(PCPU_GET(other_cpus)); + stop_cpus_hard(PCPU_GET(other_cpus)); #endif kdb_active++; diff --git a/sys/kern/subr_smp.c b/sys/kern/subr_smp.c index d64e806..d28001f 100644 --- a/sys/kern/subr_smp.c +++ b/sys/kern/subr_smp.c @@ -233,18 +233,21 @@ forward_roundrobin(void) * XXX FIXME: this is not MP-safe, needs a lock to prevent multiple CPUs * from executing at same time. */ -int -stop_cpus(cpumask_t map) +static int +generic_stop_cpus(cpumask_t map, u_int type) { int i; + KASSERT(type == IPI_STOP || type == IPI_STOP_HARD, + ("%s: invalid stop type", __func__)); + if (!smp_started) return 0; - CTR1(KTR_SMP, "stop_cpus(%x)", map); + CTR2(KTR_SMP, "stop_cpus(%x) with %u type", map, type); /* send the stop IPI to all CPUs in map */ - ipi_selected(map, IPI_STOP); + ipi_selected(map, type); i = 0; while ((stopped_cpus & map) != map) { @@ -262,6 +265,20 @@ stop_cpus(cpumask_t map) return 1; } +int +stop_cpus(cpumask_t map) +{ + + return (generic_stop_cpus(map, IPI_STOP)); +} + +int +stop_cpus_hard(cpumask_t map) +{ + + return (generic_stop_cpus(map, IPI_STOP_HARD)); +} + #if defined(__amd64__) /* * When called the executing CPU will send an IPI to all other CPUs |