cvs commit: src/sys/i386/bios apm.c

[Kris Kennaway]

John Baldwin wrote:
> On Wednesday 14 November 2007 05:01:32 am Kris Kennaway wrote:
>> Julian Elischer wrote:
>>> julian      2007-11-14 05:43:55 UTC
>>>
>>>   FreeBSD src repository
>>>
>>>   Modified files:
>>>     sys/i386/bios        apm.c 
>>>   Log:
>>>   Apply the same sort of locking done in
>>>      sys/dev/acpica/acpi.c rev 1.196 a while ago:
>>>   
>>>   Grab Giant around calls to DEVICE_SUSPEND/RESUME in
>>>   acpi_SetSleepState().
>>>   If we are resuming non-MPSAFE drivers, they need Giant held for them.
>>>   This may fix some obscure suspend/resume problems.  It has fixed keyrate
>>>   setting problems that were triggered by cardbus (MPSAFE) changing the
>>>   ordering for syscons resume (non-MPSAFE).  Also, add some asserts that
>>>   Giant is held in our suspend/resume and shutdown methods.
>>>   
>>>   Submitted by: Marko Zec
>>>   
>>>   Revision  Changes    Path
>>>   1.149     +10 -0     src/sys/i386/bios/apm.c
>> Why are we adding new unconditional giant acquisitions to the tree? 
>> Devices indicate whether or not they are mpsafe, why can't this be made 
>> conditional?
> 
> Actually, we have no way in new-bus to mark new-bus operations like bus 
> enumeration, etc. MPSAFE, and much of it is not (e.g. 
> attach/detach/suspend/resume, etc. functions).  Additionally, making this 
> conditional would require a scheme where you acquire Giant when you walk down 
> the tree into a non-MPSAFE driver which would add complication to many (if 
> not all) bus drivers.  Considering the rarity of suspend/resume (not a 
> critical-path) the overhead of adding all that may not be worth it.  At some 
> point when we do have locking for all device driver operations this can be 
> removed, but it won't really hurt anything to leave it as it is for now.

OK thanks for the explanation.

Kris