Source-Changes-HG archive

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

[src/trunk]: src/sys/dev/mca Fix serious bug in bounce buf handling - the EDF...



details:   https://anonhg.NetBSD.org/src/rev/e088499d280b
branches:  trunk
changeset: 508915:e088499d280b
user:      jdolecek <jdolecek%NetBSD.org@localhost>
date:      Sun Apr 22 11:32:49 2001 +0000

description:
Fix serious bug in bounce buf handling - the EDF_BOUNCEBUF flag needs
to be cleared always in edmcadone(), otherwise if there is a write
via bounce buffer followed by read directly to buf, the read operation
would return trashed data (the buf data would get overwritten
by contents of bounce buffer in edmcadone()).
Reset b_resid as necessary when the i/o is done, too.

g/c some unneeded stuff, use lockmgr()-style locking in ed_[un]lock(),
better avoid some deadlocks

These changes make the driver quite a bit more stable. It's actually
reliable enough to be possible to newfs the drive and use it for
read/write filesystem now.

diffstat:

 sys/dev/mca/ed_mca.c  |  143 ++++++++++++++-----------------------------------
 sys/dev/mca/edc_mca.c |   28 +++++---
 sys/dev/mca/edcreg.h  |    3 +-
 sys/dev/mca/edcvar.h  |    3 +-
 sys/dev/mca/edvar.h   |   28 ++++-----
 5 files changed, 76 insertions(+), 129 deletions(-)

diffs (truncated from 495 to 300 lines):

diff -r 97ea5da91909 -r e088499d280b sys/dev/mca/ed_mca.c
--- a/sys/dev/mca/ed_mca.c      Sun Apr 22 05:35:35 2001 +0000
+++ b/sys/dev/mca/ed_mca.c      Sun Apr 22 11:32:49 2001 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ed_mca.c,v 1.2 2001/04/19 17:17:29 jdolecek Exp $      */
+/*     $NetBSD: ed_mca.c,v 1.3 2001/04/22 11:32:49 jdolecek Exp $      */
 
 /*
  * Copyright (c) 2001 The NetBSD Foundation, Inc.
@@ -77,8 +77,6 @@
 
 #define        EDLABELDEV(dev) (MAKEDISKDEV(major(dev), DISKUNIT(dev), RAW_PART))
 
-#define ED_BB_SIZE     65536
-
 /* XXX: these should go elsewhere */
 cdev_decl(edmca);
 bdev_decl(edmca);
@@ -98,7 +96,6 @@
 static void    edgetdisklabel  __P((struct ed_softc *));
 static void    edgetdefaultlabel __P((struct ed_softc *, struct disklabel *));
 static void    ed_shutdown __P((void*));
-// static void edstart   __P((void *));
 static void    __edstart __P((struct ed_softc*, struct buf *));
 static void    bad144intern __P((struct ed_softc *));
 static void    edworker __P((void *));
@@ -149,6 +146,7 @@
 
        BUFQ_INIT(&ed->sc_q);
        spinlockinit(&ed->sc_q_lock, "edbqlock", 0);
+       lockinit(&ed->sc_lock, PRIBIO | PCATCH, "edlck", 0, 0);
 
        if (ed_get_params(ed)) {
                printf(": IDENTIFY failed, no disk found\n");
@@ -173,7 +171,7 @@
        
        /* Create a DMA map for mapping individual transfer bufs */
        if ((error = bus_dmamap_create(ed->sc_dmat, 65536, 1,
-               65536, 0, BUS_DMA_WAITOK | BUS_DMA_ALLOCNOW,
+               65536, 65536, BUS_DMA_WAITOK | BUS_DMA_ALLOCNOW,
                &ed->dmamap_xfer)) != 0) {
                printf("%s: unable to create xfer DMA map, error=%d\n",
                        ed->sc_dev.dv_xname, error);
@@ -184,9 +182,9 @@
         * Allocate DMA memory used in case where passed buf isn't
         * physically contiguous.
         */
-       ed->sc_dmam_sz = ED_BB_SIZE;
+       ed->sc_dmam_sz = MAXPHYS;
        if ((error = bus_dmamem_alloc(ed->sc_dmat, ed->sc_dmam_sz,
-               65536, 65536, ed->sc_dmam, 1, &nsegs,
+               ed->sc_dmam_sz, 65536, ed->sc_dmam, 1, &nsegs,
                BUS_DMA_WAITOK|BUS_DMA_STREAMING)) != 0) {
                printf("%s: unable to allocate DMA memory for xfer, errno=%d\n",
                                ed->sc_dev.dv_xname, error);
@@ -321,70 +319,32 @@
        biodone(bp);
 }
 
-#if 0
-/*
- * Queue a drive for I/O.
- */
-static void
-edstart(arg)
-       void *arg;
-{
-       struct ed_softc *wd = arg;
-       struct buf *bp;
-
-       WDCDEBUG_PRINT(("edstart %s\n", wd->sc_dev.dv_xname),
-           DEBUG_XFERS);
-       while (wd->openings > 0) {
-
-               /* Is there a buf for us ? */
-               if ((bp = BUFQ_FIRST(&wd->sc_q)) == NULL)
-                       return;
-               BUFQ_REMOVE(&wd->sc_q, bp);
-       
-               /* 
-                * Make the command. First lock the device
-                */
-               wd->openings--;
-
-               wd->retries = 0;
-               __edstart(wd, bp);
-       }
-}
-#endif
-
 static void
 __edstart(ed, bp)
        struct ed_softc *ed;
        struct buf *bp;
 {
        u_int16_t cmd_args[4];
-       int error = 0;
+       int error=0;
        u_int16_t track;
        u_int16_t cyl;
        u_int8_t head;
        u_int8_t sector;
 
-       WDCDEBUG_PRINT(("__edstart %s: %lu %lu %u\n", ed->sc_dev.dv_xname,
+       WDCDEBUG_PRINT(("__edstart %s (%s): %lu %lu %u\n", ed->sc_dev.dv_xname,
+               (bp->b_flags & B_READ) ? "read" : "write",
                bp->b_bcount, bp->b_resid, bp->b_rawblkno),
            DEBUG_XFERS);
 
        ed->sc_bp = bp;
 
-       /* Get physical bus mapping for buf */
-       if ((error = bus_dmamap_load(ed->sc_dmat, ed->dmamap_xfer,
+       /* Get physical bus mapping for buf. */
+       if (bus_dmamap_load(ed->sc_dmat, ed->dmamap_xfer,
                        bp->b_data, bp->b_bcount, NULL,
-                       BUS_DMA_WAITOK|BUS_DMA_STREAMING)) != 0) {
+                       BUS_DMA_WAITOK|BUS_DMA_STREAMING) != 0) {
 
-#ifdef DEBUG
-               if (bp->b_bcount > ed->sc_dmam_sz)
-                       panic("%s: edstart: bp->b_bcount: %lu > %lu",
-                               ed->sc_dev.dv_xname,
-                               bp->b_bcount, ed->sc_dmam_sz
-                               );
-#endif
                /*
                 * Use our DMA safe memory to get data to/from device.
-                * For write, copy the data from buf to it now, too.
                 */
                if ((error = bus_dmamap_load(ed->sc_dmat, ed->dmamap_xfer,
                        ed->sc_dmamkva, bp->b_bcount, NULL,
@@ -399,6 +359,7 @@
                if ((bp->b_flags & B_READ) == 0)
                        memcpy(ed->sc_dmamkva, bp->b_data, bp->b_bcount);
        }
+
        ed->sc_flags |= EDF_DMAMAP_LOADED;
 
        track = bp->b_rawblkno / ed->sectors;
@@ -412,7 +373,7 @@
 
        /* Instrumentation. */
        disk_busy(&ed->sc_dk);
-       ed->sc_dk_busy++;
+       ed->sc_flags |= EDF_DK_BUSY;
 
        /* Read or Write Data command */
        cmd_args[0] = 2;        /* Options 0000010 */
@@ -444,14 +405,20 @@
        if (ed->sc_error) {
                bp->b_error = ed->sc_error;
                bp->b_flags |= B_ERROR;
+       } else {
+               /* Set resid, most commonly to zero. */
+               bp->b_resid = ed->sc_status_block[SB_RESBLKCNT_IDX] * DEV_BSIZE;
        }
 
-       /* If no error and using bounce buffer, copy the data now. */
+       /*
+        * If read transfer finished without error and using a bounce
+        * buffer, copy the data to buf.
+        */
        if ((bp->b_flags & B_ERROR) == 0 && (ed->sc_flags & EDF_BOUNCEBUF)
             && (bp->b_flags & B_READ)) {
                memcpy(bp->b_data, ed->sc_dmamkva, bp->b_bcount);
-               ed->sc_flags &= ~EDF_BOUNCEBUF;
        }
+       ed->sc_flags &= ~EDF_BOUNCEBUF;
 
        /* Unload buf from DMA map */
        if (ed->sc_flags & EDF_DMAMAP_LOADED) {
@@ -460,34 +427,19 @@
        }
 
        /* If disk was busied, unbusy it now */
-       if (ed->sc_dk_busy > 0) {
+       if (ed->sc_flags & EDF_DK_BUSY) {
                disk_unbusy(&ed->sc_dk, (bp->b_bcount - bp->b_resid));
-               ed->sc_dk_busy--;
+               ed->sc_flags &= ~EDF_DK_BUSY;
        }
 
+       ed->sc_flags &= ~EDF_IODONE;
+
 #if NRND > 0
        rnd_add_uint32(&ed->rnd_source, bp->b_blkno);
 #endif
        biodone(bp);
 }
 
-#if 0
-void
-edmcarestart(v)
-       void *v;
-{
-       struct ed_softc *wd = v;
-       struct buf *bp = wd->sc_bp;
-       int s;
-       WDCDEBUG_PRINT(("wdrestart %s\n", wd->sc_dev.dv_xname),
-           DEBUG_XFERS);
-
-       s = splbio();
-       __edstart(v, bp);
-       splx(s);
-}
-#endif
-
 int
 edmcaread(dev, uio, flags)
        dev_t dev;
@@ -510,13 +462,10 @@
 
 /*
  * Wait interruptibly for an exclusive lock.
- *
- * XXX
- * Several drivers do this; it should be abstracted and made MP-safe.
  */
 static int
-ed_lock(wd)
-       struct ed_softc *wd;
+ed_lock(ed)
+       struct ed_softc *ed;
 {
        int error;
        int s;
@@ -524,35 +473,22 @@
        WDCDEBUG_PRINT(("ed_lock\n"), DEBUG_FUNCS);
 
        s = splbio();
+       error = lockmgr(&ed->sc_lock, LK_EXCLUSIVE, NULL);
+       splx(s);
 
-       while ((wd->sc_flags & WDF_LOCKED) != 0) {
-               wd->sc_flags |= WDF_WANTED;
-               if ((error = tsleep(wd, PRIBIO | PCATCH,
-                   "wdlck", 0)) != 0) {
-                       splx(s);
-                       return error;
-               }
-       }
-       wd->sc_flags |= WDF_LOCKED;
-       splx(s);
-       return 0;
+       return (error);
 }
 
 /*
  * Unlock and wake up any waiters.
  */
 static void
-ed_unlock(wd)
-       struct ed_softc *wd;
+ed_unlock(ed)
+       struct ed_softc *ed;
 {
-
        WDCDEBUG_PRINT(("ed_unlock\n"), DEBUG_FUNCS);
 
-       wd->sc_flags &= ~WDF_LOCKED;
-       if ((wd->sc_flags & WDF_WANTED) != 0) {
-               wd->sc_flags &= ~WDF_WANTED;
-               wakeup(wd);
-       }
+       (void) lockmgr(&ed->sc_lock, LK_RELEASE, NULL);
 }
 
 int
@@ -769,7 +705,7 @@
        struct disklabel newlabel;
 #endif
 
-       WDCDEBUG_PRINT(("wdioctl\n"), DEBUG_FUNCS);
+       WDCDEBUG_PRINT(("edioctl\n"), DEBUG_FUNCS);
 
        if ((wd->sc_flags & WDF_LOADED) == 0)
                return EIO;
@@ -912,7 +848,7 @@
        }
 
 #ifdef DIAGNOSTIC
-       panic("wdioctl: impossible");
+       panic("edioctl: impossible");
 #endif
 }
 
@@ -1166,15 +1102,20 @@
                        simple_unlock(&ed->sc_q_lock);
 
                        /* Schedule i/o operation */
+                       ed->sc_error = 0;
                        s = splbio();
                        __edstart(ed, bp);
                        splx(s);
 
                        /*
                         * Wait until the command executes; edc_intr() wakes
-                        * as up.
+                        * us up.



Home | Main Index | Thread Index | Old Index