Re: ACPI-CA update patch for review

[YONETANI Tomokazu <qhwt+dfly@xxxxxxxxxx>]

On Tue, Jan 09, 2007 at 07:15:21AM +0100, Simon 'corecode' Schubert wrote:
> YONETANI Tomokazu wrote:
> >Was it a complete hang, or stuck for a few minutes?  Was that during early
> >in the boot, or sysctl from the command line (either in single- or multi-
> >user mode)?  Also, if you were able to drop into DDB, please send me the
> >backtrace, in particular a few function names below (I mean callers of)
> >AcpiOsWaitSemaphore().
> 
> It was stuck on bootup (the random thing: seedenable + dd).  I did a ^C 
> (after some time) and this was printed:
> 
> ACPI Exception (exutils-0382): AE_TIME, Could not aquire Global Lock 
> [20061109]
> acpi_acad0: On Line
> 
> there were two processes in acsem:  acpi_task and sysctl (why sysctl has to 
> aquire the acpi lock is out of my imagination).  both seem to try to aquire 
> the global lock.  sysctl can happen to run (sleep) on CPU #0 or CPU #1.
[backtrace]

Hmm, this looks very similar to what victor@ has reported before.
I didn't even notice that the global lock has to be rewritten before
looking at your patch you sent me privately.

> on a (maybe) unrelated matter:
> 
> I saw your implementation of AcpiOsAcquireLock is not MP-safe (only uses 
> critical sections) and always allows the idle thread to take the lock (even 
> when it is already held before).  Is this intended?

Originally I thought I had to do it that way because
A. most locking primitives involving switching thread led to a panic
   when it's from the idle_thread (so I needed to choose from
   critical sections/spinlocks),

B. using just spinlocks ended up in a panic when waking up from S3:
   still holding 1 exclusive spinlocks!
   (so I thought I can't use spinlock)

C. idle thread won't usually run any ACPI code unless it's on a UP
   kernel and acpi_cpu takes over the idle_hook, and the current
   implementation uses AcpiHw*() functions with ACPI_MTX_DO_NOT_LOCK
   flag.

But after playing with it a bit more, the spinlock still being held
at lwkt_switch() was held by elsewhere and not in any AcpiOs*() functions.
So using spinlocks for AcpiOs{Acquire,Release}Lock() turned out to be
just OK but it also may have revealed that some other place needed a fix.

> oh well, i see that AS_LOCK(as) == crit_enter().  Obviously, this doesn't 
> work for SMP systems, no?

. .. Or my Athlon64 X2 box isn't actually an SMP system.  Anyway,
I'll try your patch on my system before coming back here.

Cheers.