On 18/05/2015 14:29, Andriy Gapon wrote:
> On 09/05/2015 15:28, Konstantin Belousov wrote:
>> +void
>> +acpi_cpu_idle_mwait(uint32_t mwait_hint)
>> +{
>> +    int *state;
>> +
>> +    state = (int *)PCPU_PTR(monitorbuf);
>> +    /*
>> +     * XXXKIB.  Software coordination mode should be supported,
>> +     * but all Intel CPUs provide hardware coordination.
>> +     */
>> +    cpu_monitor(state, 0, 0);
>> +    cpu_mwait(MWAIT_INTRBREAK, mwait_hint);
>> +}
>> +
> 
> Kostik,
> 
> it's been a while since I studied this code, so please pardon me if I am 
> asking
> something obvious or silly.
> 
> I wonder why this function does not set 'state' before monitor + mwait.
> As far as I can see, all other idling functions do that.  And 
> cpu_idle_wakeup()
> compares the state to STATE_MWAIT before changing it.
> 
> So, I am concerned that if the state happens to be anything other than
> STATE_MWAIT when acpi_cpu_idle_mwait() is called, then cpu_idle_wakeup() won't
> wake up the idled CPU.  It seems that if the state is not STATE_SLEEPING then 
> an
> IPI won't be sent either.  Actually, that leaves STATE_RUNNING is the only
> problematic case, but that's probably the state that the CPU would have before
> idling.
> 

After having written the above I realized what I overlooked:
acpi_cpu_idle_mwait() is called from the ACPI idle method, so the state must
already be set to STATE_SLEEPING.
So, looks like the wake-up would always be done by an IPI...

Just in case, here's what I had in my old local code:
void
acpi_cpu_mwait_cx(u_int hints)
{
        int *state;

        state = (int *)PCPU_PTR(monitorbuf);
        KASSERT(*state == STATE_SLEEPING,
            ("cpu_mwait_cx: wrong monitorbuf state"));
        *state = STATE_MWAIT;
        cpu_monitor(state, 0, 0);
        if (*state == STATE_MWAIT)
                cpu_mwait(MWAIT_INTR_BRK, hints);

        /*
         * We should exit on any event that interrupts mwait,
         * because that event might be a wanted interrupt.
         */
        *state = STATE_RUNNING;
}

This code also accounted for a time window between the CPU wanting to go idle
and it calling mwait. During that window other CPU could want to wake up the
first CPU.

-- 
Andriy Gapon
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"

Reply via email to