Source-Changes-HG archive

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

[src/trunk]: src/sys/dev/scsipi First pass at making this work again.



details:   https://anonhg.NetBSD.org/src/rev/8e14bea2970c
branches:  trunk
changeset: 934787:8e14bea2970c
user:      jdc <jdc%NetBSD.org@localhost>
date:      Fri Jun 19 10:30:27 2020 +0000

description:
First pass at making this work again.
Remove spl and add some locking around network access (needs more work).
Make sure that we consistently use the channel lock for scsipi commands.
Remove the preference for send over receive, as this can lead to deadlocks
- we only advertise 1 opening, but we can try to send before the receive is
complete in this case.
Don't use XS_CTL_ASYNC because we don't provide a buffer.
Tested on UP sparc and compile-tested on atari.
Tested with LOCKDEBUG.
Still fails with DIAGNOSTIC because we can call into the scsipi routines
from a softint.

diffstat:

 sys/dev/scsipi/if_se.c |  108 ++++++++++++++++++++++++++++--------------------
 1 files changed, 62 insertions(+), 46 deletions(-)

diffs (truncated from 362 to 300 lines):

diff -r f87679b04087 -r 8e14bea2970c sys/dev/scsipi/if_se.c
--- a/sys/dev/scsipi/if_se.c    Fri Jun 19 07:43:37 2020 +0000
+++ b/sys/dev/scsipi/if_se.c    Fri Jun 19 10:30:27 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_se.c,v 1.104 2020/01/29 05:59:50 thorpej Exp $      */
+/*     $NetBSD: if_se.c,v 1.105 2020/06/19 10:30:27 jdc Exp $  */
 
 /*
  * Copyright (c) 1997 Ian W. Dall <ian.dall%dsto.defence.gov.au@localhost>
@@ -59,7 +59,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_se.c,v 1.104 2020/01/29 05:59:50 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_se.c,v 1.105 2020/06/19 10:30:27 jdc Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -83,6 +83,8 @@
 #include <sys/disk.h>
 #include <sys/proc.h>
 #include <sys/conf.h>
+#include <sys/mutex.h>
+#include <sys/pcq.h>
 
 #include <dev/scsipi/scsipi_all.h>
 #include <dev/scsipi/scsi_ctron_ether.h>
@@ -142,7 +144,9 @@
 #define SE_POLL0 10            /* default in milliseconds */
 int se_poll = 0;               /* Delay in ticks set at attach time */
 int se_poll0 = 0;
+#ifdef SE_DEBUG
 int se_max_received = 0;       /* Instrumentation */
+#endif
 
 #define        PROTOCMD(p, d) \
        ((d) = (p))
@@ -175,6 +179,8 @@
 
        struct callout sc_ifstart_ch;
        struct callout sc_recv_ch;
+       struct kmutex sc_iflock;
+       struct if_percpuq *sc_ipq;
 
        char *sc_tbuf;
        char *sc_rbuf;
@@ -186,7 +192,6 @@
 #define PROTO_AARP     0x10
        int sc_debug;
        int sc_flags;
-#define SE_NEED_RECV 0x1
        int sc_last_timeout;
        int sc_enabled;
 };
@@ -250,7 +255,7 @@
        .d_mmap = nommap,
        .d_kqfilter = nokqfilter,
        .d_discard = nodiscard,
-       .d_flag = D_OTHER
+       .d_flag = D_OTHER | D_MPSAFE
 };
 
 const struct scsipi_periphsw se_switch = {
@@ -319,8 +324,9 @@
        printf("\n");
        SC_DEBUG(periph, SCSIPI_DB2, ("seattach: "));
 
-       callout_init(&sc->sc_ifstart_ch, 0);
-       callout_init(&sc->sc_recv_ch, 0);
+       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);
 
        /*
         * Store information needed to contact our base driver
@@ -360,13 +366,17 @@
                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 */
        }
+       sc->sc_ipq = if_percpuq_create(&sc->sc_ethercom.ec_if);
        ether_ifattach(ifp, myaddr);
        if_register(ifp);
 }
 
-
+/*
+ * Send a command to the device
+ */
 static inline int
 se_scsipi_cmd(struct scsipi_periph *periph, struct scsipi_generic *cmd,
     int cmdlen, u_char *data_addr, int datalen, int retries, int timeout,
@@ -374,21 +384,24 @@
 {
        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 */
+/*
+ * 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;
-       int s = splnet();
 
        se_ifstart(ifp);
-       (void) splx(s);
 }
 
 static void
@@ -396,19 +409,18 @@
 {
        struct ifnet *ifp = v;
        struct se_softc *sc = ifp->if_softc;
-       int s;
 
-       s = splnet();
+       mutex_enter(chan_mtx(sc->sc_periph->periph_channel));
        if (sc->sc_enabled) {
                ifp->if_flags &= ~IFF_OACTIVE;
                se_ifstart(ifp);
        }
-       splx(s);
+       mutex_exit(chan_mtx(sc->sc_periph->periph_channel));
 }
 
 /*
  * Start transmission on the interface.
- * Always called at splnet().
+ * Must be called with the scsipi channel lock held
  */
 static void
 se_ifstart(struct ifnet *ifp)
@@ -426,6 +438,9 @@
        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.
         */
@@ -464,17 +479,13 @@
        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_ASYNC | XS_CTL_DATA_OUT);
+           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);
-       if (sc->sc_flags & SE_NEED_RECV) {
-               sc->sc_flags &= ~SE_NEED_RECV;
-               se_recv((void *) sc);
-       }
 }
 
 
