Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/scsipi Use workqueues so that we don't call into the...



details:   https://anonhg.NetBSD.org/src/rev/124774138a5b
branches:  trunk
changeset: 973161:124774138a5b
user:      jdc <jdc%NetBSD.org@localhost>
date:      Mon Jun 22 16:05:20 2020 +0000

description:
Use workqueues so that we don't call into the scsipi subsystem via
a softint from the network stack.
Don't recurse through scsipi_command() when we have multiple packets
in the send queue - use a loop instead.  This means that we no longer
need sestart(), as we can now handle everything in sedone().
Fix a couple of XXX's.
Rework the locking logic slightly from the previous revision.
Now this works with DIAGNOSTIC+LOCKDEBUG.

diffstat:

 sys/dev/scsipi/if_se.c |  317 +++++++++++++++++++++++++++++-------------------
 1 files changed, 188 insertions(+), 129 deletions(-)

diffs (truncated from 558 to 300 lines):

diff -r 4a567a48254b -r 124774138a5b sys/dev/scsipi/if_se.c
--- a/sys/dev/scsipi/if_se.c    Mon Jun 22 15:07:11 2020 +0000
+++ b/sys/dev/scsipi/if_se.c    Mon Jun 22 16:05:20 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_se.c,v 1.105 2020/06/19 10:30:27 jdc Exp $  */
+/*     $NetBSD: if_se.c,v 1.106 2020/06/22 16:05:20 jdc Exp $  */
 
 /*
  * Copyright (c) 1997 Ian W. Dall <ian.dall%dsto.defence.gov.au@localhost>
@@ -49,7 +49,7 @@
  * This driver is also a bit unusual. It must look like a network
  * interface and it must also appear to be a scsi device to the scsi
  * system. Hence there are cases where there are two entry points. eg
- * sestart is to be called from the scsi subsytem and se_ifstart from
+ * sedone is to be called from the scsi subsytem and se_ifstart from
  * the network interface subsystem.  In addition, to facilitate scsi
  * commands issued by userland programs, there are open, close and
  * ioctl entry points. This allows a user program to, for example,
@@ -59,10 +59,11 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_se.c,v 1.105 2020/06/19 10:30:27 jdc Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_se.c,v 1.106 2020/06/22 16:05:20 jdc Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
+#include "opt_net_mpsafe.h"
 #include "opt_atalk.h"
 #endif
 
@@ -85,6 +86,7 @@
 #include <sys/conf.h>
 #include <sys/mutex.h>
 #include <sys/pcq.h>
+#include <sys/workqueue.h>
 
 #include <dev/scsipi/scsipi_all.h>
 #include <dev/scsipi/scsi_ctron_ether.h>
@@ -177,10 +179,12 @@
        struct ethercom sc_ethercom;    /* Ethernet common part */
        struct scsipi_periph *sc_periph;/* contains our targ, lun, etc. */
 
-       struct callout sc_ifstart_ch;
        struct callout sc_recv_ch;
        struct kmutex sc_iflock;
        struct if_percpuq *sc_ipq;
+       struct workqueue *sc_recv_wq, *sc_send_wq;
+       struct work sc_recv_work, sc_send_work;
+       int sc_recv_work_pending, sc_send_work_pending;
 
        char *sc_tbuf;
        char *sc_rbuf;
@@ -200,7 +204,6 @@
 static void    seattach(device_t, device_t, void *);
 
 static void    se_ifstart(struct ifnet *);
-static void    sestart(struct scsipi_periph *);
 
 static void    sedone(struct scsipi_xfer *, int);
 static int     se_ioctl(struct ifnet *, u_long, void *);
@@ -209,10 +212,12 @@
 #if 0
 static inline uint16_t ether_cmp(void *, void *);
 #endif
-static void    se_recv(void *);
+static void    se_recv_callout(void *);
+static void    se_recv_worker(struct work *wk, void *cookie);
+static void    se_recv(struct se_softc *);
 static struct mbuf *se_get(struct se_softc *, char *, int);
 static int     se_read(struct se_softc *, char *, int);
-static int     se_reset(struct se_softc *);
+static void    se_reset(struct se_softc *);
 static int     se_add_proto(struct se_softc *, int);
 static int     se_get_addr(struct se_softc *, uint8_t *);
 static int     se_set_media(struct se_softc *, int);
@@ -228,7 +233,7 @@
                        int cmdlen, u_char *data_addr, int datalen,
                        int retries, int timeout, struct buf *bp,
                        int flags);
