Date: Mon, 12 Oct 2009 20:35:05 GMT From: Alexander Motin <mav@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 169438 for review Message-ID: <200910122035.n9CKZ5xo078012@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=169438 Change 169438 by mav@mav_mavtest on 2009/10/12 20:34:35 Freeze devices CCB queues on errors. Previos approach with just SIM freeze is not enough for proper recovery, as it dropped just before recovery, not after, creating window for another request to run before. Affected files ... .. //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.c#67 edit .. //depot/projects/scottl-camlock/src/sys/dev/ata/ata-all.c#30 edit Differences ... ==== //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.c#67 (text+ko) ==== @@ -987,12 +987,14 @@ } /* On error, complete the rest of commands with error statuses. */ if (err) { - if (!ch->readlog) - xpt_freeze_simq(ch->sim, ch->numrslots); if (ch->frozen) { union ccb *fccb = ch->frozen; ch->frozen = NULL; fccb->ccb_h.status = CAM_REQUEUE_REQ | CAM_RELEASE_SIMQ; + if (!(fccb->ccb_h.status & CAM_DEV_QFRZN)) { + xpt_freeze_devq(fccb->ccb_h.path, 1); + fccb->ccb_h.status |= CAM_DEV_QFRZN; + } xpt_done(fccb); } for (i = 0; i < ch->numslots; i++) { @@ -1125,8 +1127,6 @@ if (error) { device_printf(slot->dev, "DMA load error\n"); - if (!ch->readlog) - xpt_freeze_simq(ch->sim, 1); ahci_end_transaction(slot, AHCI_ERR_INVALID); return; } @@ -1165,8 +1165,6 @@ /* Setup the FIS for this request */ if (!(fis_size = ahci_setup_fis(ctp, ccb, slot->slot))) { device_printf(ch->dev, "Setting up SATA FIS failed\n"); - if (!ch->readlog) - xpt_freeze_simq(ch->sim, 1); ahci_end_transaction(slot, AHCI_ERR_INVALID); return; } @@ -1233,7 +1231,6 @@ /* Kick controller into sane state */ ahci_stop(ch->dev); ahci_start(ch->dev); - xpt_freeze_simq(ch->sim, 1); } ahci_end_transaction(slot, et); return; @@ -1266,13 +1263,15 @@ ahci_stop(ch->dev); ahci_start(ch->dev); - if (!ch->readlog) - xpt_freeze_simq(ch->sim, ch->numrslots); /* Handle frozen command. */ if (ch->frozen) { union ccb *fccb = ch->frozen; ch->frozen = NULL; fccb->ccb_h.status = CAM_REQUEUE_REQ | CAM_RELEASE_SIMQ; + if (!(fccb->ccb_h.status & CAM_DEV_QFRZN)) { + xpt_freeze_devq(fccb->ccb_h.path, 1); + fccb->ccb_h.status |= CAM_DEV_QFRZN; + } xpt_done(fccb); } /* Handle command with timeout. */ @@ -1330,10 +1329,14 @@ BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE); bus_dmamap_unload(ch->dma.data_tag, slot->dma.data_map); } + /* In case of error, freeze device for proper recovery. */ + if ((et != AHCI_ERR_NONE) && (!ch->readlog) && + !(ccb->ccb_h.status & CAM_DEV_QFRZN)) { + xpt_freeze_devq(ccb->ccb_h.path, 1); + ccb->ccb_h.status |= CAM_DEV_QFRZN; + } /* Set proper result status. */ ccb->ccb_h.status &= ~CAM_STATUS_MASK; - if (et != AHCI_ERR_NONE) - ccb->ccb_h.status |= CAM_RELEASE_SIMQ; switch (et) { case AHCI_ERR_NONE: ccb->ccb_h.status |= CAM_REQ_CMP; @@ -1347,6 +1350,7 @@ ccb->ccb_h.status |= CAM_REQUEUE_REQ; break; case AHCI_ERR_TFE: + case AHCI_ERR_NCQ: if (ccb->ccb_h.func_code == XPT_SCSI_IO) { ccb->ccb_h.status |= CAM_SCSI_STATUS_ERROR; ccb->csio.scsi_status = SCSI_STATUS_CHECK_COND; @@ -1355,13 +1359,19 @@ } break; case AHCI_ERR_SATA: - ccb->ccb_h.status |= CAM_UNCOR_PARITY; + if (!ch->readlog) { + xpt_freeze_simq(ch->sim, 1); + ccb->ccb_h.status |= CAM_RELEASE_SIMQ; + } + ccb->ccb_h.status |= CAM_UNCOR_PARITY; break; case AHCI_ERR_TIMEOUT: + if (!ch->readlog) { + xpt_freeze_simq(ch->sim, 1); + ccb->ccb_h.status |= CAM_RELEASE_SIMQ; + } ccb->ccb_h.status |= CAM_CMD_TIMEOUT; break; - case AHCI_ERR_NCQ: - ccb->ccb_h.status |= CAM_ATA_STATUS_ERROR; default: ccb->ccb_h.status |= CAM_REQ_CMP_ERR; } @@ -1445,7 +1455,8 @@ ataio->cmd.lba_low = 0x10; ataio->cmd.lba_mid = 0; ataio->cmd.lba_mid_exp = 0; - + /* Freeze SIM while doing READ LOG EXT. */ + xpt_freeze_simq(ch->sim, 1); ahci_begin_transaction(dev, ccb); } @@ -1500,6 +1511,7 @@ } free(ccb->ataio.data_ptr, M_AHCI); xpt_free_ccb(ccb); + xpt_release_simq(ch->sim, TRUE); } static void @@ -1624,12 +1636,15 @@ if (bootverbose) device_printf(dev, "AHCI reset...\n"); - xpt_freeze_simq(ch->sim, ch->numrslots); /* Requeue freezed command. */ if (ch->frozen) { union ccb *fccb = ch->frozen; ch->frozen = NULL; fccb->ccb_h.status = CAM_REQUEUE_REQ | CAM_RELEASE_SIMQ; + if (!(fccb->ccb_h.status & CAM_DEV_QFRZN)) { + xpt_freeze_devq(fccb->ccb_h.path, 1); + fccb->ccb_h.status |= CAM_DEV_QFRZN; + } xpt_done(fccb); } /* Kill the engine and requeue all running commands. */ @@ -1641,6 +1656,8 @@ /* XXX; Commands in loading state. */ ahci_end_transaction(&ch->slot[i], AHCI_ERR_INNOCENT); } + /* Tell the XPT about the event */ + xpt_async(AC_BUS_RESET, ch->path, NULL); /* Disable port interrupts */ ATA_OUTL(ch->r_mem, AHCI_P_IE, 0); /* Reset and reconnect PHY, */ @@ -1670,8 +1687,6 @@ AHCI_P_IX_DS | AHCI_P_IX_PS | (ctlr->ccc ? 0 : AHCI_P_IX_DHR))); if (bootverbose) device_printf(dev, "AHCI reset done: device found\n"); - /* Tell the XPT about the event */ - xpt_async(AC_BUS_RESET, ch->path, NULL); } static int ==== //depot/projects/scottl-camlock/src/sys/dev/ata/ata-all.c#30 (text+ko) ==== @@ -1299,9 +1299,11 @@ union ccb *ccb = request->ccb; ccb->ccb_h.status &= ~CAM_STATUS_MASK; - if (request->flags & ATA_R_TIMEOUT) - ccb->ccb_h.status |= CAM_CMD_TIMEOUT; - else if (request->status & ATA_S_ERROR) { + if (request->flags & ATA_R_TIMEOUT) { + xpt_freeze_simq(ch->sim, 1); + ccb->ccb_h.status &= ~CAM_STATUS_MASK; + ccb->ccb_h.status |= CAM_CMD_TIMEOUT | CAM_RELEASE_SIMQ; + } else if (request->status & ATA_S_ERROR) { if (ccb->ccb_h.func_code == XPT_ATA_IO) { ccb->ccb_h.status |= CAM_ATA_STATUS_ERROR; } else { @@ -1314,9 +1316,10 @@ ccb->ccb_h.status |= CAM_REQ_CMP_ERR; else ccb->ccb_h.status |= CAM_REQ_CMP; - if ((ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) { - xpt_freeze_simq(ch->sim, 1); - ccb->ccb_h.status |= CAM_RELEASE_SIMQ; + if ((ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP && + !(ccb->ccb_h.status & CAM_DEV_QFRZN)) { + xpt_freeze_devq(ccb->ccb_h.path, 1); + ccb->ccb_h.status |= CAM_DEV_QFRZN; } if (ccb->ccb_h.func_code == XPT_ATA_IO && ((request->status & ATA_S_ERROR) ||
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200910122035.n9CKZ5xo078012>