Bill Holler wrote:
> 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.
>
^--- NOT
Sorry, I meant to say accessing these registers does *not* halt
the processor.
Bill
> 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
>>>>
>>>>
>>
>>
>
> _______________________________________________
> tesla-dev mailing list
> tesla-dev at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/tesla-dev
>