-static void    se_delayed_ifstart(void *);
+static void    se_send_worker(struct work *wk, void *cookie);
 static int     se_set_mode(struct se_softc *, int, int);
 
 int    se_enable(struct se_softc *);
@@ -260,7 +265,7 @@
 
 const struct scsipi_periphsw se_switch = {
        NULL,                   /* Use default error handler */
-       sestart,                /* have a queue, served by this */
+       NULL,                   /* have no queue */
        NULL,                   /* have no async handler */
        sedone,                 /* deal with stats at interrupt time */
 };
@@ -317,6 +322,7 @@
        struct scsipi_periph *periph = sa->sa_periph;
        struct ifnet *ifp = &sc->sc_ethercom.ec_if;
        uint8_t myaddr[ETHER_ADDR_LEN];
+       char wqname[MAXCOMLEN];
        int rv;
 
        sc->sc_dev = self;
@@ -324,7 +330,6 @@
        printf("\n");
        SC_DEBUG(periph, SCSIPI_DB2, ("seattach: "));
 
-       callout_init(&sc->sc_ifstart_ch, CALLOUT_MPSAFE);
        callout_init(&sc->sc_recv_ch, CALLOUT_MPSAFE);
        mutex_init(&sc->sc_iflock, MUTEX_DEFAULT, IPL_SOFTNET);
 
@@ -335,21 +340,17 @@
        periph->periph_dev = sc->sc_dev;
        periph->periph_switch = &se_switch;
 
-       /* XXX increase openings? */
-
        se_poll = (SE_POLL * hz) / 1000;
        se_poll = se_poll? se_poll: 1;
        se_poll0 = (SE_POLL0 * hz) / 1000;
        se_poll0 = se_poll0? se_poll0: 1;
 
        /*
-        * Initialize and attach a buffer
+        * Initialize and attach send and receive buffers
         */
        sc->sc_tbuf = malloc(ETHERMTU + sizeof(struct ether_header),
                             M_DEVBUF, M_WAITOK);
-       sc->sc_rbuf = malloc(RBUF_LEN, M_DEVBUF, M_WAITOK);/* A Guess */
-
-       se_get_addr(sc, myaddr);
+       sc->sc_rbuf = malloc(RBUF_LEN, M_DEVBUF, M_WAITOK);
 
        /* Initialize ifnet structure. */
        strlcpy(ifp->if_xname, device_xname(sc->sc_dev), sizeof(ifp->if_xname));
@@ -358,17 +359,44 @@
        ifp->if_ioctl = se_ioctl;
        ifp->if_watchdog = sewatchdog;
        ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
+       ifp->if_extflags = IFEF_MPSAFE;
        IFQ_SET_READY(&ifp->if_snd);
 
+       se_get_addr(sc, myaddr);
+
        /* Attach the interface. */
        rv = if_initialize(ifp);
        if (rv != 0) {
                free(sc->sc_tbuf, M_DEVBUF);
-               callout_destroy(&sc->sc_ifstart_ch);
+               callout_destroy(&sc->sc_recv_ch);
+               mutex_destroy(&sc->sc_iflock);
+               return; /* Error */
+       }
+       snprintf(wqname, sizeof(wqname), "%sRx", device_xname(sc->sc_dev));
+       rv = workqueue_create(&sc->sc_recv_wq, wqname, se_recv_worker, sc,
+           PRI_SOFTNET, IPL_NET, WQ_MPSAFE);
+       if (rv != 0) {
+               aprint_error_dev(sc->sc_dev,
+                   "unable to create recv workqueue\n");
+               free(sc->sc_tbuf, M_DEVBUF);
                callout_destroy(&sc->sc_recv_ch);
                mutex_destroy(&sc->sc_iflock);
                return; /* Error */
        }
+       sc->sc_recv_work_pending = false;
+       snprintf(wqname, sizeof(wqname), "%sTx", device_xname(sc->sc_dev));
+       rv = workqueue_create(&sc->sc_send_wq, wqname, se_send_worker, ifp,
+           PRI_SOFTNET, IPL_NET, WQ_MPSAFE);
+       if (rv != 0) {
+               aprint_error_dev(sc->sc_dev,
+                   "unable to create send workqueue\n");
+               free(sc->sc_tbuf, M_DEVBUF);
+               callout_destroy(&sc->sc_recv_ch);
+               mutex_destroy(&sc->sc_iflock);
+               workqueue_destroy(sc->sc_send_wq);
+               return; /* Error */
+       }
+       sc->sc_send_work_pending = false;
        sc->sc_ipq = if_percpuq_create(&sc->sc_ethercom.ec_if);
        ether_ifattach(ifp, myaddr);
        if_register(ifp);
