From owner-freebsd-scsi@FreeBSD.ORG Wed Mar 10 11:35:25 2010 Return-Path: Delivered-To: freebsd-scsi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8C33A106564A for ; Wed, 10 Mar 2010 11:35:25 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id CDFB08FC13 for ; Wed, 10 Mar 2010 11:35:24 +0000 (UTC) Received: from porto.topspin.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id NAA11978 for ; Wed, 10 Mar 2010 13:24:38 +0200 (EET) (envelope-from avg@icyb.net.ua) Received: from localhost.topspin.kiev.ua ([127.0.0.1]) by porto.topspin.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1NpK1l-0007pl-VG for freebsd-scsi@freebsd.org; Wed, 10 Mar 2010 13:24:38 +0200 Message-ID: <4B978175.3030805@icyb.net.ua> Date: Wed, 10 Mar 2010 13:24:37 +0200 From: Andriy Gapon User-Agent: Thunderbird 2.0.0.23 (X11/20100211) MIME-Version: 1.0 To: freebsd-scsi@freebsd.org X-Enigmail-Version: 0.96.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: pass(4): always retry CAM_REQUEUE_REQ ccbs X-BeenThere: freebsd-scsi@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SCSI subsystem List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Mar 2010 11:35:25 -0000 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