Date: Tue, 29 Apr 2003 17:53:01 -0400 (EDT) From: John Baldwin <jhb@FreeBSD.org> To: Andrew Gallatin <gallatin@cs.duke.edu> Cc: "M. Warner Losh" <imp@bsdimp.com> Subject: Re: cvs commit: src/sys/dev/fxp if_fxp.c if_fxpvar.h Message-ID: <XFMail.20030429175301.jhb@FreeBSD.org> In-Reply-To: <16046.60754.563011.201060@grasshopper.cs.duke.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
On 29-Apr-2003 Andrew Gallatin wrote:
>
> John Baldwin writes:
>
> > test for the special case in foo_intr(). Hmmmm. You almost want a way
> > to turn off the interrupt handler as the very first thing (maybe just mark
> > it as dead if it is still running, but not block) while holding the driver
> > lock, then turn off the ability to generate interrupts, then sit and wait
> > for the interrupt handler to remove itself. Maybe:
> >
> >
> > mtx_lock(&sc->sc_mtx);
> > /* handler no longer executes */
> > bus_disable_intr(inthand_cookie, &sc->sc_mtx);
> > /* dink with device */
> > mtx_unlock(&sc->sc_mtx);
> > bus_teardown_intr(inthande_cookie);
> >
> > With bus_disable_intr() calling a function like this:
> >
> > void
> > ithread_disable_handler(void *cookie, struct mtx *m)
> > {
> > struct intrhand *ih;
> > struct ithread *it;
> >
> > ih = (struct intrhand *)cookie;
> > it = ih->ih_it;
> > mtx_lock(&it->it_lock);
> > if (ithread_is_running) {
> > it->it_need = 1;
> > ih->ih_flags |= IH_DISABLING;
> > ih->ih_mutex = m;
> > mtx_unlock(&it->it_lock);
> > msleep(ih, m, ...);
> > } else {
> > ih->ih_flags |= IH_DISABLED;
> > mtx_unlock(&it->it_lock);
> > }
> > }
> >
> > and some changes to ithread_loop() along the lines of:
> >
> > CTR6(... /* existing */)
> > if (ih->ih_flags & IH_DISABLED)
> > continue;
> > if (ih->ih_flags & IH_DISABLING) {
> > struct mtx *m;
> >
> > m = ih->ih_mutex;
> > ih->ih_flags |= IH_DISABLED;
> > ih->ih_flags &= ~IH_DISABLING;
> > mtx_unlock(&it->it_lock);
> > mtx_lock(&m);
> > wakeup(ih);
> > mtx_unlock(&m);
> > goto restart;
> > }
> > if (ih->ih_flags & IH_DEAD) {
> > TAILQ_REMOVE(...);
> > wakeup(ih);
> > continue;
> > }
> >
> > I would need to hold the ithread lock around the TAILQ_FOREACH()
> > bit and drop it around the execution of the handlers, but I probably
> > need that anyways. Of course, all this really does, is take your
> > gone test and move it up in the stack. You are still checking a flag
> > on every interrupt handler invocation.
>
> Since you we already need to check for IH_DEAD, you could avoid extra
> compares by doing
>
> if (ih->ih_flags & (IH_DISABLED | IH_DISABLING | IH_DEAD)) {
> /* sort out rare cases outside the critical path */
> }
>
> Now we're doing only one check in the common case on something that
> you apparently need to check anyway. I'd be all for something like
> this.
Well, this also now involves extra locking in ithread_loop() (though we
should be doing it anyway). I'm kind of busy doing sigacts locking so
I can finally get all of signal handling out from under Giant, but I
can come back to this after that.
> > > No, in theory increased performance should come from increased
> > > parallelism with no increased overhead. Any increased overhead is a
> > > bug. Linux 2.4 runs a thread safe kernel with less overhead than we
> > > have in 4.x. Its possible.
> >
> > How are we going to achieve increased paralellism w/o increased overhead?
> > Discounting redesigning algo's which would have been a win in the
> > non-parallel kernel as well. A mutex is far more expensive than an spl.
> > You have to protect against more things. Of course overhead is going to
> > go up.
>
> I have no idea really. All I know is that linux 2.4 is more parallel
> than 5.x is today, yet it outperforms 4.x for things like system call
> latency and networking packets/sec through a single interface.
> Therefore it must be possible.
Well, current does suck, but I am saying that a fully tuned 5.x is going
to have more overhead than 4.x because it has more issues to deal with.
--
John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!" - http://www.FreeBSD.org/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.20030429175301.jhb>