Re: [PATCH] OsdSynch.c modernization

On Monday 24 September 2007 01:03:18 pm Nate Lawson wrote:
> 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?

But not a spin mutex....

-- 
John Baldwin