Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 23 Mar 2013 11:48:50 +0200
From:      Andriy Gapon <avg@FreeBSD.org>
To:        FreeBSD Current <freebsd-current@FreeBSD.org>, freebsd-hackers@FreeBSD.org, freebsd-acpi@FreeBSD.org
Subject:   Re: call suspend_cpus() under smp_ipi_mtx
Message-ID:  <514D7A82.9000105@FreeBSD.org>
In-Reply-To: <5114AB2E.2050909@FreeBSD.org>
References:  <5114AB2E.2050909@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help

Looks like this issue needs more thinking and discussing.

The basic idea is that suspend_cpus() must be called with smp_ipi_mtx held (on
SMP systems).
This is for exactly the same reasons as to why we first take smp_ipi_mtx before
calling stop_cpus() in the shutdown path.  Essentially one CPU could be holding
smp_ipi_mtx (and thus with interrupts disabled[*]) and waiting for an
acknowledgement from other CPUs (e.g. in smp_rendezvous or in a TLB shootdown),
while another CPU could be with interrupts disabled (explicitly - like in the
shutdown or ACPI suspend paths) and trying to deliver an IPI to other CPUs.

In my opinion, we must consistently use the same lock, smp_ipi_mtx, for all
regular (non-NMI) synchronous IPI-based communication between CPUs.  Otherwise a
deadlock is quite possible.

Some obstacles for just going ahead and making the suggested change:

- acpi_sleep_machdep() calls intr_suspend() with interrupts disabled; currently
witness(9) is not aware of that, but if smp_ipi_mtx spin-lock is used, then we
would have to make intr_table_lock and msi_lock the spin-locks as well;
- AcpiLeaveSleepStatePrep() (from ACPICA) is called with interrupts disabled and
currently it performs an action that requires memory allocation; again, with
interrupts disabled via intr_disable() this fact is not visible to witness, etc,
but with smp_ipi_mtx it needs to be somehow handled.

I talked to ACPICA guys about the last issue and they told me that what is
currently done in AcpiLeaveSleepStatePrep does not need to be with interrupts
disabled and can be moved to AcpiLeaveSleepState.  This is after the _BFS and
_GTS support was removed.

What do you think?
Thank you.
-- 
Andriy Gapon



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