Source-Changes-HG archive

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

[src/trunk]: src Make scsipi framework MPSAFE.



details:   https://anonhg.NetBSD.org/src/rev/890eb0e37c6b
branches:  trunk
changeset: 819157:890eb0e37c6b
user:      mlelstv <mlelstv%NetBSD.org@localhost>
date:      Sun Nov 20 15:37:19 2016 +0000

description:
Make scsipi framework MPSAFE.

Data structures are now protected by a per-adapter mutex at IPL_BIO
that is created by the scsibus or atapibus instance when the adapter
is configured.
The enable reference counter and the channel freeze counter which are
currently used by HBA code before the adapter is configured, are made
atomic.
The target drivers are now all tagged as D_MPSAFE.

Almost all HBA drivers still require the kernel lock to present,
so all callbacks into HBA code are still protected by kernel lock
unless the driver is tagged as SCSIPI_ADAPT_MPSAFE.

TODO: refactor sd and cd to use dksubr.

diffstat:

 share/man/man9/scsipi.9       |   21 +-
 sys/dev/scsipi/atapi_wdc.c    |    6 +-
 sys/dev/scsipi/atapiconf.c    |   38 ++-
 sys/dev/scsipi/cd.c           |   50 ++--
 sys/dev/scsipi/ch.c           |    6 +-
 sys/dev/scsipi/if_se.c        |    6 +-
 sys/dev/scsipi/scsi_base.c    |   10 +-
 sys/dev/scsipi/scsiconf.c     |   57 ++---
 sys/dev/scsipi/scsipi_base.c  |  420 +++++++++++++++++++++++++++--------------
 sys/dev/scsipi/scsipi_base.h  |   36 +++-
 sys/dev/scsipi/scsipi_ioctl.c |   32 +-
 sys/dev/scsipi/scsipiconf.c   |   33 ++-
 sys/dev/scsipi/scsipiconf.h   |   50 +++-
 sys/dev/scsipi/sd.c           |   56 +++--
 sys/dev/scsipi/ses.c          |    6 +-
 sys/dev/scsipi/ss.c           |   37 ++-
 sys/dev/scsipi/ss_mustek.c    |    8 +-
 sys/dev/scsipi/ss_scanjet.c   |    6 +-
 sys/dev/scsipi/st.c           |   40 ++-
 sys/dev/scsipi/uk.c           |    6 +-
 20 files changed, 584 insertions(+), 340 deletions(-)

diffs (truncated from 2745 to 300 lines):

diff -r 589937de67ac -r 890eb0e37c6b share/man/man9/scsipi.9
--- a/share/man/man9/scsipi.9   Sun Nov 20 13:28:32 2016 +0000
+++ b/share/man/man9/scsipi.9   Sun Nov 20 15:37:19 2016 +0000
@@ -1,4 +1,4 @@
-.\"    $NetBSD: scsipi.9,v 1.28 2016/07/29 19:27:45 palle Exp $
+.\"    $NetBSD: scsipi.9,v 1.29 2016/11/20 15:37:19 mlelstv Exp $
 .\"
 .\"
 .\" Copyright (c) 2001 Manuel Bouyer.
@@ -55,6 +55,10 @@
 .Fn scsipi_target_detach "struct scsipi_channel *chan" "int target" "int lun" "int flags"
 .Ft int
 .Fn scsipi_thread_call_callback "struct scsipi_channel *chan" "void (*callback)(struct scsipi_channel *, void *)" "void *arg"
+.Ft int
+.Fn scsipi_adapter_addref "struct scsipi_adapter *adapt"
+.Ft void
+.Fn scsipi_adapter_delref "struct scsipi_adapter *adapt"
 .Sh DESCRIPTION
 The
 .Nm
@@ -108,6 +112,14 @@
 see below)
 .It Va int adapt_max_periph
 number of commands the adapter can handle per device
+.It Va int adapt_flags
+adapter properties
+.Bl -tag -width SCSIPI_ADAPT_POLL_ONLY -compact
+.It Dv SCSIPI_ADAPT_POLL_ONLY
+the HBA can't do interrupts
+.It Dv SCSIPI_ADAPT_MPSAFE
+don't acquire the kernel lock when doing HBA callbacks
+.El
 .El
 .Pp
 The following callbacks should be provided through the
@@ -402,6 +414,11 @@
 This callback is optional, and is useful mostly for hot-plug devices.
 For example, this callback would power on or off
 the relevant PCMCIA socket for a PCMCIA controller.
+.Fn scsipi_adapter_addref
+and
+.Fn scsipi_adapter_delref
+maintain a reference count, the enable callback is called appropriately
+for the first reference and the last reference.
 .It int Fn adapt_getgeom "struct scsipi_periph *periph" "struct disk_parms *params" "u_long sectors"
 Optional callback, used by high-level drivers to get the fictitious geometry
 used by the controller's firmware for the specified periph.
