From owner-p4-projects@FreeBSD.ORG Mon Oct 12 20:35:06 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 1B2A9106568B; Mon, 12 Oct 2009 20:35:06 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D3E061065676 for ; Mon, 12 Oct 2009 20:35:05 +0000 (UTC) (envelope-from mav@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id B841A8FC0A for ; Mon, 12 Oct 2009 20:35:05 +0000 (UTC) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n9CKZ5EW078014 for ; Mon, 12 Oct 2009 20:35:05 GMT (envelope-from mav@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n9CKZ5xo078012 for perforce@freebsd.org; Mon, 12 Oct 2009 20:35:05 GMT (envelope-from mav@freebsd.org) Date: Mon, 12 Oct 2009 20:35:05 GMT Message-Id: <200910122035.n9CKZ5xo078012@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to mav@freebsd.org using -f From: Alexander Motin To: Perforce Change Reviews Cc: Subject: PERFORCE change 169438 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Oct 2009 20:35:06 -0000 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) ||