Date: Wed, 10 Mar 2010 07:40:25 -0800 From: Matthew Jacob <mj@feral.com> To: freebsd-scsi@freebsd.org Subject: Re: pass(4): always retry CAM_REQUEUE_REQ ccbs Message-ID: <4B97BD69.1040504@feral.com> In-Reply-To: <4B978175.3030805@icyb.net.ua> References: <4B978175.3030805@icyb.net.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
I think it's a good idea, but returning ERESTART directly would be a more seamless change. > 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? > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4B97BD69.1040504>