Module Name: src
Committed By: jdc
Date: Mon Jun 22 16:05:20 UTC 2020
Modified Files:
src/sys/dev/scsipi: if_se.c
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.105 -r1.106 src/sys/dev/scsipi/if_se.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/scsipi/if_se.c
diff -u src/sys/dev/scsipi/if_se.c:1.105 src/sys/dev/scsipi/if_se.c:1.106
--- src/sys/dev/scsipi/if_se.c:1.105 Fri Jun 19 10:30:27 2020
+++ src/sys/dev/scsipi/if_se.c Mon Jun 22 16:05:20 2020
@@ -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.d...@dsto.defence.gov.au>
@@ -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 @@ __KERNEL_RCSID(0, "$NetBSD: if_se.c,v 1.
#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 se_softc {
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 int sematch(device_t, cfdata_t, v
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 @@ static void sewatchdog(struct ifnet *);
#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 @@ static inline int se_scsipi_cmd(struct s
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 cdevsw se_cdevsw = {
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 @@ seattach(device_t parent, device_t self,
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 @@ seattach(device_t parent, device_t self,
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 @@ seattach(device_t parent, device_t self,
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 @@ seattach(device_t parent, device_t self,
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 @@ se_scsipi_cmd(struct scsipi_periph *peri
{
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
+ * Start routine for calling from network sub system
*/
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)
+se_ifstart(struct ifnet *ifp)
{
- struct ifnet *ifp = v;
struct se_softc *sc = ifp->if_softc;
+ int i = 100;
- mutex_enter(chan_mtx(sc->sc_periph->periph_channel));
- if (sc->sc_enabled) {
- ifp->if_flags &= ~IFF_OACTIVE;
- se_ifstart(ifp);
+ mutex_enter(&sc->sc_iflock);
+ while (i && sc->sc_send_work_pending == true) {
+ i--;
+ delay(10);
}
- mutex_exit(chan_mtx(sc->sc_periph->periph_channel));
+ 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);
}
/*
- * Start transmission on the interface.
- * Must be called with the scsipi channel lock held
+ * Invoke the transmit workqueue and transmission on the interface.
*/
static void
-se_ifstart(struct ifnet *ifp)
+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;
-
- 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);
+ while (1) {
+ IFQ_DEQUEUE(&ifp->if_snd, m0);
+ if (m0 == 0)
+ break;
- /* 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;
-
- /* Chain; copy into linear buffer we 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;
- m = m0 = m_free(m);
- }
- if (len < SEMINSIZE) {
+ /* 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;
+
+ /* 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;
+ m = m0 = m_free(m);
+ }
+ if (len < SEMINSIZE) {
#ifdef SEDEBUG
- if (sc->sc_debug)
- printf("se: packet size %d (%zu) < %d\n", len,
- cp - (u_char *)sc->sc_tbuf, SEMINSIZE);
+ if (sc->sc_debug)
+ printf("se: packet size %d (%zu) < %d\n", len,
+ cp - (u_char *)sc->sc_tbuf, SEMINSIZE);
#endif
- memset(cp, 0, SEMINSIZE - len);
- len = SEMINSIZE;
- }
-
- /* Fill out SCSI command. */
- PROTOCMD(ctron_ether_send, send_cmd);
- _lto2b(len, send_cmd.length);
+ memset(cp, 0, SEMINSIZE - len);
+ len = SEMINSIZE;
+ }
- /* Send command to device. */
- error = se_scsipi_cmd(sc->sc_periph,
- (void *)&send_cmd, sizeof(send_cmd),
- sc->sc_tbuf, len, SERETRIES,
- SETIMEOUT, NULL, XS_CTL_NOSLEEP | XS_CTL_DATA_OUT);
- if (error) {
- aprint_error_dev(sc->sc_dev, "not queued, error %d\n", error);
- if_statinc(ifp, if_oerrors);
- ifp->if_flags &= ~IFF_OACTIVE;
- } else
- if_statinc(ifp, if_opackets);
+ /* Fill out SCSI command. */
+ PROTOCMD(ctron_ether_send, send_cmd);
+ _lto2b(len, send_cmd.length);
+
+ /* Send command to device. */
+ error = se_scsipi_cmd(sc->sc_periph,
+ (void *)&send_cmd, sizeof(send_cmd),
+ sc->sc_tbuf, len, SERETRIES,
+ SETIMEOUT, NULL, XS_CTL_NOSLEEP | XS_CTL_DATA_OUT);
+ if (error) {
+ aprint_error_dev(sc->sc_dev,
+ "not queued, error %d\n", error);
+ if_statinc(ifp, if_oerrors);
+ ifp->if_flags &= ~IFF_OACTIVE;
+ } else
+ if_statinc(ifp, if_opackets);
+ }
}
@@ -500,16 +528,7 @@ sedone(struct scsipi_xfer *xs, int error
struct ifnet *ifp = &sc->sc_ethercom.ec_if;
if (IS_SEND(cmd)) {
- if (xs->error == XS_BUSY) {
- printf("se: busy, retry txmit\n");
- callout_reset(&sc->sc_ifstart_ch, hz,
- se_delayed_ifstart, ifp);
- } else {
- ifp->if_flags &= ~IFF_OACTIVE;
- /* the generic scsipi_done will call
- * sestart (through scsipi_free_xs).
- */
- }
+ ifp->if_flags &= ~IFF_OACTIVE;
} else if (IS_RECV(cmd)) {
/* RECV complete */
/* pass data up. reschedule a recv */
@@ -517,7 +536,7 @@ sedone(struct scsipi_xfer *xs, int error
if (error) {
/* Reschedule after a delay */
callout_reset(&sc->sc_recv_ch, se_poll,
- se_recv, (void *)sc);
+ se_recv_callout, (void *)sc);
} else {
int n, ntimeo;
n = se_read(sc, xs->data, xs->datalen - xs->resid);
@@ -539,22 +558,61 @@ sedone(struct scsipi_xfer *xs, int error
}
sc->sc_last_timeout = ntimeo;
callout_reset(&sc->sc_recv_ch, ntimeo,
- se_recv, (void *)sc);
+ se_recv_callout, (void *)sc);
}
}
}
+/*
+ * Setup a receive command by queuing the work.
+ * Usually called from a callout, but also from se_init().
+ */
static void
-se_recv(void *v)
+se_recv_callout(void *v)
{
/* do a recv command */
struct se_softc *sc = (struct se_softc *) v;
- struct scsi_ctron_ether_recv recv_cmd;
- int error;
if (sc->sc_enabled == 0)
return;
+ mutex_enter(&sc->sc_iflock);
+ if (sc->sc_recv_work_pending == true) {
+ callout_reset(&sc->sc_recv_ch, se_poll,
+ se_recv_callout, (void *)sc);
+ return;
+ }
+
+ sc->sc_recv_work_pending = true;
+ workqueue_enqueue(sc->sc_recv_wq, &sc->sc_recv_work, NULL);
+ mutex_exit(&sc->sc_iflock);
+}
+
+/*
+ * Invoke the receive workqueue
+ */
+static void
+se_recv_worker(struct work *wk, void *cookie)
+{
+ struct se_softc *sc = (struct se_softc *) cookie;
+
+ mutex_enter(&sc->sc_iflock);
+ sc->sc_recv_work_pending = false;
+ mutex_exit(&sc->sc_iflock);
+ se_recv(sc);
+
+}
+
+/*
+ * Do the actual work of receiving data.
+ */
+static void
+se_recv(struct se_softc *sc)
+{
+ struct scsi_ctron_ether_recv recv_cmd;
+ int error;
+
+ /* do a recv command */
PROTOCMD(ctron_ether_recv, recv_cmd);
error = se_scsipi_cmd(sc->sc_periph,
@@ -562,7 +620,8 @@ se_recv(void *v)
sc->sc_rbuf, RBUF_LEN, SERETRIES, SETIMEOUT, NULL,
XS_CTL_NOSLEEP | XS_CTL_DATA_IN);
if (error)
- callout_reset(&sc->sc_recv_ch, se_poll, se_recv, (void *)sc);
+ callout_reset(&sc->sc_recv_ch, se_poll,
+ se_recv_callout, (void *)sc);
}
/*
@@ -692,22 +751,19 @@ sewatchdog(struct ifnet *ifp)
se_reset(sc);
}
-static int
+static void
se_reset(struct se_softc *sc)
{
- int error;
-
#if 0
/* Maybe we don't *really* want to reset the entire bus
* because the ctron isn't working. We would like to send a
* "BUS DEVICE RESET" message, but don't think the ctron
* understands it.
*/
- error = se_scsipi_cmd(sc->sc_periph, 0, 0, 0, 0, SERETRIES, 2000, NULL,
+ se_scsipi_cmd(sc->sc_periph, 0, 0, 0, 0, SERETRIES, 2000, NULL,
XS_CTL_RESET);
#endif
- error = se_init(sc);
- return (error);
+ se_init(sc);
}
static int
@@ -826,11 +882,14 @@ se_init(struct se_softc *sc)
if ((ifp->if_flags & (IFF_RUNNING | IFF_UP)) == IFF_UP) {
ifp->if_flags |= IFF_RUNNING;
- se_recv(sc);
+ mutex_enter(&sc->sc_iflock);
+ sc->sc_recv_work_pending = true;
+ workqueue_enqueue(sc->sc_recv_wq, &sc->sc_recv_work, NULL);
+ mutex_exit(&sc->sc_iflock);
ifp->if_flags &= ~IFF_OACTIVE;
- mutex_enter(chan_mtx(sc->sc_periph->periph_channel));
- se_ifstart(ifp);
- mutex_exit(chan_mtx(sc->sc_periph->periph_channel));
+ mutex_enter(&sc->sc_iflock);
+ workqueue_enqueue(sc->sc_send_wq, &sc->sc_send_work, NULL);
+ mutex_exit(&sc->sc_iflock);
}
return (error);
}
@@ -846,13 +905,10 @@ se_set_multi(struct se_softc *sc, uint8_
ether_sprintf(addr));
PROTOCMD(ctron_ether_set_multi, set_multi_cmd);
- _lto2b(sizeof(addr), set_multi_cmd.length);
- /* XXX sizeof(addr) is the size of the pointer. Surely it
- * is too small? --dyoung
- */
+ _lto2b(ETHER_ADDR_LEN, set_multi_cmd.length);
error = se_scsipi_cmd(sc->sc_periph,
(void *)&set_multi_cmd, sizeof(set_multi_cmd),
- addr, sizeof(addr), SERETRIES, SETIMEOUT, NULL, XS_CTL_DATA_OUT);
+ addr, ETHER_ADDR_LEN, SERETRIES, SETIMEOUT, NULL, XS_CTL_DATA_OUT);
return (error);
}
@@ -867,13 +923,10 @@ se_remove_multi(struct se_softc *sc, uin
ether_sprintf(addr));
PROTOCMD(ctron_ether_remove_multi, remove_multi_cmd);
- _lto2b(sizeof(addr), remove_multi_cmd.length);
- /* XXX sizeof(addr) is the size of the pointer. Surely it
- * is too small? --dyoung
- */
+ _lto2b(ETHER_ADDR_LEN, remove_multi_cmd.length);
error = se_scsipi_cmd(sc->sc_periph,
(void *)&remove_multi_cmd, sizeof(remove_multi_cmd),
- addr, sizeof(addr), SERETRIES, SETIMEOUT, NULL, XS_CTL_DATA_OUT);
+ addr, ETHER_ADDR_LEN, SERETRIES, SETIMEOUT, NULL, XS_CTL_DATA_OUT);
return (error);
}
@@ -928,6 +981,12 @@ se_stop(struct se_softc *sc)
/* Don't schedule any reads */
callout_stop(&sc->sc_recv_ch);
+ /* Wait for the workqueues to finish */
+ mutex_enter(&sc->sc_iflock);
+ workqueue_wait(sc->sc_recv_wq, &sc->sc_recv_work);
+ workqueue_wait(sc->sc_send_wq, &sc->sc_send_work);
+ mutex_exit(&sc->sc_iflock);
+
/* Abort any scsi cmds in progress */
mutex_enter(chan_mtx(sc->sc_periph->periph_channel));
scsipi_kill_pending(sc->sc_periph);
