Date: Wed, 31 May 2006 11:23:01 -0400 From: John Baldwin <jhb@freebsd.org> To: Scott Long <scottl@samsco.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, Warner Losh <imp@freebsd.org>, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/dev/syscons/apm apm_saver.c src/sys/i386/bios apm.c apm.h Message-ID: <200605311123.02334.jhb@freebsd.org> In-Reply-To: <447DAF19.3050901@samsco.org> References: <200605252306.k4PN6cCS081708@repoman.freebsd.org> <200605311050.12156.jhb@freebsd.org> <447DAF19.3050901@samsco.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 31 May 2006 10:58, Scott Long wrote:
> John Baldwin wrote:
> > 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:
> >
> > softclock()
> > {
> > ...
> > THREAD_NO_SLEEPING();
> > c_func(c_arg);
> > THREAD_SLEEPING_OK();
> > ...
> > }
> >
> > That has been in place since 6.0 IIRC.
> >
>
> I thought that it was only enabled for DIAGNOSTIC.
>
> Scott
Nah, that was phk's other timekeeping code to see which timeouts take a long
time to execute. The THREAD_NO_SLEEPING() stuff was added just before 6.0
was released and replaced a couple of "special" mutexes that were held just
to provoke WITNESS warnings.
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200605311123.02334.jhb>