cvs commit: src/sys/dev/fxp if_fxp.c if_fxpvar.h

[Andrew Gallatin]

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.

 > > 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.

Drew