@@ -384,108 +412,108 @@
 {
        int error;
 
-       KASSERT(!mutex_owned(chan_mtx(periph->periph_channel)));
-
        error = scsipi_command(periph, cmd, cmdlen, data_addr,
            datalen, retries, timeout, bp, flags);
        return (error);
 }
 
 /*
- * Start routine for calling from scsi sub system
- * Called with the channel lock held
- */
-static void
-sestart(struct scsipi_periph *periph)
-{
-       struct se_softc *sc = device_private(periph->periph_dev);
-       struct ifnet *ifp = &sc->sc_ethercom.ec_if;
-
-       se_ifstart(ifp);
-}
-
-static void
-se_delayed_ifstart(void *v)
-{
-       struct ifnet *ifp = v;
-       struct se_softc *sc = ifp->if_softc;
-
-       mutex_enter(chan_mtx(sc->sc_periph->periph_channel));
-       if (sc->sc_enabled) {
-               ifp->if_flags &= ~IFF_OACTIVE;
-               se_ifstart(ifp);
-       }
-       mutex_exit(chan_mtx(sc->sc_periph->periph_channel));
-}
-
-/*
- * Start transmission on the interface.
- * Must be called with the scsipi channel lock held
+ * Start routine for calling from network sub system
  */
 static void
 se_ifstart(struct ifnet *ifp)
 {
        struct se_softc *sc = ifp->if_softc;
+       int i = 100;
+
+       mutex_enter(&sc->sc_iflock);
+       while (i && sc->sc_send_work_pending == true) {
+               i--;
+               delay(10);
+       }
+       if (i) {
+               sc->sc_send_work_pending = true;
+               workqueue_enqueue(sc->sc_send_wq, &sc->sc_send_work, NULL);
+       } else
+               if_statinc(ifp, if_oerrors);
+       mutex_exit(&sc->sc_iflock);
+}
+
+/*
+ * Invoke the transmit workqueue and transmission on the interface.
+ */
+static void
+se_send_worker(struct work *wk, void *cookie)
+{
+       struct ifnet *ifp = cookie;
+       struct se_softc *sc = ifp->if_softc;
        struct scsi_ctron_ether_generic send_cmd;
        struct mbuf *m, *m0;
        int len, error;
        u_char *cp;
 
+       mutex_enter(&sc->sc_iflock);
+       sc->sc_send_work_pending = false;
+       mutex_exit(&sc->sc_iflock);
+
+       KASSERT(if_is_mpsafe(ifp));
+
        /* Don't transmit if interface is busy or not running */
        if ((ifp->if_flags & (IFF_RUNNING | IFF_OACTIVE)) != IFF_RUNNING)
                return;
 
-       IFQ_DEQUEUE(&ifp->if_snd, m0);
-       if (m0 == 0)
-               return;
+       while (1) {
+               IFQ_DEQUEUE(&ifp->if_snd, m0);
+               if (m0 == 0)
+                       break;
 
-       KASSERT(mutex_owned(chan_mtx(sc->sc_periph->periph_channel)));
+               /* If BPF is listening on this interface, let it see the
+                * packet before we commit it to the wire.
+                */
+               bpf_mtap(ifp, m0, BPF_D_OUT);
 
-       /* If BPF is listening on this interface, let it see the
-        * packet before we commit it to the wire.
-        */
-       bpf_mtap(ifp, m0, BPF_D_OUT);
+               /* We need to use m->m_pkthdr.len, so require the header */
+               if ((m0->m_flags & M_PKTHDR) == 0)
+                       panic("ctscstart: no header mbuf");
+               len = m0->m_pkthdr.len;
+
+               /* Mark the interface busy. */
+               ifp->if_flags |= IFF_OACTIVE;
 
-       /* We need to use m->m_pkthdr.len, so require the header */
-       if ((m0->m_flags & M_PKTHDR) == 0)
-               panic("ctscstart: no header mbuf");
-       len = m0->m_pkthdr.len;
+               /* Chain; copy into linear buffer allocated at attach time. */
+               cp = sc->sc_tbuf;
+               for (m = m0; m != NULL; ) {
+                       memcpy(cp, mtod(m, u_char *), m->m_len);
+                       cp += m->m_len;



Home | Main Index | Thread Index | Old Index