Dana.Myers wrote:

> Li, Aubrey wrote:
>> Dana.Myers wrote:
>> 
>>> Let's take a step back here and think about what we really
>>> want to happen.
>>> 
>>> Cx is controlled using P_BLK registers, which are a per-CPU
>>> hardware resource; is it correct to assume that manipulation of
>>> a P_BLK is always done on the corresponding CPU?  Another way of
>>> stating this - is it correct to assume that a P_BLK is never
>>> manipulated by another CPU?
>> 
>> No, we are not talking about P_BLK, we are talking about BM_STS in
>> PM1 Status Registers, which is not a per-CPU hardware resource, and
>> BM_RLD as well. 
>> 
> Ah, well.  Something seems wrong with a thread holding a machine
> global lock on machine global resources on a CPU that's not executing
> - while other CPUs may contend for that same lock.  Help me
> understand the use-case and why this isn't bad.

The problem is, currently we are using adaptive mutex for this global
lock,
and we are in idle thread. So one cpu hold this lock, and another cpu
has
to wait. We can keep spinning here, and we also can put this cpu's
thread to 
sleep. Here, we are not spinning here, we are trying to put this thread
to sleep.
But, this is idle thread, it can't be put to sleep.

Thanks,
-Aubrey


> 
> Thanks -
> Dana
>> Thanks,
>> -Aubrey
>> 
>>> 
>>> 
>>> Li, Aubrey wrote:
>>>> Dana.Myers wrote:
>>>> 
>>>>> Li, Aubrey wrote:
>>>>>> I would suggest this following patch for this issue:
>>>>>> =====================================
>>>>>> diff -r cb29bc2e1e6e usr/src/uts/intel/io/acpica/osl.c
>>>>>> --- a/usr/src/uts/intel/io/acpica/osl.c  Sun Jul 13 14:36:23 2008
>>>>>> -0700 +++ b/usr/src/uts/intel/io/acpica/osl.c    Mon Jul 14
10:44:33
>>>>>>                  2008 +0800 @@ -439,7 +439,7 @@ return
(AE_BAD_PARAMETER);
>>>>>>          mp = (kmutex_t *)kmem_alloc(sizeof (kmutex_t),
KM_SLEEP);
>>>>>> -        mutex_init(mp, NULL, MUTEX_DRIVER, NULL);
>>>>>> +        mutex_init(mp, NULL, MUTEX_SPIN, (void *)PIL_MAX);
>>>>>>          *OutHandle = (ACPI_HANDLE)mp;
>>>>>>          return (AE_OK);
>>>>>> =====================================
>>>>>> 
>>>>>> Any thoughts?
>>>>>> 
>>>>> Absolutely.  I'm not yet certain what the semantics of a Linux
>>>>> spinlock are and whether the semantics of a Solaris mutex(...
>>>>> MUTEX_SPIN ...) truly match it.
>>>>> 
>>>>> What are the semantics of a Linux spinlock?  I'll take a look at
>>>>> this in the morning.
>>>> If I understand correctly, spinlock is a very simple single-holder
>>>> lock, designed for multiprocessor and useless in UP environment. If
>>>> a process attempts to acquire a spinlock and it's unavailable, the
>>>> process will keep trying(spinning) until it can acquire the lock.
>>>> 
>>>> Thanks,
>>>> -Aubrey
>>>> 
>>>>> Cheers,
>>>>> Dana
>>>>>> Thanks,
>>>>>> -Aubrey
>>>>>> 
>>>>>> Li, Aubrey wrote:
>>>>>> 
>>>>>> 
>>>>>>> Liu, Jiang wrote:
>>>>>>> 
>>>>>>> 
>>>>>>>> Hi Aubrey,
>>>>>>>>        Currently solaris ACPICA OSL layer uses adaptive mutex
to
>>>>>>>> implement ACPI Lock which seems not confirm to ACPICA
>>>>>>>> programmer manual. Please refer to Section 7.4.5 of "ACPI
>>>>>>>> Component Architecture Programmer Reference" as below:
>>>>>>>> 
>>>>>>>> 7.4.5 AcpiOsCreateLock
>>>>>>>> Functional Description:
>>>>>>>> Create a spin lock. Spin locks are used in the ACPI CA
>>>>>>>> subsystem only when there is requirement for mutual exclusion
>>>>>>>> on data structures that are accessed by both interrupt
>>>>>>>> handlers and normal code. 
>>>>>>>> 
>>>>>>> Yeah, from ACPICA manual, hardware lock must a spin lock.
>>>>>>> Solaris OSL initialize it as MUTEX_DRIVER, which could be either
>>>>>>> MUTEX_ADAPTIVE or MUTEX_SPIN depending on the iblock cookie.
>>>>>>> Since iblock cookie is NULL for AcpiGbl_HardwareLock, so it's
>>>>>>> adaptive. I think it should be initialized as MUTEX_SPIN.
>>>>>>> 
>>>>>>> Thanks,
>>>>>>> -Aubrey
>>>>>>> 
>>>>>>> 
>>>>>>>> Aubrey Li <> wrote:
>>>>>>>> 
>>>>>>>>> On Sat, Jul 12, 2008 at 5:49 AM, Bill Holler
>>>>>>>>> <Bill.Holler at sun.com> wrote:
>>>>>>>>> 
>>>>>>>>>> Dana H. Myers wrote:
>>>>>>>>>> 
>>>>>>>>>>> Bill Holler wrote:
>>>>>>>>>>> 
>>>>>>>>>>>> Hi Aubrey,
>>>>>>>>>>>> 
>>>>>>>>>>>> I suspect we should raise the interrupt priority level high
>>>>>>>>>>>> enough for mutex_enter() to treat AcpiGbl_HardwareLock as
>>>>>>>>>>>> a spin lock. 
>>>>>>>>>>>> 
>>>>>>>>>> I was thinking of something else.  Raising the spl will not
>>>>>>>>>> help. 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>>> AcpiGbl_HardwareLock is declared as an ACPI_SPINLOCK.
>>>>>>>>>>>> AcpiOsCreateLock() initializes AcpiGbl_HardwareLock as a
>>>>>>>>>>>> MUTEX_DRIVER.  The mutex must be an ADAPTIVE lock.
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>> What is the idle code doing with interrupts while it places
>>>>>>>>>>>> the cpu into deeper c-states with the ACPI interface?
>>>>>>>>>>>> I assume it will be like the current cpu_halt() function
>>>>>>>>>>>> masks interrupts before halting the processor?
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>> I'd like to understand better the exposure here.  It's true
>>>>>>>>>>> that AcpiOsCreateLock() uses adaptive mutexes; this hasn't
>>>>>>>>>>> appeared to present a problem in the past and I'd like
>>>>>>>>>>> to understand the specific cases where a spin mutex
>>>>>>>>>>> is truly required.  Perhaps the problem is better addressed
>>>>>>>>>>> at a higher algorithmic level, for example.
>>>>>>>>>>> 
>>>>>>>>>> The ACPI interfaces need to be used to halt the processor
>>>>>>>>>> into ACPI C2 and C3.  The idle thread cannot block on a mutex
>>>>>>>>>> because the idle thread must always be dispatch-able.
>>>>>>>>>> It would be much simpler if the idle thread can use the ACPI
>>>>>>>>>> interface instead of alternatives such as creating a new
>>>>>>>>>> block-able thread to do this. 
>>>>>>>>>> 
>>>>>>>>>> AcpiGbl_HardwareLock is defined as an ACPI_"SPIN"LOCK.  :-)
>>>>>>>>>> On the other hand, changing AcpiGbl_HardwareLock does
>>>>>>>>>> sound risky.
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>>> We need interrupts to be masked when the processor wakes
>>>>>>>>>>>> up, so we can fixup the local APIC state.
>>>>>>>>>>>> 
>>>>>>>>>>> It sounds like there's a local CPU vs. other CPU issue
>>>>>>>>>>> with respect to masking interrupts then?
>>>>>>>>>>> 
>>>>>>>>>> This is a related but different issue than the above idle
>>>>>>>>>> thread blocking in the ACPI interfaces.
>>>>>>>>>> 
>>>>>>>>>> The current idle loop services interrupts from the HALT state
>>>>>>>>>> before continuing with the idle loop.  CPUs in deep C-state
>>>>>>>>>> will have to execute some code to cleanup cpu state before
>>>>>>>>>> servicing interrupts. 
>>>>>>>>>> 
>>>>>>>>>> One option is to have the hardware return control to the idle
>>>>>>>>>> thread with interrupts masked.  Another option is to check
>>>>>>>>>> for deep c-state cleanup in the interrupt path similar to
>>>>>>>>>> tlb_service(). 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> I think the issue might be different.
>>>>>>>>> The following is what idle code doing.
>>>>>>>>> cpu_acpi_idle()
>>>>>>>>> {
>>>>>>>>>         value  = AcpiGetRegister();  /* need a lock */
>>>>>>>>>         using value to determine next idle type;
>>>>>>>>>         cli();
>>>>>>>>>         place the processor into Cx;
>>>>>>>>>         sti();
>>>>>>>>> }
>>>>>>>>> 
>>>>>>>>> The problem occurs in AcpiGetRegister(), it uses
>>>>>>>>> AcpiGbl_HardwareLock. I suspect AcpiGbl_HardwareLock really
>>>>>>>>> works as a spin lock. Otherwise it will not go into
>>>>>>>>> turnstiles block. 
>>>>>>>>> 
>>>>>>>>> Thanks,
>>>>>>>>> -Aubrey
>>>>>>>>> _______________________________________________
>>>>>>>>> tesla-dev mailing list
>>>>>>>>> tesla-dev at opensolaris.org
>>>>>>>>> http://mail.opensolaris.org/mailman/listinfo/tesla-dev
>>>>>>>>> 
>>>>>>>> _______________________________________________
>>>>>>>> tesla-dev mailing list
>>>>>>>> tesla-dev at opensolaris.org
>>>>>>>> http://mail.opensolaris.org/mailman/listinfo/tesla-dev
>>>>>>>> 
>>>>>>> _______________________________________________
>>>>>>> tesla-dev mailing list
>>>>>>> tesla-dev at opensolaris.org
>>>>>>> http://mail.opensolaris.org/mailman/listinfo/tesla-dev
>>>> _______________________________________________
>>>> tesla-dev mailing list
>>>> tesla-dev at opensolaris.org
>>>> http://mail.opensolaris.org/mailman/listinfo/tesla-dev
>> 
>> _______________________________________________
>> tesla-dev mailing list
>> tesla-dev at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/tesla-dev


Reply via email to