@@ -487,9 +498,7 @@
        struct se_softc *sc = device_private(xs->xs_periph->periph_dev);
        struct scsipi_generic *cmd = xs->cmd;
        struct ifnet *ifp = &sc->sc_ethercom.ec_if;
-       int s;
 
-       s = splnet();
        if (IS_SEND(cmd)) {
                if (xs->error == XS_BUSY) {
                        printf("se: busy, retry txmit\n");
@@ -512,8 +521,10 @@
                } else {
                        int n, ntimeo;
                        n = se_read(sc, xs->data, xs->datalen - xs->resid);
+#ifdef SE_DEBUG
                        if (n > se_max_received)
                                se_max_received = n;
+#endif
                        if (n == 0)
                                ntimeo = se_poll;
                        else if (n >= RDATA_MAX)
@@ -527,18 +538,10 @@
                                          se_poll: ntimeo);
                        }
                        sc->sc_last_timeout = ntimeo;
-                       if (ntimeo == se_poll0  &&
-                           IFQ_IS_EMPTY(&ifp->if_snd) == 0)
-                               /* Output is pending. Do next recv
-                                * after the next send. */
-                               sc->sc_flags |= SE_NEED_RECV;
-                       else {
-                               callout_reset(&sc->sc_recv_ch, ntimeo,
-                                   se_recv, (void *)sc);
-                       }
+                       callout_reset(&sc->sc_recv_ch, ntimeo,
+                           se_recv, (void *)sc);
                }
        }
-       splx(s);
 }
 
 static void
@@ -557,7 +560,7 @@
        error = se_scsipi_cmd(sc->sc_periph,
            (void *)&recv_cmd, sizeof(recv_cmd),
            sc->sc_rbuf, RBUF_LEN, SERETRIES, SETIMEOUT, NULL,
-           XS_CTL_NOSLEEP | XS_CTL_ASYNC | XS_CTL_DATA_IN);
+           XS_CTL_NOSLEEP | XS_CTL_DATA_IN);
        if (error)
                callout_reset(&sc->sc_recv_ch, se_poll, se_recv, (void *)sc);
 }
@@ -667,7 +670,7 @@
                }
 
                /* Pass the packet up. */
-               if_input(ifp, m);
+               if_percpuq_enqueue(sc->sc_ipq, m);
 
        next_packet:
                data += len;
@@ -693,7 +696,7 @@
 se_reset(struct se_softc *sc)
 {
        int error;
-       int s = splnet();
+
 #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
@@ -704,7 +707,6 @@
            XS_CTL_RESET);
 #endif
        error = se_init(sc);
-       splx(s);
        return (error);
 }
 
@@ -826,7 +828,9 @@
                ifp->if_flags |= IFF_RUNNING;
                se_recv(sc);
                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));
        }
        return (error);
 }
@@ -924,7 +928,10 @@
        /* Don't schedule any reads */
        callout_stop(&sc->sc_recv_ch);
 
-       /* How can we abort any scsi cmds in progress? */
+       /* Abort any scsi cmds in progress */
+       mutex_enter(chan_mtx(sc->sc_periph->periph_channel));
+       scsipi_kill_pending(sc->sc_periph);
+       mutex_exit(chan_mtx(sc->sc_periph->periph_channel));
 }
 
 
@@ -938,16 +945,17 @@
        struct ifaddr *ifa = (struct ifaddr *)data;
        struct ifreq *ifr = (struct ifreq *)data;
        struct sockaddr *sa;
-       int s, error = 0;
+       int error = 0;
 
-       s = splnet();
 
        switch (cmd) {
 
        case SIOCINITIFADDR:
+               mutex_enter(&sc->sc_iflock);
                if ((error = se_enable(sc)) != 0)
                        break;
                ifp->if_flags |= IFF_UP;
+               mutex_exit(&sc->sc_iflock);
 
                if ((error = se_set_media(sc, CMEDIA_AUTOSENSE)) != 0)
                        break;
@@ -986,15 +994,20 @@
                         * stop it.
                         */
                        se_stop(sc);
+                       mutex_enter(&sc->sc_iflock);
                        ifp->if_flags &= ~IFF_RUNNING;
                        se_disable(sc);



Home | Main Index | Thread Index | Old Index