Re: mpsafe vmx(4)

[mxb]

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 = >cmd_ring;
>   struct vmxnet3_comp_ring *comp_ring = >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_rxq[0]);
>   vmxnet3_txintr(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 = >comp_ring;
>   struct vmxnet3_txcompdesc *txcd;
>   struct ifnet *ifp = >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 = _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(>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(sc->sc_ih);
> 

mpsafe vmx(4)

[David Gwynne]

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.c24 Jun 2015 09:40:54 -  1.30
+++ if_vmx.c14 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 = >cmd_ring;
struct vmxnet3_comp_ring *comp_ring = >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_rxq[0]);
vmxnet3_txintr(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 = >comp_ring;
struct vmxnet3_txcompdesc *txcd;
struct ifnet *ifp = >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 = _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(>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(sc->sc_ih);
+
for (queue = 0; queue < NTXQUEUE; queue++)
vmxnet3_txstop(sc, >sc_txq[queue]);
for (queue = 0; queue < NRXQUEUE; queue++)
@@ -944,6 +969,11 @@ vmxnet3_init(struct vmxnet3_softc