cvs commit: src/sys/dev/syscons/apm apm_saver.c src/sys/i386/bios apm.c apm.h

[Scott Long]

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