Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 25 May 2006 22:06:11 -0600 (MDT)
From:      Warner Losh <imp@bsdimp.com>
To:        scottl@samsco.org
Cc:        cvs-src@freebsd.org, src-committers@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:  <20060525.220611.74708877.imp@bsdimp.com>
In-Reply-To: <44766F75.9060100@samsco.org>
References:  <200605252306.k4PN6cCS081708@repoman.freebsd.org> <44766F75.9060100@samsco.org>

next in thread | previous in thread | raw e-mail | index | archive | help
From: Scott Long <scottl@samsco.org>
Subject: Re: cvs commit: src/sys/dev/syscons/apm apm_saver.c src/sys/i386/bios apm.c apm.h
Date: Thu, 25 May 2006 21:01:09 -0600

> 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.

At the very least, we should mandate that timeouts are a non-sleepable
event.  Sleeping just doesn't work there.  taskqueues, I'm less sure
of, since short sleeps there work, but do degrade performance.  I like
this idea.

Warner



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