> On 25 Apr 2016, at 02:13, Visa Hankala <[email protected]> wrote:
>
> This adds MP-safe TX for cnmac(4). OK?
nearly. see inline.
>
> 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 24 Apr 2016 15:35:04 -0000
> @@ -285,6 +285,7 @@ octeon_eth_attach(struct device *parent,
> octeon_eth_gsc[sc->sc_port] = sc;
>
> ml_init(&sc->sc_sendq);
> + mtx_init(&sc->sc_sendq_mtx, IPL_NET);
this is unnecessary because the ifq machinery makes sure that the call to the
drivers start routine is serialised. the driver doesn't have to do it again.
> sc->sc_soft_req_thresh = 15/* XXX */;
> sc->sc_ext_callback_cnt = 0;
>
> @@ -317,6 +318,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 +744,7 @@ octeon_eth_ioctl(struct ifnet *ifp, u_lo
> error = 0;
> }
>
> - octeon_eth_start(ifp);
> + if_start(ifp);
>
> splx(s);
> return (error);
> @@ -959,18 +961,19 @@ 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;
> + }
> +
> + mtx_enter(&sc->sc_sendq_mtx);
> +
> /*
> * 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,13 +983,16 @@ octeon_eth_start(struct ifnet *ifp)
> * and bail out.
> */
> if (octeon_eth_send_queue_is_full(sc)) {
> + mtx_leave(&sc->sc_sendq_mtx);
not directly related to these changes, but the driver should ifq_set_oactive()
in this situation.
> return;
> }
> /* XXX */
>
> IFQ_DEQUEUE(&ifp->if_snd, m);
> - if (m == NULL)
> + if (m == NULL) {
> + mtx_leave(&sc->sc_sendq_mtx);
> return;
> + }
>
> OCTEON_ETH_TAP(ifp, m, BPF_DIRECTION_OUT);
>
> @@ -1008,8 +1014,9 @@ octeon_eth_start(struct ifnet *ifp)
> octeon_eth_send_queue_flush_prefetch(sc);
> }
>
> -last:
> octeon_eth_send_queue_flush_fetch(sc);
> +
> + mtx_leave(&sc->sc_sendq_mtx);
> }
>
> void
> @@ -1025,7 +1032,7 @@ octeon_eth_watchdog(struct ifnet *ifp)
> ifq_clr_oactive(&ifp->if_snd);
> ifp->if_timer = 0;
>
> - octeon_eth_start(ifp);
> + if_start(ifp);
i think you mean ifq_restart here.
> }
>
> int
> @@ -1066,6 +1073,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 +1083,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;
> }
>
> @@ -1372,9 +1380,8 @@ octeon_eth_tick_free(void *arg)
> {
> struct octeon_eth_softc *sc = arg;
> int timo;
> - int s;
>
> - s = splnet();
> + mtx_enter(&sc->sc_sendq_mtx);
> /* XXX */
> if (ml_len(&sc->sc_sendq) > 0) {
> octeon_eth_send_queue_flush_prefetch(sc);
> @@ -1389,7 +1396,7 @@ octeon_eth_tick_free(void *arg)
> timo = 10;
> timeout_add_msec(&sc->sc_tick_free_ch, 1000 * timo / hz);
> /* XXX */
> - splx(s);
> + mtx_leave(&sc->sc_sendq_mtx);
ah, i see why you want the mutex now. you could serialise that work with the
start routine via ifq_serialize().
cnmac doesnt interrupt for completions?
> }
>
> /*
> 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 24 Apr 2016 15:35:04 -0000
> @@ -80,6 +80,7 @@ struct octeon_eth_softc {
> int64_t sc_hard_done_cnt;
> int sc_prefetch;
> struct mbuf_list sc_sendq;
> + struct mutex sc_sendq_mtx;
> uint64_t sc_ext_callback_cnt;
>
> uint32_t sc_port;
>
