On Tue, Apr 26, 2016 at 05:29:43PM +1000, David Gwynne wrote:
> 
> > On 25 Apr 2016, at 02:13, Visa Hankala <[email protected]> wrote:
> > 
> > This adds MP-safe TX for cnmac(4). OK?
> 
> nearly. see inline.

Here is a new try. ifq_serialize() is just what the code needs. Thanks!
To simplify things a bit, octeon_eth_tick_free() now uses a constant
timeout.

Index: arch/octeon/dev/if_cnmac.c
===================================================================
RCS file: src/sys/arch/octeon/dev/if_cnmac.c,v
retrieving revision 1.38
diff -u -p -r1.38 if_cnmac.c
--- arch/octeon/dev/if_cnmac.c  13 Apr 2016 11:34:00 -0000      1.38
+++ arch/octeon/dev/if_cnmac.c  26 Apr 2016 09:41:49 -0000
@@ -173,6 +173,7 @@ int octeon_eth_reset(struct octeon_eth_s
 int    octeon_eth_configure(struct octeon_eth_softc *);
 int    octeon_eth_configure_common(struct octeon_eth_softc *);
 
+void   octeon_eth_free_task(void *);
 void   octeon_eth_tick_free(void *arg);
 void   octeon_eth_tick_misc(void *);
 
@@ -290,6 +291,7 @@ octeon_eth_attach(struct device *parent,
 
        cn30xxgmx_stats_init(sc->sc_gmx_port);
 
+       task_set(&sc->sc_free_task, octeon_eth_free_task, sc);
        timeout_set(&sc->sc_tick_misc_ch, octeon_eth_tick_misc, sc);
        timeout_set(&sc->sc_tick_free_ch, octeon_eth_tick_free, sc);
 
@@ -317,6 +319,7 @@ octeon_eth_attach(struct device *parent,
        strncpy(ifp->if_xname, sc->sc_dev.dv_xname, sizeof(ifp->if_xname));
        ifp->if_softc = sc;
        ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
+       ifp->if_xflags = IFXF_MPSAFE;
        ifp->if_ioctl = octeon_eth_ioctl;
        ifp->if_start = octeon_eth_start;
        ifp->if_watchdog = octeon_eth_watchdog;
@@ -742,7 +745,7 @@ octeon_eth_ioctl(struct ifnet *ifp, u_lo
                error = 0;
        }
 
-       octeon_eth_start(ifp);
+       if_start(ifp);
 
        splx(s);
        return (error);
@@ -959,18 +962,17 @@ octeon_eth_start(struct ifnet *ifp)
        struct octeon_eth_softc *sc = ifp->if_softc;
        struct mbuf *m;
 
+       if (__predict_false(!cn30xxgmx_link_status(sc->sc_gmx_port))) {
+               IFQ_PURGE(&ifp->if_snd);
+               return;
+       }
+
        /*
         * performance tuning
         * presend iobdma request 
         */
        octeon_eth_send_queue_flush_prefetch(sc);
 
-       if (!(ifp->if_flags & IFF_RUNNING) || ifq_is_oactive(&ifp->if_snd))
-               goto last;
-
-       if (__predict_false(!cn30xxgmx_link_status(sc->sc_gmx_port)))
-               goto last;
-
        for (;;) {
                octeon_eth_send_queue_flush_fetch(sc); /* XXX */
 
@@ -980,6 +982,7 @@ octeon_eth_start(struct ifnet *ifp)
                 * and bail out.
                 */
                if (octeon_eth_send_queue_is_full(sc)) {
+                       ifq_set_oactive(&ifp->if_snd);
                        return;
                }
                /* XXX */
@@ -1008,7 +1011,6 @@ octeon_eth_start(struct ifnet *ifp)
                octeon_eth_send_queue_flush_prefetch(sc);
        }
 
-last:
        octeon_eth_send_queue_flush_fetch(sc);
 }
 
@@ -1019,13 +1021,14 @@ octeon_eth_watchdog(struct ifnet *ifp)
 
        printf("%s: device timeout\n", sc->sc_dev.dv_xname);
 
+       octeon_eth_stop(ifp, 0);
+
        octeon_eth_configure(sc);
 
        SET(ifp->if_flags, IFF_RUNNING);
-       ifq_clr_oactive(&ifp->if_snd);
        ifp->if_timer = 0;
 
-       octeon_eth_start(ifp);
+       ifq_restart(&ifp->if_snd);
 }
 
 int
@@ -1066,6 +1069,8 @@ octeon_eth_stop(struct ifnet *ifp, int d
 {
        struct octeon_eth_softc *sc = ifp->if_softc;
 
+       CLR(ifp->if_flags, IFF_RUNNING);
+
        timeout_del(&sc->sc_tick_misc_ch);
        timeout_del(&sc->sc_tick_free_ch);
        timeout_del(&sc->sc_resume_ch);
@@ -1074,13 +1079,12 @@ octeon_eth_stop(struct ifnet *ifp, int d
 
        cn30xxgmx_port_enable(sc->sc_gmx_port, 0);
 
-       /* Mark the interface as down and cancel the watchdog timer. */
-       CLR(ifp->if_flags, IFF_RUNNING);
+       intr_barrier(octeon_eth_pow_recv_ih);
+       ifq_barrier(&ifp->if_snd);
+
        ifq_clr_oactive(&ifp->if_snd);
        ifp->if_timer = 0;
 
-       intr_barrier(octeon_eth_pow_recv_ih);
-
        return 0;
 }
 
@@ -1361,6 +1365,26 @@ octeon_eth_recv_intr(void *data, uint64_
 
 /* ---- tick */
 
+void
+octeon_eth_free_task(void *arg)
+{
+       struct octeon_eth_softc *sc = arg;
+       struct ifnet *ifp = &sc->sc_arpcom.ac_if;
+
+       if (ml_len(&sc->sc_sendq) > 0) {
+               octeon_eth_send_queue_flush_prefetch(sc);
+               octeon_eth_send_queue_flush_fetch(sc);
+               octeon_eth_send_queue_flush(sc);
+       }
+
+       if (ifq_is_oactive(&ifp->if_snd)) {
+               ifq_clr_oactive(&ifp->if_snd);
+               octeon_eth_start(ifp);
+       }
+
+       timeout_add_sec(&sc->sc_tick_free_ch, 1);
+}
+
 /*
  * octeon_eth_tick_free
  *
@@ -1371,25 +1395,9 @@ void
 octeon_eth_tick_free(void *arg)
 {
        struct octeon_eth_softc *sc = arg;
-       int timo;
-       int s;
-
-       s = splnet();
-       /* XXX */
-       if (ml_len(&sc->sc_sendq) > 0) {
-               octeon_eth_send_queue_flush_prefetch(sc);
-               octeon_eth_send_queue_flush_fetch(sc);
-               octeon_eth_send_queue_flush(sc);
-       }
-       /* XXX */
+       struct ifnet *ifp = &sc->sc_arpcom.ac_if;
 
-       /* XXX ??? */
-       timo = hz - (100 * sc->sc_ext_callback_cnt);
-       if (timo < 10)
-                timo = 10;
-       timeout_add_msec(&sc->sc_tick_free_ch, 1000 * timo / hz);
-       /* XXX */
-       splx(s);
+       ifq_serialize(&ifp->if_snd, &sc->sc_free_task);
 }
 
 /*
Index: arch/octeon/dev/if_cnmacvar.h
===================================================================
RCS file: src/sys/arch/octeon/dev/if_cnmacvar.h,v
retrieving revision 1.7
diff -u -p -r1.7 if_cnmacvar.h
--- arch/octeon/dev/if_cnmacvar.h       8 Oct 2015 14:24:32 -0000       1.7
+++ arch/octeon/dev/if_cnmacvar.h       26 Apr 2016 09:41:49 -0000
@@ -76,6 +76,8 @@ struct octeon_eth_softc {
        struct timeout          sc_tick_free_ch;
        struct timeout          sc_resume_ch;
 
+       struct task             sc_free_task;
+
        int64_t                 sc_soft_req_thresh;
        int64_t                 sc_hard_done_cnt;
        int                     sc_prefetch;

Reply via email to