Date: Sun, 1 Nov 2009 23:37:16 GMT From: Alexander Motin <mav@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 170059 for review Message-ID: <200911012337.nA1NbGSc034715@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/chv.cgi?CH=170059 Change 170059 by mav@mav_mavtest on 2009/11/01 23:36:18 Undo previous timeout handling on ATA XPT layer and implement it on SIM level, same as done for SPI transport. Potentially SIM has lesser possible recovery methods (just bus hard- or device soft-reset), but same time it has full information about the problem to use them properly. Rework siis timeout handling, to make driver wait for concurrently running requests completion or timeout. Concurrent requests may not be affected by fault that cause timeout, especially with PMP used. Affected files ... .. //depot/projects/scottl-camlock/src/sys/cam/ata/ata_xpt.c#49 edit .. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#116 edit .. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.h#19 edit .. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt_internal.h#12 edit .. //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.c#72 edit .. //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.h#25 edit .. //depot/projects/scottl-camlock/src/sys/dev/ata/ata-all.c#34 edit .. //depot/projects/scottl-camlock/src/sys/dev/siis/siis.c#12 edit .. //depot/projects/scottl-camlock/src/sys/dev/siis/siis.h#2 edit Differences ... ==== //depot/projects/scottl-camlock/src/sys/cam/ata/ata_xpt.c#49 (text+ko) ==== @@ -182,12 +182,10 @@ struct cam_ed *device, void *async_arg); static void ata_action(union ccb *start_ccb); -static void ata_done(union ccb *done_ccb); static struct xpt_xport ata_xport = { .alloc_device = ata_alloc_device, .action = ata_action, - .done = ata_done, .async = ata_dev_async, }; @@ -1227,36 +1225,6 @@ } static void -ata_done(union ccb *done_ccb) -{ - struct cam_path *path; - union ccb *work_ccb; - - switch (done_ccb->ccb_h.status & CAM_STATUS_MASK) { - case CAM_CMD_TIMEOUT: - case CAM_UNCOR_PARITY: - work_ccb = xpt_alloc_ccb_nowait(); - if (work_ccb == NULL) - break; - if (xpt_create_path(&path, xpt_periph, done_ccb->ccb_h.path_id, - CAM_TARGET_WILDCARD, CAM_LUN_WILDCARD) != CAM_REQ_CMP) { - xpt_free_ccb(work_ccb); - break; - } - xpt_setup_ccb(&work_ccb->ccb_h, path, CAM_PRIORITY_NORMAL); - work_ccb->ccb_h.func_code = XPT_RESET_BUS; - work_ccb->ccb_h.cbfcnp = NULL; - CAM_DEBUG(path, CAM_DEBUG_SUBTRACE, ("Resetting Bus\n")); - xpt_action(work_ccb); - xpt_free_ccb(work_ccb); - break; - } - - /* Call default done handler. */ - xpt_done_default(done_ccb); -} - -static void scsi_set_transfer_settings(struct ccb_trans_settings *cts, struct cam_ed *device, int async_update) { ==== //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#116 (text+ko) ==== @@ -3033,13 +3033,6 @@ } void -xpt_done_default(union ccb *done_ccb) -{ - /* Call the peripheral driver's callback */ - (*done_ccb->ccb_h.cbfcnp)(done_ccb->ccb_h.path->periph, done_ccb); -} - -void xpt_polled_action(union ccb *start_ccb) { u_int32_t timeout; @@ -3780,7 +3773,6 @@ static struct xpt_xport xport_default = { .alloc_device = xpt_alloc_device_default, .action = xpt_action_default, - .done = xpt_done_default, .async = xpt_dev_async_default, }; @@ -5067,8 +5059,8 @@ xpt_run_dev_sendq(ccb_h->path->bus); } - /* Call the XPT's callback */ - (*(ccb_h->path->bus->xport->done))((union ccb *)ccb_h); + /* Call the peripheral driver's callback */ + (*ccb_h->cbfcnp)(ccb_h->path->periph, (union ccb *)ccb_h); } } ==== //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.h#19 (text+ko) ==== @@ -87,7 +87,6 @@ void xpt_action(union ccb *new_ccb); void xpt_action_default(union ccb *new_ccb); -void xpt_done_default(union ccb *new_ccb); void xpt_setup_ccb(struct ccb_hdr *ccb_h, struct cam_path *path, u_int32_t priority); ==== //depot/projects/scottl-camlock/src/sys/cam/cam_xpt_internal.h#12 (text+ko) ==== @@ -39,7 +39,6 @@ lun_id_t lun_id); typedef void (*xpt_release_device_func)(struct cam_ed *device); typedef void (*xpt_action_func)(union ccb *start_ccb); -typedef void (*xpt_done_func)(union ccb *done_ccb); typedef void (*xpt_dev_async_func)(u_int32_t async_code, struct cam_eb *bus, struct cam_et *target, @@ -52,7 +51,6 @@ xpt_alloc_device_func alloc_device; xpt_release_device_func reldev; xpt_action_func action; - xpt_done_func done; xpt_dev_async_func async; xpt_announce_periph_func announce; }; ==== //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.c#72 (text+ko) ==== @@ -1275,10 +1275,8 @@ ATA_INL(ch->r_mem, AHCI_P_IS), ATA_INL(ch->r_mem, AHCI_P_CI), ATA_INL(ch->r_mem, AHCI_P_SACT), ch->rslots, ATA_INL(ch->r_mem, AHCI_P_TFD), ATA_INL(ch->r_mem, AHCI_P_SERR)); - /* Kick controller into sane state. */ - ahci_stop(ch->dev); - ahci_start(ch->dev); + ch->fatalerr = 1; /* Handle frozen command. */ if (ch->frozen) { union ccb *fccb = ch->frozen; @@ -1360,6 +1358,7 @@ ccb->csio.scsi_status = SCSI_STATUS_OK; break; case AHCI_ERR_INVALID: + ch->fatalerr = 1; ccb->ccb_h.status |= CAM_REQ_INVALID; break; case AHCI_ERR_INNOCENT: @@ -1375,6 +1374,7 @@ } break; case AHCI_ERR_SATA: + ch->fatalerr = 1; if (!ch->readlog) { xpt_freeze_simq(ch->sim, 1); ccb->ccb_h.status &= ~CAM_STATUS_MASK; @@ -1383,6 +1383,7 @@ ccb->ccb_h.status |= CAM_UNCOR_PARITY; break; case AHCI_ERR_TIMEOUT: + ch->fatalerr = 1; if (!ch->readlog) { xpt_freeze_simq(ch->sim, 1); ccb->ccb_h.status &= ~CAM_STATUS_MASK; @@ -1391,6 +1392,7 @@ ccb->ccb_h.status |= CAM_CMD_TIMEOUT; break; default: + ch->fatalerr = 1; ccb->ccb_h.status |= CAM_REQ_CMP_ERR; } /* Free slot. */ @@ -1414,12 +1416,13 @@ ahci_begin_transaction(dev, ccb); return; } + /* If it was our READ LOG command - process it. */ + if (ch->readlog) { + ahci_process_read_log(dev, ccb); /* If it was NCQ command error, put result on hold. */ - if (et == AHCI_ERR_NCQ) { + } else if (et == AHCI_ERR_NCQ) { ch->hold[slot->slot] = ccb; - } else if (ch->readlog) /* If it was our READ LOG command - process it. */ - ahci_process_read_log(dev, ccb); - else + } else xpt_done(ccb); /* Unfreeze frozen command. */ if (ch->frozen && ch->numrslots == 0) { @@ -1428,6 +1431,13 @@ ahci_begin_transaction(dev, fccb); xpt_release_simq(ch->sim, TRUE); } + /* If we have no other active commands, ... */ + if (ch->rslots == 0) { + /* if there was fatal error - reset port. */ + if (ch->fatalerr) { + ahci_reset(dev); + } + } /* Start PM timer. */ if (ch->numrslots == 0 && ch->pm_level > 3) { callout_schedule(&ch->pm_timer, @@ -1674,6 +1684,13 @@ /* XXX; Commands in loading state. */ ahci_end_transaction(&ch->slot[i], AHCI_ERR_INNOCENT); } + for (i = 0; i < ch->numslots; i++) { + if (!ch->hold[i]) + continue; + xpt_done(ch->hold[i]); + ch->hold[i] = NULL; + } + ch->fatalerr = 0; /* Tell the XPT about the event */ xpt_async(AC_BUS_RESET, ch->path, NULL); /* Disable port interrupts */ ==== //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.h#25 (text+ko) ==== @@ -366,6 +366,7 @@ int numrslots; /* Number of running slots */ int numtslots; /* Number of tagged slots */ int readlog; /* Our READ LOG active */ + int fatalerr; /* Fatal error happend */ int lastslot; /* Last used slot */ int taggedtarget; /* Last tagged target */ union ccb *frozen; /* Frozen command */ ==== //depot/projects/scottl-camlock/src/sys/dev/ata/ata-all.c#34 (text+ko) ==== @@ -1302,12 +1302,14 @@ { struct ata_channel *ch = device_get_softc(dev); union ccb *ccb = request->ccb; + int fatalerr = 0; ccb->ccb_h.status &= ~CAM_STATUS_MASK; 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; + fatalerr = 1; } else if (request->status & ATA_S_ERROR) { if (ccb->ccb_h.func_code == XPT_ATA_IO) { ccb->ccb_h.status |= CAM_ATA_STATUS_ERROR; @@ -1344,6 +1346,9 @@ } ata_free_request(request); xpt_done(ccb); + /* Do error recovery if needed. */ + if (fatalerr) + ata_reinit(dev); } static void ==== //depot/projects/scottl-camlock/src/sys/dev/siis/siis.c#12 (text+ko) ==== @@ -767,7 +767,7 @@ estatus == SIIS_P_CMDERR_DATAFIS) { tslots = ch->numtslots[port]; for (i = 0; i < SIIS_MAX_SLOTS; i++) { - /* XXX: reqests in loading state. */ + /* XXX: requests in loading state. */ if (((ch->rslots >> i) & 1) == 0) continue; if (ch->slot[i].ccb->ccb_h.target_id != port) @@ -799,7 +799,7 @@ } else et = SIIS_ERR_INVALID; for (i = 0; i < SIIS_MAX_SLOTS; i++) { - /* XXX: reqests in loading state. */ + /* XXX: requests in loading state. */ if (((ch->rslots >> i) & 1) == 0) continue; siis_end_transaction(&ch->slot[i], et); @@ -970,13 +970,33 @@ return; } +/* Must be called with channel locked. */ +static void +siis_process_timeout(device_t dev) +{ + struct siis_channel *ch = device_get_softc(dev); + int i; + + mtx_assert(&ch->mtx, MA_OWNED); + if (!ch->readlog && !ch->recovery) { + xpt_freeze_simq(ch->sim, ch->numrslots); + ch->recovery = 1; + } + /* Handle the rest of commands. */ + for (i = 0; i < SIIS_MAX_SLOTS; i++) { + /* Do we have a running request on slot? */ + if (ch->slot[i].state < SIIS_SLOT_RUNNING) + continue; + siis_end_transaction(&ch->slot[i], SIIS_ERR_TIMEOUT); + } +} + /* Locked by callout mechanism. */ static void siis_timeout(struct siis_slot *slot) { device_t dev = slot->dev; struct siis_channel *ch = device_get_softc(dev); - int i; mtx_assert(&ch->mtx, MA_OWNED); device_printf(dev, "Timeout on slot %d\n", slot->slot); @@ -984,32 +1004,15 @@ __func__, ATA_INL(ch->r_mem, SIIS_P_IS), ATA_INL(ch->r_mem, SIIS_P_SS), ch->rslots, ATA_INL(ch->r_mem, SIIS_P_CMDERR), ATA_INL(ch->r_mem, SIIS_P_STS), ATA_INL(ch->r_mem, SIIS_P_SERR)); - /* Kick controller into sane state. */ - siis_portinit(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_STATUS_MASK; - 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. */ - siis_end_transaction(&ch->slot[slot->slot], SIIS_ERR_TIMEOUT); - /* Handle the rest of commands. */ - for (i = 0; i < SIIS_MAX_SLOTS; i++) { - /* Do we have a running request on slot? */ - if (ch->slot[i].state < SIIS_SLOT_RUNNING) - continue; - siis_end_transaction(&ch->slot[i], SIIS_ERR_INNOCENT); - } + if (ch->toslots == 0) + xpt_freeze_simq(ch->sim, 1); + ch->toslots |= (1 << slot->slot); + if ((ch->rslots & ~ch->toslots) == 0) + siis_process_timeout(dev); + else + device_printf(dev, " ... waiting for slots %08x\n", + ch->rslots & ~ch->toslots); } /* Must be called with channel locked. */ @@ -1074,6 +1077,7 @@ ccb->csio.scsi_status = SCSI_STATUS_OK; break; case SIIS_ERR_INVALID: + ch->fatalerr = 1; ccb->ccb_h.status |= CAM_REQ_INVALID; break; case SIIS_ERR_INNOCENT: @@ -1089,9 +1093,11 @@ } break; case SIIS_ERR_SATA: + ch->fatalerr = 1; ccb->ccb_h.status |= CAM_UNCOR_PARITY; break; case SIIS_ERR_TIMEOUT: + ch->fatalerr = 1; ccb->ccb_h.status |= CAM_CMD_TIMEOUT; break; default: @@ -1100,6 +1106,11 @@ /* Free slot. */ ch->rslots &= ~(1 << slot->slot); ch->aslots &= ~(1 << slot->slot); + if (et != SIIS_ERR_TIMEOUT) { + if (ch->toslots == (1 << slot->slot)) + xpt_release_simq(ch->sim, TRUE); + ch->toslots &= ~(1 << slot->slot); + } slot->state = SIIS_SLOT_EMPTY; slot->ccb = NULL; /* Update channel stats. */ @@ -1108,13 +1119,14 @@ (ccb->ataio.cmd.flags & CAM_ATAIO_FPDMA)) { ch->numtslots[ccb->ccb_h.target_id]--; } + /* If it was our READ LOG command - process it. */ + if (ch->readlog) { + siis_process_read_log(dev, ccb); /* If it was NCQ command error, put result on hold. */ - if (et == SIIS_ERR_NCQ) { + } else if (et == SIIS_ERR_NCQ) { ch->hold[slot->slot] = ccb; ch->numhslots++; - } else if (ch->readlog) /* If it was our READ LOG command - process it. */ - siis_process_read_log(dev, ccb); - else + } else xpt_done(ccb); /* Unfreeze frozen command. */ if (ch->frozen && ch->numrslots == 0) { @@ -1125,13 +1137,20 @@ } /* If we have no other active commands, ... */ if (ch->rslots == 0) { - /* if we have slots in error, we can reinit port. */ - if (ch->eslots != 0) - siis_portinit(dev); - /* if there commands on hold, we can do READ LOG. */ - if (!ch->readlog && ch->numhslots) - siis_issue_read_log(dev); - } + /* if there were timeouts or fatal error - reset port. */ + if (ch->toslots != 0 || ch->fatalerr) { + siis_reset(dev); + } else { + /* if we have slots in error, we can reinit port. */ + if (ch->eslots != 0) + siis_portinit(dev); + /* if there commands on hold, we can do READ LOG. */ + if (!ch->readlog && ch->numhslots) + siis_issue_read_log(dev); + } + /* If all the reset of commands are in timeout - abort them. */ + } else if ((ch->rslots & ~ch->toslots) == 0) + siis_process_timeout(dev); } static void @@ -1304,8 +1323,9 @@ if (bootverbose) device_printf(dev, "SIIS reset...\n"); - xpt_freeze_simq(ch->sim, ch->numrslots); - /* Requeue freezed command. */ + if (!ch->readlog && !ch->recovery) + xpt_freeze_simq(ch->sim, ch->numrslots); + /* Requeue frozen command. */ if (ch->frozen) { union ccb *fccb = ch->frozen; ch->frozen = NULL; @@ -1325,6 +1345,20 @@ /* XXX; Commands in loading state. */ siis_end_transaction(&ch->slot[i], SIIS_ERR_INNOCENT); } + /* Finish all holden commands as-is. */ + for (i = 0; i < SIIS_MAX_SLOTS; i++) { + if (!ch->hold[i]) + continue; + xpt_done(ch->hold[i]); + ch->hold[i] = NULL; + ch->numhslots--; + } + if (ch->toslots != 0) + xpt_release_simq(ch->sim, TRUE); + ch->eslots = 0; + ch->recovery = 0; + ch->toslots = 0; + ch->fatalerr = 0; /* Disable port interrupts */ ATA_OUTL(ch->r_mem, SIIS_P_IECLR, 0x0000FFFF); /* Set speed limit. */ ==== //depot/projects/scottl-camlock/src/sys/dev/siis/siis.h#2 (text+ko) ==== @@ -373,13 +373,14 @@ uint32_t rslots; /* Running slots */ uint32_t aslots; /* Slots with atomic commands */ uint32_t eslots; /* Slots in error */ + uint32_t toslots; /* Slots in timeout */ int numrslots; /* Number of running slots */ int numtslots[SIIS_MAX_SLOTS]; /* Number of tagged slots */ int numhslots; /* Number of holden slots */ int readlog; /* Our READ LOG active */ + int fatalerr; /* Fatal error happend */ int recovery; /* Some slots are in error */ int lastslot; /* Last used slot */ - int taggedtarget; /* Last tagged target */ union ccb *frozen; /* Frozen command */ };
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200911012337.nA1NbGSc034715>