Re: KASSERT() in kern_timeout.c

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Sun, Apr 18, 2010 at 02:51:38PM +0100, Mindaugas Rasiukevicius wrote:
> Manuel Bouyer <bouyer%antioche.eu.org@localhost> wrote:
> > > 
> > > AFAIK syncache is not using callout_halt(). It's testing callout_invoking
> > > () though. 
> > > I can't say if droping the softnet lock in syn_cache_put() (this is
> > > where the pool_put() really is) is safe or not.
> > 
> > Can someone familiar with the TCP code have a look at this ?
> > I've got several panic from this bug since I wrote this mail, I've
> > looked at it quickly and the fix doesn't look obvious ...
> 
> There are multiple paths calling syn_cache_put() and not only SYN cache,
> but also upper TCP layers and IP would need to be inspected.  Unlikely to
> be safe and unlikely to be trivial to re-structure to be safe.. there we
> shall start a big MP-safe network stack revamp.
> 
> For a quick fix (apart from horrible hack, like having another softint
> or kthread), it seems that we can use SCF_DEAD with syn_cache_timer()
> invoked via our callout, which would destroy... itself.  :)

You mean, something like the attached diff ?

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: tcp_input.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/tcp_input.c,v
retrieving revision 1.291.4.2
diff -u -p -u -r1.291.4.2 tcp_input.c
--- tcp_input.c 26 Sep 2009 18:34:29 -0000      1.291.4.2
+++ tcp_input.c 19 Apr 2010 10:54:27 -0000
@@ -3343,12 +3343,7 @@ syn_cache_put(struct syn_cache *sc)
        if (sc->sc_ipopts)
                (void) m_free(sc->sc_ipopts);
        rtcache_free(&sc->sc_route);
-       if (callout_invoking(&sc->sc_timer))
-               sc->sc_flags |= SCF_DEAD;
-       else {
-               callout_destroy(&sc->sc_timer);
-               pool_put(&syn_cache_pool, sc);
-       }
+       sc->sc_flags |= SCF_DEAD;
 }
 
 void