This one in the tree, so it’s live on my side.
> On 14 sep. 2015, at 13:09, David Gwynne wrote:
>
> this is an attempt to make the interrupt path in vmx mpsafe.
>
> seems to hold up under load here, but more testing would be
> appreciated.
>
> Index: if_vmx.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_vmx.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 if_vmx.c
> --- if_vmx.c 24 Jun 2015 09:40:54 - 1.30
> +++ if_vmx.c 14 Sep 2015 11:08:09 -
> @@ -61,8 +61,9 @@ struct vmxnet3_txring {
> struct mbuf *m[NTXDESC];
> bus_dmamap_t dmap[NTXDESC];
> struct vmxnet3_txdesc *txd;
> - u_int head;
> - u_int next;
> + u_int prod;
> + u_int cons;
> + u_int free;
> u_int8_t gen;
> };
>
> @@ -107,6 +108,7 @@ struct vmxnet3_softc {
> bus_space_handle_t sc_ioh0;
> bus_space_handle_t sc_ioh1;
> bus_dma_tag_t sc_dmat;
> + void *sc_ih;
>
> struct vmxnet3_txqueue sc_txq[NTXQUEUE];
> struct vmxnet3_rxqueue sc_rxq[NRXQUEUE];
> @@ -167,7 +169,8 @@ void vmxnet3_reset(struct vmxnet3_softc
> int vmxnet3_init(struct vmxnet3_softc *);
> int vmxnet3_ioctl(struct ifnet *, u_long, caddr_t);
> void vmxnet3_start(struct ifnet *);
> -int vmxnet3_load_mbuf(struct vmxnet3_softc *, struct mbuf *);
> +int vmxnet3_load_mbuf(struct vmxnet3_softc *, struct vmxnet3_txring *,
> +struct mbuf *);
> void vmxnet3_watchdog(struct ifnet *);
> void vmxnet3_media_status(struct ifnet *, struct ifmediareq *);
> int vmxnet3_media_change(struct ifnet *);
> @@ -239,8 +242,8 @@ vmxnet3_attach(struct device *parent, st
> printf(": failed to map interrupt\n");
> return;
> }
> - pci_intr_establish(pa->pa_pc, ih, IPL_NET, vmxnet3_intr, sc,
> - self->dv_xname);
> + sc->sc_ih = pci_intr_establish(pa->pa_pc, ih, IPL_NET | IPL_MPSAFE,
> + vmxnet3_intr, sc, self->dv_xname);
> intrstr = pci_intr_string(pa->pa_pc, ih);
> if (intrstr)
> printf(": %s", intrstr);
> @@ -466,7 +469,8 @@ vmxnet3_txinit(struct vmxnet3_softc *sc,
> struct vmxnet3_txring *ring = &tq->cmd_ring;
> struct vmxnet3_comp_ring *comp_ring = &tq->comp_ring;
>
> - ring->head = ring->next = 0;
> + ring->cons = ring->prod = 0;
> + ring->free = NTXDESC;
> ring->gen = 1;
> comp_ring->next = 0;
> comp_ring->gen = 1;
> @@ -594,16 +598,19 @@ vmxnet3_intr(void *arg)
>
> if (READ_BAR1(sc, VMXNET3_BAR1_INTR) == 0)
> return 0;
> - if (sc->sc_ds->event)
> +
> + if (sc->sc_ds->event) {
> + KERNEL_LOCK();
> vmxnet3_evintr(sc);
> -#ifdef VMXNET3_STAT
> - vmxstat.intr++;
> -#endif
> + KERNEL_UNLOCK();
> + }
> +
> if (ifp->if_flags & IFF_RUNNING) {
> vmxnet3_rxintr(sc, &sc->sc_rxq[0]);
> vmxnet3_txintr(sc, &sc->sc_txq[0]);
> vmxnet3_enable_intr(sc, 0);
> }
> +
> return 1;
> }
>
> @@ -649,7 +656,12 @@ vmxnet3_txintr(struct vmxnet3_softc *sc,
> struct vmxnet3_comp_ring *comp_ring = &tq->comp_ring;
> struct vmxnet3_txcompdesc *txcd;
> struct ifnet *ifp = &sc->sc_arpcom.ac_if;
> - u_int sop;
> + bus_dmamap_t map;
> + struct mbuf *m;
> + u_int cons;
> + u_int free = 0;
> +
> + cons = ring->cons;
>
> for (;;) {
> txcd = &comp_ring->txcd[comp_ring->next];
> @@ -664,21 +676,32 @@ vmxnet3_txintr(struct vmxnet3_softc *sc,
> comp_ring->gen ^= 1;
> }
>
> - sop = ring->next;
> - if (ring->m[sop] == NULL)
> - panic("%s: NULL ring->m[%u]", __func__, sop);
> - m_freem(ring->m[sop]);
> - ring->m[sop] = NULL;
> - bus_dmamap_unload(sc->sc_dmat, ring->dmap[sop]);
> - ring->next = (letoh32((txcd->txc_word0 >>
> + m = ring->m[cons];
> + ring->m[cons] = NULL;
> +
> + KASSERT(m != NULL);
> +
> + map = ring->dmap[cons];
> + free += map->dm_nsegs;
> + bus_dmamap_unload(sc->sc_dmat, map);
> + m_freem(m);
> +
> + cons = (letoh32((txcd->txc_word0 >>
> VMXNET3_TXC_EOPIDX_S) & VMXNET3_TXC_EOPIDX_M) + 1)
> % NTXDESC;
> -
> - ifp->if_flags &= ~IFF_OACTIVE;
> }
> - if (ring->head == ring->next)
> +
> + ring->cons = cons;
> +
> + if (atomic_add_int_nv(&ring->free, free) == NTXDESC)
> ifp->if_timer = 0;
> - vmxnet3_start(ifp);
> +
> + if (ISSET(ifp->if_flags, IFF_OACTIVE)) {
> + KERNEL_LOCK();
> + CLR(ifp->if_flags, IFF_OACTIVE);
> + vmxnet3_start(ifp);
> + KERNEL_UNLOCK();
> + }
> }
>
> void
> @@ -911,6 +934,8 @@ vmxnet3_stop(struct ifnet *ifp)
>
> WRITE_CMD(sc, VMXNET3_CMD_DISABLE);
>
> + intr_barrier(