ath / 802.11n performance issues and timer code
attilio at freebsd.org
Tue Sep 27 13:57:55 UTC 2011
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
>> * 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
>> 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.
> if (sched_runnable())
> __asm __volatile("sti; hlt");
> *state = STATE_RUNNING;
> I don't know if it is possible to do the same thing with the mips "wait"
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.
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).
Peace can only be achieved by understanding - A. Einstein
More information about the freebsd-current