Date: Mon, 13 Nov 2006 21:17:25 GMT From: Matt Jacob <mjacob@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 109885 for review Message-ID: <200611132117.kADLHPER099475@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=109885 Change 109885 by mjacob@newisp on 2006/11/13 21:17:03 Clean up interrupt handler to look a bit closer at FC errors to try and trap the 24XX problems I've been seeing after SAN reconnection. So, we now look at FCP Response Codes (finally). We also adjust the sense pointer appropriately for the 24XX because the Sense data is shifted down if RV is set. Redo the command watchdog routine. We've seen some cases where (inexplicably) we're getting a watchdog call on a command who has definitely been completed (and the CCB been freed). Because the watchdog argument is the CCB, and from that we derive the SIM's softc and *then* search the SIM's outstanding command list to see if the command is still outstanding. The problem is that we then can dereference freed memory. So, the watchdog routine now just searches all SIMs. This doesn't really eliminate the actual problem (watchdog call for a command that's done), but at least keeps us from panicing. Affected files ... .. //depot/projects/newisp/dev/isp/isp.c#31 edit .. //depot/projects/newisp/dev/isp/isp_freebsd.c#25 edit .. //depot/projects/newisp/dev/isp/isp_freebsd.h#17 edit .. //depot/projects/newisp/dev/isp/isp_stds.h#7 edit .. //depot/projects/newisp/dev/isp/ispvar.h#15 edit Differences ... ==== //depot/projects/newisp/dev/isp/isp.c#31 (text+ko) ==== @@ -4650,6 +4650,8 @@ isphdr_t *hp; int buddaboom, etype, scsi_status, completion_status; int req_status_flags, req_state_flags; + uint8_t *snsp, *resp; + uint32_t rlen, slen; long resid; uint16_t oop; @@ -4814,13 +4816,37 @@ XS_SETERR(xs, HBA_BOTCH); } + resp = NULL; + rlen = 0; + snsp = NULL; + slen = 0; + if (IS_24XX(isp) && (scsi_status & RQCS_RV) != 0) { + resp = ((isp24xx_statusreq_t *)sp)->req_rsp_sense; + rlen = ((isp24xx_statusreq_t *)sp)->req_response_len; + } else if (IS_FC(isp) && (scsi_status & RQCS_RV) != 0) { + resp = sp->req_response; + rlen = sp->req_response_len; + } if (IS_FC(isp) && (scsi_status & RQCS_SV) != 0) { /* * Fibre Channel F/W doesn't say we got status * if there's Sense Data instead. I guess they * think it goes w/o saying. */ - req_state_flags |= RQSF_GOT_STATUS; + req_state_flags |= RQSF_GOT_STATUS|RQSF_GOT_SENSE; + if (IS_24XX(isp)) { + snsp = + ((isp24xx_statusreq_t *)sp)->req_rsp_sense; + snsp += rlen; + slen = + ((isp24xx_statusreq_t *)sp)->req_sense_len; + } else { + snsp = sp->req_sense_data; + slen = sp->req_sense_len; + } + } else if (IS_SCSI(isp) && (req_state_flags & RQSF_GOT_SENSE)) { + snsp = sp->req_sense_data; + slen = sp->req_sense_len; } if (req_state_flags & RQSF_GOT_STATUS) { *XS_STSP(xs) = scsi_status & 0xff; @@ -4828,8 +4854,13 @@ switch (etype) { case RQSTYPE_RESPONSE: - /* XXX won't work for 24xx */ XS_SET_STATE_STAT(isp, xs, sp); + if (resp) { + isp_prt(isp, ISP_LOGWARN, + "FCP RESPONSE: 0x%x", + resp[FCP_RSPNS_CODE_OFFSET]); + XS_SETERR(xs, HBA_BOTCH); + } if (IS_24XX(isp)) { isp_parse_status_24xx(isp, (isp24xx_statusreq_t *)sp, xs, &resid); @@ -4842,11 +4873,6 @@ } if (IS_SCSI(isp)) { XS_RESID(xs) = resid; - if ((req_state_flags & RQSF_GOT_STATUS) && - (*XS_STSP(xs) == SCSI_CHECK) && - (req_state_flags & RQSF_GOT_SENSE)) { - XS_SAVE_SENSE(xs, sp); - } /* * A new synchronous rate was negotiated for * this target. Mark state such that we'll go @@ -4868,13 +4894,9 @@ } else { XS_RESID(xs) = 0; } - if ((req_state_flags & RQSF_GOT_STATUS) && - (*XS_STSP(xs) == SCSI_CHECK) && - (scsi_status & RQCS_SV)) { - XS_SAVE_SENSE(xs, sp); - /* solely for the benefit of debug */ - req_state_flags |= RQSF_GOT_SENSE; - } + } + if (snsp && slen) { + XS_SAVE_SENSE(xs, snsp, slen); } isp_prt(isp, ISP_LOGDEBUG2, "asked for %ld got raw resid %ld settled for %ld", ==== //depot/projects/newisp/dev/isp/isp_freebsd.c#25 (text+ko) ==== @@ -2038,11 +2038,11 @@ } -static void -isp_watchdog(void *arg) +static int isp_watchdog_work(ispsoftc_t *, XS_T *); + +static int +isp_watchdog_work(ispsoftc_t *isp, XS_T *xs) { - XS_T *xs = arg; - ispsoftc_t *isp = XS_ISP(xs); uint32_t handle; /* @@ -2060,14 +2060,14 @@ isp_prt(isp, ISP_LOGDEBUG1, "watchdog found done cmd (handle 0x%x)", handle); ISP_UNLOCK(isp); - return; + return (1);; } if (XS_CMD_WDOG_P(xs)) { isp_prt(isp, ISP_LOGDEBUG2, "recursive watchdog (handle 0x%x)", handle); ISP_UNLOCK(isp); - return; + return (1); } XS_CMD_S_WDOG(xs); @@ -2085,7 +2085,7 @@ * Make sure the command is *really* dead before we * release the handle (and DMA resources) for reuse. */ - (void) isp_control(isp, ISPCTL_ABORT_CMD, arg); + (void) isp_control(isp, ISPCTL_ABORT_CMD, xs); /* * After this point, the comamnd is really dead. @@ -2108,12 +2108,29 @@ XS_CMD_S_GRACE(xs); isp->isp_sendmarker |= 1 << XS_CHANNEL(xs); } - } else { - isp_prt(isp, ISP_LOGWARN, "watchdog with no command"); + ISP_UNLOCK(isp); + return (1); } ISP_UNLOCK(isp); + return (0); } +static void +isp_watchdog(void *arg) +{ + ispsoftc_t *isp; + XS_T *xs = arg; + for (isp = isplist; isp != NULL; isp = isp->isp_osinfo.next) { + if (isp_watchdog_work(isp, xs)) { + break; + } + } + if (isp == NULL) { + printf("isp_watchdog: nobody had %p active\n", arg); + } +} + + #if __FreeBSD_version >= 500000 #define isp_make_here(isp, tgt) isp_announce(isp, tgt, AC_FOUND_DEVICE) #define isp_make_gone(isp, tgt) isp_announce(isp, tgt, AC_LOST_DEVICE) ==== //depot/projects/newisp/dev/isp/isp_freebsd.h#17 (text+ko) ==== @@ -336,10 +336,9 @@ #define XS_INITERR(ccb) \ XS_SETERR(ccb, CAM_REQ_INPROG), (ccb)->ccb_h.spriv_field0 = 0 -#define XS_SAVE_SENSE(xs, sp) \ - (xs)->ccb_h.status |= CAM_AUTOSNS_VALID, \ - memcpy(&(xs)->sense_data, sp->req_sense_data, \ - imin(XS_SNSLEN(xs), sp->req_sense_len)) +#define XS_SAVE_SENSE(xs, sense_ptr, sense_len) \ + (xs)->ccb_h.status |= CAM_AUTOSNS_VALID; \ + memcpy(&(xs)->sense_data, sense_ptr, imin(XS_SNSLEN(xs), sense_len)) #define XS_SET_STATE_STAT(a, b, c) ==== //depot/projects/newisp/dev/isp/isp_stds.h#7 (text+ko) ==== @@ -129,7 +129,7 @@ /* * RFT_ID Requet CT_IU * - * Source: INCITS xxx-200x Generic Services- 5 Rev 8.5 Section 5.2.5.30 + * Source: NCITS xxx-200x Generic Services- 5 Rev 8.5 Section 5.2.5.30 */ typedef struct { ct_hdr_t rftid_hdr; @@ -138,6 +138,19 @@ uint32_t rftid_fc4types[8]; } rft_id_t; +/* + * FCP Response Code Definitions + * Source: NCITS T10, Project 1144D, Revision 07a (aka FCP2r07a) + */ +#define FCP_RSPNS_CODE_OFFSET 3 + +#define FCP_RSPNS_TMF_DONE 0 +#define FCP_RSPNS_DLBRSTX 1 +#define FCP_RSPNS_BADCMND 2 +#define FCP_RSPNS_EROFS 3 +#define FCP_RSPNS_TMF_REJECT 4 +#define FCP_RSPNS_TMF_FAILED 5 + /* unconverted miscellany */ /* ==== //depot/projects/newisp/dev/isp/ispvar.h#15 (text+ko) ==== @@ -967,7 +967,7 @@ * XS_NOERR(xs) there is no error currently set * XS_INITERR(xs) initialize error state * - * XS_SAVE_SENSE(xs, sp) save sense data + * XS_SAVE_SENSE(xs, sp, len) save sense data * * XS_SET_STATE_STAT(isp, sp, xs) platform dependent interpreter of * response queue entry status bits
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200611132117.kADLHPER099475>