[PATCH] OsdSynch.c modernization
Nate Lawson
nate at root.org
Mon Sep 24 10:10:18 PDT 2007
Jung-uk Kim wrote:
> On Monday 24 September 2007 12:28 pm, John Baldwin wrote:
>> On Monday 24 September 2007 12:11:07 pm Nate Lawson wrote:
>>> John Baldwin wrote:
>>>> 2007/9/22, Jung-uk Kim <jkim at freebsd.org>:
>>>>> I thought exactly the same when I started rewriting it (almost
>>>>> half year ago!). I have tried all of the above, spent
>>>>> numerous sleepless nights, and miserably failed. :-(
>>>>>
>>>>> Spin mutex is too restrictive (e.g., it cannot be used with
>>>>> other locks gracefully). critical_enter() causes:
>>>>>
>>>>> panic: blockable sleep lock (sleep mutex) 32 @
>>>>> /usr/src/sys/vm/uma_core.c:1830 cpuid = 0
>>>>> KDB: enter: panic
>>>>> [thread pid 21 tid 100013 ]
>>>>> Stopped at kdb_enter+0x32: leave
>>>> However, disabling interrupts while you block on other locks is
>>>> just as
>> bad,
>>
>>>> we just don't assert for it. Better would be to fix ACPI-CA to
>>>> not try to malloc() while holding a spin lock. You should be
>>>> able to see where it is doing that via the stack trace. If the
>>>> malloc is using M_NOWAIT you will
>> be
>>
>>>> far better off using a plain mutex and just not disabling
>>>> interrupts.
>>> For 7.0, we're going with what we have (sx locks) since it's
>>> well-tested and not wrong, maybe just less than optimal.
>>> Remember that acpi locks are acquired a few dozen times every 10
>>> seconds or so, so this is not at risk of being a performance
>>> issue.
>> Disabling interrupts and then calling malloc() is wrong however.
>
> Understood. As I said earlier, I really like to fix it correctly.
>
> <rant>
> However, the problem is that there are so many different BIOSes out
> there, taking so different code paths. Whenever I thought it's
> fixed, someone says 'you broke my laptop' or 'FreeBSD is bad because
> it doesn't boot on my laptop but Linux and Windows boot fine'. :-(
> </rant>
>
> (At least on my laptop) I found the malloc() was called from our code,
> i.e., AcpiOsExecute() from OsdSched.c. I'll try something shortly
> cause I was going to rewrite the file anyway.
Yep, that's because we need a task structure that's different for each
call and acpi-ca doesn't like the "pending" argument (see
OsdSchedule.c). One fix for this is to just use a hack and cast the fn
to discard the extra arg. Not sure this would work.
I thought malloc(...NOWAIT) *could* be called with a mutex held? It
just checks a list and returns NULL if empty, right?
--
Nate
More information about the freebsd-current
mailing list