Re: [PATCH] OsdSynch.c modernization

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 _at_
>>>>> /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