@@ -565,7 +582,7 @@
 and
 .Fa arg
 as arguments, from the channel completion thread.
-The callback is run at splbio.
+The callback is run at IPL_BIO with the channel lock held.
 .Fn scsipi_thread_call_callback
 will freeze the channel by one, it's up to the caller to thaw it when
 appropriate.
diff -r 589937de67ac -r 890eb0e37c6b sys/dev/scsipi/atapi_wdc.c
--- a/sys/dev/scsipi/atapi_wdc.c        Sun Nov 20 13:28:32 2016 +0000
+++ b/sys/dev/scsipi/atapi_wdc.c        Sun Nov 20 15:37:19 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: atapi_wdc.c,v 1.122 2016/05/02 19:18:29 christos Exp $ */
+/*     $NetBSD: atapi_wdc.c,v 1.123 2016/11/20 15:37:19 mlelstv Exp $  */
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -25,7 +25,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: atapi_wdc.c,v 1.122 2016/05/02 19:18:29 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: atapi_wdc.c,v 1.123 2016/11/20 15:37:19 mlelstv Exp $");
 
 #ifndef ATADEBUG
 #define ATADEBUG
@@ -153,7 +153,7 @@
 /*
  * Kill off all pending xfers for a periph.
  *
- * Must be called at splbio().
+ * Must be called with adapter lock held
  */
 static void
 wdc_atapi_kill_pending(struct scsipi_periph *periph)
diff -r 589937de67ac -r 890eb0e37c6b sys/dev/scsipi/atapiconf.c
--- a/sys/dev/scsipi/atapiconf.c        Sun Nov 20 13:28:32 2016 +0000
+++ b/sys/dev/scsipi/atapiconf.c        Sun Nov 20 15:37:19 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: atapiconf.c,v 1.88 2016/03/13 09:01:04 tsutsui Exp $   */
+/*     $NetBSD: atapiconf.c,v 1.89 2016/11/20 15:37:19 mlelstv Exp $   */
 
 /*
  * Copyright (c) 1996, 2001 Manuel Bouyer.  All rights reserved.
@@ -25,7 +25,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: atapiconf.c,v 1.88 2016/03/13 09:01:04 tsutsui Exp $");
+__KERNEL_RCSID(0, "$NetBSD: atapiconf.c,v 1.89 2016/11/20 15:37:19 mlelstv Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -152,6 +152,11 @@
        aprint_naive("\n");
        aprint_normal(": %d targets\n", chan->chan_ntargets);
 
+       mutex_init(&chan->chan_adapter->adapt_mtx, MUTEX_DEFAULT, IPL_BIO);
+       cv_init(&chan->chan_cv_thr, "scshut");
+       cv_init(&chan->chan_cv_comp, "sccomp");
+       cv_init(&chan->chan_cv_xs, "xscmd");
+
        /* Initialize the channel. */
        chan->chan_init_cb = NULL;
        chan->chan_init_cb_arg = NULL;
@@ -172,20 +177,16 @@
        struct scsipi_periph *periph;
        int target;
 
-       /* XXXSMP scsipi */
-       KERNEL_LOCK(1, curlwp);
-
+       mutex_enter(chan_mtx(chan));
        for (target = 0; target < chan->chan_ntargets; target++) {
-               periph = scsipi_lookup_periph(chan, target, 0);
+               periph = scsipi_lookup_periph_locked(chan, target, 0);
                if (periph == NULL || periph->periph_dev != child)
                        continue;
                scsipi_remove_periph(chan, periph);
-               free(periph, M_DEVBUF);
+               scsipi_free_periph(periph);
                break;
        }
-
-       /* XXXSMP scsipi */
-       KERNEL_UNLOCK_ONE(curlwp);
+       mutex_exit(chan_mtx(chan));
 }
 
 static int
@@ -201,23 +202,32 @@
         */
        scsipi_channel_shutdown(chan);
 
-       /* XXXSMP scsipi */
+       /* for config_detach() */
        KERNEL_LOCK(1, curlwp);
 
        /*
         * Now detach all of the periphs.
         */
+       mutex_enter(chan_mtx(chan));
        for (target = 0; target < chan->chan_ntargets; target++) {
-               periph = scsipi_lookup_periph(chan, target, 0);
+               periph = scsipi_lookup_periph_locked(chan, target, 0);
                if (periph == NULL)
                        continue;
                error = config_detach(periph->periph_dev, flags);
-               if (error)
+               if (error) {
+                       mutex_exit(chan_mtx(chan));
                        goto out;
+               }
                KASSERT(scsipi_lookup_periph(chan, target, 0) == NULL);
        }
+       mutex_exit(chan_mtx(chan));
 
 out:
+       cv_destroy(&chan->chan_cv_xs);
+       cv_destroy(&chan->chan_cv_comp);
+       cv_destroy(&chan->chan_cv_thr);
+       mutex_destroy(chan_mtx(chan));
+
        /* XXXSMP scsipi */
        KERNEL_UNLOCK_ONE(curlwp);
        return error;
@@ -289,7 +299,7 @@
        } else {
                atapibusprint(sa, device_xname(sc->sc_dev));
                aprint_normal(" not configured\n");
-               free(periph, M_DEVBUF);
+               scsipi_free_periph(periph);
                return NULL;
        }
 }
diff -r 589937de67ac -r 890eb0e37c6b sys/dev/scsipi/cd.c
--- a/sys/dev/scsipi/cd.c       Sun Nov 20 13:28:32 2016 +0000
+++ b/sys/dev/scsipi/cd.c       Sun Nov 20 15:37:19 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cd.c,v 1.332 2016/11/20 02:38:24 pgoyette Exp $        */
+/*     $NetBSD: cd.c,v 1.333 2016/11/20 15:37:19 mlelstv Exp $ */
 
 /*-
  * Copyright (c) 1998, 2001, 2003, 2004, 2005, 2008 The NetBSD Foundation,
@@ -50,7 +50,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cd.c,v 1.332 2016/11/20 02:38:24 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cd.c,v 1.333 2016/11/20 15:37:19 mlelstv Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -211,7 +211,7 @@
        .d_dump = cddump,
        .d_psize = cdsize,
        .d_discard = nodiscard,
-       .d_flag = D_DISK
+       .d_flag = D_DISK | D_MPSAFE
 };
 
 const struct cdevsw cd_cdevsw = {
@@ -226,7 +226,7 @@
        .d_mmap = nommap,
        .d_kqfilter = nokqfilter,
        .d_discard = nodiscard,
-       .d_flag = D_DISK
+       .d_flag = D_DISK | D_MPSAFE
 };
 
 static struct dkdriver cddkdriver = {
@@ -316,7 +316,9 @@
 cddetach(device_t self, int flags)
 {
        struct cd_softc *cd = device_private(self);
-       int s, bmaj, cmaj, i, mn;
+       struct scsipi_periph *periph = cd->sc_periph;
+       struct scsipi_channel *chan = periph->periph_channel;
+       int bmaj, cmaj, i, mn;
 
        if (cd->sc_dk.dk_openmask != 0 && (flags & DETACH_FORCE) == 0)
                return EBUSY;
@@ -334,7 +336,7 @@
        /* kill any pending restart */
        callout_stop(&cd->sc_callout);
 
-       s = splbio();
+       mutex_enter(chan_mtx(chan));
 
        /* Kill off any queued buffers. */
        bufq_drain(cd->buf_queue);
@@ -342,7 +344,7 @@
        /* Kill off any pending commands. */
        scsipi_kill_pending(cd->sc_periph);
 
-       splx(s);
+       mutex_exit(chan_mtx(chan));
 
        bufq_free(cd->buf_queue);
 
@@ -576,8 +578,8 @@
        struct cd_softc *cd = device_lookup_private(&cd_cd,CDUNIT(bp->b_dev));
        struct disklabel *lp;
        struct scsipi_periph *periph = cd->sc_periph;
+       struct scsipi_channel *chan = periph->periph_channel;
        daddr_t blkno;
-       int s;
 
        SC_DEBUG(cd->sc_periph, SCSIPI_DB2, ("cdstrategy "));
        SC_DEBUG(cd->sc_periph, SCSIPI_DB1,
@@ -719,7 +721,7 @@
                                cd->params.blksize;
                }
        }
-       s = splbio();
+       mutex_enter(chan_mtx(chan));
 
        /*
         * Place it in the queue of disk activities for this disk.
@@ -733,9 +735,9 @@
         * Tell the device to get going on the transfer if it's
         * not doing anything, otherwise just wait for completion
         */
-       cdstart(cd->sc_periph);
-
-       splx(s);
+       cdstart(periph);
+
+       mutex_exit(chan_mtx(chan));
        return;
 
 done:
@@ -759,8 +761,8 @@
  * have been made of the scsi driver, to ensure that the queue
  * continues to be drained.
  *
- * must be called at the correct (highish) spl level
- * cdstart() is called at splbio from cdstrategy, cdrestart and scsipi_done
+ * must be called with channel lock held
+ * cdstart() is called from cdstrategy, cdrestart and scsipi_done
  */
 static void
 cdstart(struct scsipi_periph *periph)
@@ -774,6 +776,7 @@
        int flags, nblks, cmdlen, error __diagused;
 
        SC_DEBUG(periph, SCSIPI_DB2, ("cdstart "));
+
        /*
         * Check if the device has room for another command
         */
@@ -785,7 +788,7 @@



Home | Main Index | Thread Index | Old Index