Date: Thu, 29 Nov 2001 06:31:16 -0800 From: Luigi Rizzo <rizzo@aciri.org> To: Bruce Evans <bde@zeta.org.au> Cc: net@FreeBSD.ORG Subject: Re: Revised polling code for STABLE Message-ID: <20011129063116.B19430@iguana.aciri.org> In-Reply-To: <20011129222256.W1389-100000@gamplex.bde.org> References: <20011126115704.L88153@iguana.aciri.org> <20011129222256.W1389-100000@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Bruce, thanks for the feedback.
Re. the second issue:
the code was placed in kern_clock.c just as a placeholder (and
because i needed to touch a couple of places in that file, i tried
to minimize the number of files affected by the patch). But surely,
the core of the code can go somewhere else, e.g. in net/
(i have had it for a long time in if_ethersubr.c), or maybe
even in a separate file, as you like.
The call to update_poll_threshold() however needs to be done by
either hardclock() or statclock() (i am experimenting with that
right now) because the purpose is to sample what the CPU is doing
when we get that particular interrupt.
I do not know how/if I can replace the schednetisr() with an ordinary timeout:
i need the handler to be invoked as soon as possible after the
clock ticks, as the task it performs are as urgent as interrupt requests.
Certainly I do not want it to be run after other netisr handlers
(this is also why NETISR_POLL is lower than any other NETISR, so
the corresponding handler is called first).
It looked like the schednetisr() was the simplest (one-line change) and
more effective way to achieve this, if you know of other ways involving
timeouts please let me know.
Re. the first one, yes the code might probably benefit from
a bit of rearrangement, such as below
#ifdef XORP_ETHER_POLLING
call _idle_poll
#else
call _vm_page_zero_idle
#endif
testl %eax, %eax
jnz idle_loop
with idle_poll being
int idle_poll() {
{
idle_done=1;
if (poll_handlers > 0) {
int s = splimp();
__asm __volatile("sti" : : : "memory");
retval = ether_poll(poll_burst);
__asm __volatile("cli" : : : "memory");
splx(s);
vm_page_zero_idle();
return 1;
} else
return vm_page_zero_idle();
}
The thing is, with polling you cannot halt the CPU if there are handlers
registered for polling. You can try and deregister polling on
interfaces which are idle, which then should be handled correctly
by the above function.
Does the above make sense ?
cheers
luigi
On Thu, Nov 29, 2001 at 11:26:20PM +1100, Bruce Evans wrote:
> > On Mon, 26 Nov 2001, Luigi Rizzo wrote:
> >
> > > [Bcc to -stable in case someone is interested there]
> > >
> > > Attached you can find the latest version of the polling code
> > > for STABLE (which is useful to make boxes much more robust
> > > to attacks and have much better responsiveness under [over]load).
> > > ...
> > Index: sys/i386/i386/swtch.s
> > ===================================================================
> > RCS file: /home/xorpc/u2/freebsd/src/sys/i386/i386/swtch.s,v
> > retrieving revision 1.89.2.4
> > diff -u -r1.89.2.4 swtch.s
> > --- sys/i386/i386/swtch.s 2001/07/26 02:29:10 1.89.2.4
> > +++ sys/i386/i386/swtch.s 2001/10/26 21:54:19
> > @@ -246,6 +246,17 @@
> > call _procrunnable
> > testl %eax,%eax
> > CROSSJUMP(jnz, sw1a, jz)
> > +#ifdef XORP_ETHER_POLLING
> > + incl _idle_done
> > + pushl _poll_burst
> > + call _ether_poll
> > + addl $4,%esp
> > + sti
> > + nop
> > + cli
> > + testl %eax, %eax
> > + jnz idle_loop
> > +#endif
> > call _vm_page_zero_idle
> > testl %eax, %eax
> > jnz idle_loop
>
> This has some serious bugs:
> (1) ether_poll() is called with interrupts disabled. This breaks fast
> interrupt handlers. ether_poll() uses splimp() so it is apparently
> not intended to run with priority higher than splhigh().
> (2) The sti/cli stuff belongs in ether_poll(), and the test of the return
> code of ether_poll() only works because ether_poll() always returns 1.
> vm_page_zero_idle() and the hlt instruction are never reached and the
> system spins calling ether_poll() from idle until a process becomes
> runnable. ether_poll() needs quite different handling than
> vm_page_zero_idle() since it can reasonably find something to do
> whenever an interface is up, while vm_page_zero_idle() soon runs out
> of things to do if it is called a lot.
>
> > Index: sys/kern/kern_clock.c
>
> It shouldn't be necessary to touch this file. Everything involving
> "ether" certainly doesn't belong here. Everything related to periodic
> polling can be done, probably more efficiently, using schedsoft*() or
> timeout(). E.g., schedsoftnet() arranges for netisrs to be checked
> for at the next clock interrupt. But this can now be done almost as
> well (probably better in -current) using an ordinary timeout.
>
> Bruce
>
>
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-net" in the body of the message
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20011129063116.B19430>