Li, Aubrey wrote:
> 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.
>
To be clear: the BM_STS and PM1 status register access does
halt the CPU. The CPU does not halt holding the lock.
Bill
> 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
>>>
>
>
