cvs commit: src/sys/dev/syscons/apm apm_saver.c
src/sys/i386/bios apm.c apm.h
jhb at freebsd.org
Wed May 31 07:50:32 PDT 2006
On Thursday 25 May 2006 23:01, Scott Long wrote:
> Warner Losh wrote:
> > imp 2006-05-25 23:06:38 UTC
> > FreeBSD src repository
> > Modified files:
> > sys/dev/syscons/apm apm_saver.c
> > sys/i386/bios apm.c apm.h
> > Log:
> > APM was calling the suspend process from a timeout. This meant that
> > other timeouts could not happen while suspending, including timeouts
> > for things like msleep. This caused the system to hang on suspend
> > when the cbb was enabled, since its suspend path powered down the
> > socket which used a timeout to wait for it to be done.
> > APM now creates a thread when it is enabled, and deletes the thread
> > when it is disabled. This thread takes the place of the timeout by
> > doing its polling every ~.9s. When the thread is disabled, it will
> > wakeup early, otherwise it times out and polls the varius things the
> > old timeout polled (APM events, suspend delays, etc).
> > This makes my Sony VAIO 505TS suspend/resume correctly when APM is
> > enabled (ACPI is black listed on my 505TS).
> > This will likely fix other problems with the suspend path where
> > drivers would sleep with msleep and/or do other timeouts. Maybe
> > there's some special case code that would use DELAY while suspending
> > and msleep otherwise that can be revisited and removed.
> > This was also tested by glebius@, who pointed out that in the patch I
> > sent him, I'd forgotten apm_saver.c
> > MFC After: 3 weeks
> In the past, I've been against mandating that callouts/timeouts/generic
> taskqueues should not be allowed to sleep. However, after looking over
> the history of this problem as well as others, it seems that it's just
> too easy for driver authors to make bad assumptions and wind up with a
> priority inversion/deadlock like this. It would be relatively trivial
> to mark these contexts as being non-sleepable and have the msleep code
> enforce it, like is done with ithreads. What do you think? Anyways,
> thanks for looking at this and fixing it.
We already do for timeouts if INVARIANTS is on:
That has been in place since 6.0 IIRC.
More information about the cvs-src