pass(4): always retry CAM_REQUEUE_REQ ccbs
mj at feral.com
Wed Mar 10 15:40:20 UTC 2010
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.
> 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.
> - (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)
> @@ -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?
More information about the freebsd-scsi