Skip site navigation (1)Skip section navigation (2)
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>