From owner-freebsd-current@FreeBSD.ORG  Fri Feb  8 07:37:23 2013
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
Delivered-To: freebsd-current@FreeBSD.org
Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115])
 by hub.freebsd.org (Postfix) with ESMTP id AEA7DEE;
 Fri,  8 Feb 2013 07:37:23 +0000 (UTC) (envelope-from avg@FreeBSD.org)
Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140])
 by mx1.freebsd.org (Postfix) with ESMTP id C175B957;
 Fri,  8 Feb 2013 07:37:22 +0000 (UTC)
Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua
 [212.40.38.100])
 by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id JAA07938;
 Fri, 08 Feb 2013 09:37:21 +0200 (EET) (envelope-from avg@FreeBSD.org)
Received: from localhost ([127.0.0.1])
 by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD))
 id 1U3iWO-0002Tt-Si; Fri, 08 Feb 2013 09:37:20 +0200
Message-ID: <5114AB2E.2050909@FreeBSD.org>
Date: Fri, 08 Feb 2013 09:37:18 +0200
From: Andriy Gapon <avg@FreeBSD.org>
User-Agent: Mozilla/5.0 (X11; FreeBSD amd64;
 rv:17.0) Gecko/20130121 Thunderbird/17.0.2
MIME-Version: 1.0
To: FreeBSD Current <freebsd-current@FreeBSD.org>
Subject: call suspend_cpus() under smp_ipi_mtx
X-Enigmail-Version: 1.4.6
Content-Type: text/plain; charset=X-VIET-VPS
Content-Transfer-Encoding: 7bit
X-BeenThere: freebsd-current@freebsd.org
X-Mailman-Version: 2.1.14
Precedence: list
List-Id: Discussions about the use of FreeBSD-current
 <freebsd-current.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/freebsd-current>, 
 <mailto:freebsd-current-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-current>
List-Post: <mailto:freebsd-current@freebsd.org>
List-Help: <mailto:freebsd-current-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
 <mailto:freebsd-current-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 08 Feb 2013 07:37:23 -0000


Could you please review and/or test the following patch?

The idea is exactly the same as for cpu_stop() invocation in the shutdown path.
Please note that I've kept intr_disable() just because potentially mtx_lock_spin
could be implemented in such a way that it wouldn't block all interrupts via CPU
flags, but would use LAPIC TPR, for example.

I've also decided to add smp_ipi_mtx assertions to suspend_cpus and stop_cpus.

diff --git a/sys/dev/acpica/acpi.c b/sys/dev/acpica/acpi.c
index f0c750f..9750d46 100644
--- a/sys/dev/acpica/acpi.c
+++ b/sys/dev/acpica/acpi.c
@@ -2741,6 +2741,15 @@ acpi_EnterSleepState(struct acpi_softc *sc, int state)
     if (sc->acpi_sleep_delay > 0)
 	DELAY(sc->acpi_sleep_delay * 1000000);

+#ifdef SMP
+    /*
+     * Prevent inter-CPU deadlock possibility between suspend_cpus()
+     * and other inter-CPU synchronous calls like smp_rendezvous and
+     * TLB shootdowns.
+     */
+    if (state != ACPI_STATE_S1)
+	mtx_lock_spin(&smp_ipi_mtx);
+#endif
     intr = intr_disable();
     if (state != ACPI_STATE_S1) {
 	sleep_result = acpi_sleep_machdep(sc, state);
@@ -2784,6 +2793,9 @@ acpi_EnterSleepState(struct acpi_softc *sc, int state)
 	}

 	intr_restore(intr);
+#ifdef SMP
+        mtx_unlock_spin(&smp_ipi_mtx);
+#endif

 	/* call acpi_wakeup_machdep() again with interrupt enabled */
 	acpi_wakeup_machdep(sc, state, sleep_result, 1);
diff --git a/sys/kern/subr_smp.c b/sys/kern/subr_smp.c
index 3614798..edf16db 100644
--- a/sys/kern/subr_smp.c
+++ b/sys/kern/subr_smp.c
@@ -260,6 +260,7 @@ int
 stop_cpus(cpuset_t map)
 {

+	mtx_assert(&smp_ipi_mtx, MA_OWNED);
 	return (generic_stop_cpus(map, IPI_STOP));
 }

@@ -275,6 +276,7 @@ int
 suspend_cpus(cpuset_t map)
 {

+	mtx_assert(&smp_ipi_mtx, MA_OWNED);
 	return (generic_stop_cpus(map, IPI_SUSPEND));
 }
 #endif

-- 
Andriy Gapon