Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 12 Oct 2010 17:40:45 +0000 (UTC)
From:      Andriy Gapon <avg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r213736 - head/sys/kern
Message-ID:  <201010121740.o9CHejUr055340@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: avg
Date: Tue Oct 12 17:40:45 2010
New Revision: 213736
URL: http://svn.freebsd.org/changeset/base/213736

Log:
  generic_stop_cpus: prevent parallel execution
  
  This is based on the same approach as used in panic().
  In theory parallel execution of generic_stop_cpus()  could lead to two CPUs
  stopping each other and everyone else, and thus a total system halt.
  Also, in theory, we should have some smarter locking here, because two
  (or more CPUs) could be stopping unrelated sets of CPUs.
  But in practice, it seems, this function is only used to stop
  "all other" CPUs.
  
  Additionally, I took this opportunity to make amd64-specific suspend_cpus()
  function use generic_stop_cpus() instead of rolling out essentially
  duplicate code.
  
  This code is based on code by Sandvine Incorporated.
  
  Suggested by:	mdf
  Reviewed by:	jhb, jkim (earlier version)
  MFC after:	2 weeks

Modified:
  head/sys/kern/subr_smp.c

Modified: head/sys/kern/subr_smp.c
==============================================================================
--- head/sys/kern/subr_smp.c	Tue Oct 12 17:16:51 2010	(r213735)
+++ head/sys/kern/subr_smp.c	Tue Oct 12 17:40:45 2010	(r213736)
@@ -198,22 +198,32 @@ forward_signal(struct thread *td)
  *   0: NA
  *   1: ok
  *
- * XXX FIXME: this is not MP-safe, needs a lock to prevent multiple CPUs
- *            from executing at same time.
  */
 static int
 generic_stop_cpus(cpumask_t map, u_int type)
 {
+	static volatile u_int stopping_cpu = NOCPU;
 	int i;
 
-	KASSERT(type == IPI_STOP || type == IPI_STOP_HARD,
+	KASSERT(
+#if defined(__amd64__)
+	    type == IPI_STOP || type == IPI_STOP_HARD || type == IPI_SUSPEND,
+#else
+	    type == IPI_STOP || type == IPI_STOP_HARD,
+#endif
 	    ("%s: invalid stop type", __func__));
 
 	if (!smp_started)
-		return 0;
+		return (0);
 
 	CTR2(KTR_SMP, "stop_cpus(%x) with %u type", map, type);
 
+	if (stopping_cpu != PCPU_GET(cpuid))
+		while (atomic_cmpset_int(&stopping_cpu, NOCPU,
+		    PCPU_GET(cpuid)) == 0)
+			while (stopping_cpu != NOCPU)
+				cpu_spinwait(); /* spin */
+
 	/* send the stop IPI to all CPUs in map */
 	ipi_selected(map, type);
 
@@ -230,7 +240,8 @@ generic_stop_cpus(cpumask_t map, u_int t
 #endif
 	}
 
-	return 1;
+	stopping_cpu = NOCPU;
+	return (1);
 }
 
 int
@@ -248,50 +259,11 @@ stop_cpus_hard(cpumask_t map)
 }
 
 #if defined(__amd64__)
-/*
- * When called the executing CPU will send an IPI to all other CPUs
- *  requesting that they halt execution.
- *
- * Usually (but not necessarily) called with 'other_cpus' as its arg.
- *
- *  - Signals all CPUs in map to suspend.
- *  - Waits for each to suspend.
- *
- * Returns:
- *  -1: error
- *   0: NA
- *   1: ok
- *
- * XXX FIXME: this is not MP-safe, needs a lock to prevent multiple CPUs
- *            from executing at same time.
- */
 int
 suspend_cpus(cpumask_t map)
 {
-	int i;
 
-	if (!smp_started)
-		return (0);
-
-	CTR1(KTR_SMP, "suspend_cpus(%x)", map);
-
-	/* send the suspend IPI to all CPUs in map */
-	ipi_selected(map, IPI_SUSPEND);
-
-	i = 0;
-	while ((stopped_cpus & map) != map) {
-		/* spin */
-		cpu_spinwait();
-		i++;
-#ifdef DIAGNOSTIC
-		if (i == 100000) {
-			printf("timeout suspending cpus\n");
-			break;
-		}
-#endif
-	}
-
-	return (1);
+	return (generic_stop_cpus(map, IPI_SUSPEND));
 }
 #endif

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201010121740.o9CHejUr055340>