Date: Tue, 27 Sep 2011 15:57:53 +0200 From: Attilio Rao <attilio@freebsd.org> To: John Baldwin <jhb@freebsd.org> Cc: Adrian Chadd <adrian@freebsd.org>, freebsd-current@freebsd.org, freebsd-mips@freebsd.org Subject: Re: ath / 802.11n performance issues and timer code Message-ID: <CAJ-FndBve%2BkHTMzrkPUrkrcJZa0oREF6EuTn3O%2B2Gnvy3s8=ag@mail.gmail.com> In-Reply-To: <201109270951.07839.jhb@freebsd.org> References: <CAJ-VmomZyDJV62yCQOvG=UB6H4wfz9=3_cWzEL7vWAA14TCyYA@mail.gmail.com> <CAJ-VmonbAgsNjdCstd_Ap6JBqowD1NX0J5rQ=t9ideaiXTXd%2BA@mail.gmail.com> <CAJ-VmonJRLQFanKx2VJoigm=SUmMBuYPJT%2BxfAisO9q%2B=qTShw@mail.gmail.com> <201109270951.07839.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
2011/9/27 John Baldwin <jhb@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 t=
he
> cpu_idle() implementations. =C2=A0On x86 the idle hooks all check sched_r=
unnable()
> with interrupts disabled and then atomically re-enable interrupts and sle=
ep
> only if that is false, e.g.:
>
> static void
> cpu_idle_hlt(int busy)
> {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0int *state;
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0state =3D (int *)PCPU_PTR(monitorbuf);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0*state =3D STATE_SLEEPING;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0/*
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 * We must absolutely guarentee that hlt is th=
e next instruction
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 * after sti or we introduce a timing window.
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0disable_intr();
> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (sched_runnable())
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0enable_intr();
> =C2=A0 =C2=A0 =C2=A0 =C2=A0else
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0__asm __volatile("=
sti; hlt");
> =C2=A0 =C2=A0 =C2=A0 =C2=A0*state =3D 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=3D1)
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
--=20
Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndBve%2BkHTMzrkPUrkrcJZa0oREF6EuTn3O%2B2Gnvy3s8=ag>