Re: ath / 802.11n performance issues and timer code

2011/9/27 John Baldwin <jhb_at_freebsd.org>:
> On Monday, September 26, 2011 11:36:26 pm Adrian Chadd wrote:
>> .. and as a follow up (and cc'ing attillo and freebsd-mips, in case
>> it's relevant to other platforms and there's a MIPS specific thing to
>> fix):
>>
>> * 2128: mi_switch to idle
>> * 2129: kern_clocksource.c:762 - ie, cpu_idleclock() has been called
>> * 2130: the ath interrupt comes in
>> * 2134: it's skipped for now as the idle thread is in a critical section
>> * 2136: kern_clocksource.c:266 - ie, getnextcpuevent(), inside
> cpu_idleclock().
>>
>> What I bet is happening is this race between the critical section +
>> cpu_idleclock() and the ath0 interrupt:
>>
>> * idle gets scheduled
>> * critical_enter() is called in the mips cpu_idle() routine
>> * the ath interrupt comes in here and gets handled, but since we're in
>> a critical section, it won't preempt things
>> * the cpu_idleclock() code completes without releasing the preemption,
>> and the only thing that wakes up from that wait is the next interrupt
>> (clock, arge0, etc.)
>
> I think this is a mips-specific bug, though it may be well to audit all the
> cpu_idle() implementations.  On x86 the idle hooks all check sched_runnable()
> with interrupts disabled and then atomically re-enable interrupts and sleep
> only if that is false, e.g.:
>
> static void
> cpu_idle_hlt(int busy)
> {
>        int *state;
>
>        state = (int *)PCPU_PTR(monitorbuf);
>        *state = STATE_SLEEPING;
>        /*
>         * We must absolutely guarentee that hlt is the next instruction
>         * after sti or we introduce a timing window.
>         */
>        disable_intr();
>        if (sched_runnable())
>                enable_intr();
>        else
>                __asm __volatile("sti; hlt");
>        *state = STATE_RUNNING;
> }
>
> I don't know if it is possible to do the same thing with the mips "wait"
> instruction.

After thinking about it I think this check is unnecessary.
sti, infact, doesn't enable interrupts before hlt (it just sets IF=1)
but interrupts can fire only after hlt, thus I don't think a
preemption or interrupts firing there can be possible.

This patch:
http://www.freebsd.org/~attilio/stihlt.patch

removes the check and also replaces simple 'hlt' instruction insertion
in C code with the already defined halt().

I still have to go through Adrian's e-mails so I'm not sure if the
logic you post is going to help him or not, but this is my thinking on
the x86 implementation (specifically).

Comments?

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein