Add some robustness to the error-requeue code. FreeBSD-5's (new) ata driver
authorMatthew Dillon <dillon@dragonflybsd.org>
Tue, 17 Aug 2004 20:59:39 +0000 (20:59 +0000)
committerMatthew Dillon <dillon@dragonflybsd.org>
Tue, 17 Aug 2004 20:59:39 +0000 (20:59 +0000)
had an issue with the donecount not being properly reset.  This issue is not
believed to occur with the old code but add sanity checks to be sure.

sys/dev/disk/ata/ata-all.c
sys/dev/disk/ata/ata-disk.c
sys/dev/disk/ata/atapi-all.c

index b47f50e..9bcb7ee 100644 (file)
@@ -26,7 +26,7 @@
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
  * $FreeBSD: src/sys/dev/ata/ata-all.c,v 1.50.2.45 2003/03/12 14:47:12 sos Exp $
- * $DragonFly: src/sys/dev/disk/ata/ata-all.c,v 1.20 2004/06/24 07:55:26 dillon Exp $
+ * $DragonFly: src/sys/dev/disk/ata/ata-all.c,v 1.21 2004/08/17 20:59:39 dillon Exp $
  */
 
 #include "opt_ata.h"
@@ -702,6 +702,12 @@ ata_start(struct ata_channel *ch)
        TAILQ_REMOVE(&ch->ata_queue, ad_request, chain);
        ch->active = ATA_ACTIVE_ATA;
        ch->running = ad_request;
+
+       /*
+        * The donecount had better be 0 here because the channel may not
+        * have retained the setup for the request (if a retry).
+        */
+       KKASSERT(ad_request->donecount == 0);
        if (ad_transfer(ad_request) == ATA_OP_CONTINUES) {
            splx(s);
            return;
index c912af5..5ec8862 100644 (file)
@@ -26,7 +26,7 @@
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
  * $FreeBSD: src/sys/dev/ata/ata-disk.c,v 1.60.2.24 2003/01/30 07:19:59 sos Exp $
- * $DragonFly: src/sys/dev/disk/ata/ata-disk.c,v 1.20 2004/06/23 06:52:26 dillon Exp $
+ * $DragonFly: src/sys/dev/disk/ata/ata-disk.c,v 1.21 2004/08/17 20:59:39 dillon Exp $
  */
 
 #include "opt_ata.h"
@@ -81,6 +81,7 @@ static struct cdevsw ad_cdevsw = {
 };
 
 /* prototypes */
+static void ad_requeue(struct ata_channel *, struct ad_request *);
 static void ad_invalidatequeue(struct ad_softc *, struct ad_request *);
 static int ad_tagsupported(struct ad_softc *);
 static void ad_timeout(struct ad_request *);
@@ -439,6 +440,19 @@ ad_start(struct ata_device *atadev)
     TAILQ_INSERT_TAIL(&atadev->channel->ata_queue, request, chain);
 }
 
+void
+ad_requeue(struct ata_channel *chan, struct ad_request *req)
+{
+        if (req->donecount) {
+                ata_printf(chan, -1,
+                       "WARNING: resetting donecount %u for retry\n",
+                        req->donecount);
+                req->bytecount += req->donecount;
+                req->donecount = 0;
+        }
+        TAILQ_INSERT_HEAD(&chan->ata_queue, req, chain);
+}
+
 int
 ad_transfer(struct ad_request *request)
 {
@@ -588,7 +602,7 @@ transfer_failed:
 
     /* if retries still permit, reinject this request */
     if (request->retries++ < AD_MAX_RETRIES)
-       TAILQ_INSERT_HEAD(&adp->device->channel->ata_queue, request, chain);
+       ad_requeue(adp->device->channel, request);
     else {
        /* retries all used up, return error */
        request->bp->b_error = EIO;
@@ -642,7 +656,7 @@ ad_interrupt(struct ad_request *request)
                ata_dmainit(adp->device, ata_pmode(adp->device->param), -1, -1);
                printf(" falling back to PIO mode\n");
            }
-           TAILQ_INSERT_HEAD(&adp->device->channel->ata_queue, request, chain);
+           ad_requeue(adp->device->channel, request);
            return ATA_OP_FINISHED;
        }
 
@@ -653,7 +667,7 @@ ad_interrupt(struct ad_request *request)
            ata_dmainit(adp->device, ata_pmode(adp->device->param), -1, -1);
            request->flags |= ADR_F_FORCE_PIO;
            printf(" trying PIO mode\n");
-           TAILQ_INSERT_HEAD(&adp->device->channel->ata_queue, request, chain);
+           ad_requeue(adp->device->channel, request);
            return ATA_OP_FINISHED;
        }
 
@@ -834,7 +848,7 @@ ad_invalidatequeue(struct ad_softc *adp, struct ad_request *request)
            if (tmpreq == request || tmpreq == NULL)
                continue;
            untimeout((timeout_t *)ad_timeout, tmpreq, tmpreq->timeout_handle);
-           TAILQ_INSERT_HEAD(&adp->device->channel->ata_queue, tmpreq, chain);
+           ad_requeue(adp->device->channel, tmpreq);
        }
        if (ata_command(adp->device, ATA_C_NOP,
                        0, 0, ATA_C_F_FLUSHQUEUE, ATA_WAIT_READY))
@@ -900,7 +914,7 @@ ad_timeout(struct ad_request *request)
 
     /* if retries still permit, reinject this request */
     if (request->retries++ < AD_MAX_RETRIES) {
-       TAILQ_INSERT_HEAD(&adp->device->channel->ata_queue, request, chain);
+       ad_requeue(adp->device->channel, request);
     }
     else {
        /* retries all used up, return error */
index aa7131d..2deae32 100644 (file)
@@ -26,7 +26,7 @@
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
  * $FreeBSD: src/sys/dev/ata/atapi-all.c,v 1.46.2.18 2002/10/31 23:10:33 thomas Exp $
- * $DragonFly: src/sys/dev/disk/ata/atapi-all.c,v 1.12 2004/07/06 19:00:06 dillon Exp $
+ * $DragonFly: src/sys/dev/disk/ata/atapi-all.c,v 1.13 2004/08/17 20:59:39 dillon Exp $
  */
 
 #include "opt_ata.h"
@@ -50,6 +50,7 @@
 #include "atapi-all.h"
 
 /* prototypes */
+static void atapi_requeue(struct ata_channel *chan, struct atapi_request *req);
 static void atapi_read(struct atapi_request *, int);
 static void atapi_write(struct atapi_request *, int);
 static void atapi_finish(struct atapi_request *);
@@ -249,6 +250,19 @@ atapi_start(struct ata_device *atadev)
     }
 }
 
+void
+atapi_requeue(struct ata_channel *chan, struct atapi_request *req)
+{
+        if (req->donecount) {
+                ata_printf(chan, -1,
+                       "WARNING: atapi resetting donecount %u for retry\n",
+                         req->donecount);
+                req->bytecount += req->donecount;
+                req->donecount = 0;
+        }
+       TAILQ_INSERT_HEAD(&chan->atapi_queue, req, chain);
+}
+
 int
 atapi_transfer(struct atapi_request *request)
 {
@@ -629,7 +643,7 @@ atapi_timeout(struct atapi_request *request)
 
     /* if retries still permit, reinject this request */
     if (request->retries++ < ATAPI_MAX_RETRIES) {
-       TAILQ_INSERT_HEAD(&atadev->channel->atapi_queue, request, chain);
+       atapi_requeue(atadev->channel, request);
     }
     else {
        /* retries all used up, return error */