Re: [Resend][PATCH] ACPI / EC: Remove race between EC driver and suspend process (rev. 3)

[Rafael J. Wysocki]

On Thursday 04 March 2010, Alan Jenkins wrote:
> On 3/4/10, Len Brown <lenb@xxxxxxxxxx> wrote:
> > Isn't the problem at hand that the boot-kernel stops
> > in mid-transaction, and the resumed hibernate image
> > then blunders forward with its first transaction
> > only to find the EC in an intermediate state?
> >
> > If yes, I then making the transaction atomic WRT
> > the boot kernel stopping should fix the problem.
> >
> > The patch sets a FROZEN bit, and subsequent
> > requests for EC transactions simply fail.
> > How do we know that we set the bit at the right time?

Well, setting it later simply doesn't make sense and setting it earlier might
disturb things.

> > What transactions or parts of transactions will fail,
> > and what are the consequences of those failures?
> 
> The problem transactions on my machine were probably started by
> ec_check_sci().  I.e. QUERY transactions which read the value of an
> event raised by the EC.  (I triggered the problem by pressing ACPI
> hotkeys, although I haven't been able to reproduce it since).
> 
> That might explain why the transactions can be asynchronous with the
> s2disk task - because these transactions are run in the ACPI
> workqueue.  Perhaps the workqueue needs to be flushed after the EC GPE
> is disabled?

Yes, and that's what the patch does.  It calls acpi_os_wait_events_complete()
to flush the workqueue (actually, two of them) and _then_ sets the FROZEN
bit.

Dropping stuff on the floor after the FROZEN bit has been set doesn't change
things too much, because we're going to turn interrupts off shortly after that.
However, the FROZEN bit is set under the EC mutex, which automatically makes
us wait if any transaction is in progress while we're trying to set the FROZEN
bit.

Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/