Date: Wed, 10 Mar 2010 13:24:37 +0200 From: Andriy Gapon <avg@icyb.net.ua> To: freebsd-scsi@freebsd.org Subject: pass(4): always retry CAM_REQUEUE_REQ ccbs Message-ID: <4B978175.3030805@icyb.net.ua>
next in thread | raw e-mail | index | archive | help
At present pass(4) never retries a request (or performs any other kind of error recovery) unless the request has CAM_PASS_ERR_RECOVER flag set. This gives applications a control over error checking and handling. But I think that in the case of CAM_REQUEUE_REQ status error recovery, specifically a request retry, should always be performed. Rationale: CAM_REQUEUE_REQ is really an internal flag/state in CAM subsystem. It doesn't convey any information about state of medium, device, bus, controller or the ccb itself. Application can not make any meaningful differentiation based on this status. It either should always retry a ccb with this status or face truly random failures. As such, I haven't encountered so far any application that expects to get CAM_REQUEUE_REQ status. So I think that CAM_REQUEUE_REQ should be contained within the CAM and never be leaked to applications unless retry count is exhausted (to prevent endless loops in case of programming errors, primarily). What do you think? Do you see any reason not to do it? Here's a small patch that implements the behavior: --- a/sys/cam/scsi/scsi_pass.c +++ b/sys/cam/scsi/scsi_pass.c @@ -564,9 +564,8 @@ passsendccb * error recovery. */ cam_periph_runccb(ccb, - (ccb->ccb_h.flags & CAM_PASS_ERR_RECOVER) ? passerror : NULL, - /* cam_flags */ CAM_RETRY_SELTO, /* sense_flags */SF_RETRY_UA, - softc->device_stats); + passerror, /* cam_flags */ CAM_RETRY_SELTO, + /* sense_flags */SF_RETRY_UA, softc->device_stats); if (need_unmap != 0) cam_periph_unmapmem(ccb, &mapinfo); @@ -586,7 +585,10 @@ passerror periph = xpt_path_periph(ccb->ccb_h.path); softc = (struct pass_softc *)periph->softc; - - return(cam_periph_error(ccb, cam_flags, sense_flags, - &softc->saved_ccb)); + + if ((ccb->ccb_h.flags & CAM_PASS_ERR_RECOVER) != 0 || + (ccb->ccb_h.status & CAM_STATUS_MASK) == CAM_REQUEUE_REQ) + return(cam_periph_error(ccb, cam_flags, sense_flags, + &softc->saved_ccb)); + return(0); } I am not sure, perhaps I could just explicitly return ERESTART in the case of CAM_REQUEUE_REQ instead of going through cam_periph_error? -- Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4B978175.3030805>