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.

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