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>