From owner-freebsd-current@freebsd.org Tue Jul 14 01:45:26 2015 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B934A998C32 for ; Tue, 14 Jul 2015 01:45:26 +0000 (UTC) (envelope-from okuno.kohji@jp.panasonic.com) Received: from smtp.mei.co.jp (smtp.mei.co.jp [133.183.100.20]) by mx1.freebsd.org (Postfix) with ESMTP id EB72C1D1F; Tue, 14 Jul 2015 01:45:25 +0000 (UTC) (envelope-from okuno.kohji@jp.panasonic.com) Received: from mail-gw.jp.panasonic.com ([157.8.1.157]) by smtp.mei.co.jp (8.12.11.20060614/3.7W/kc-maile13) with ESMTP id t6E1jOgC001015; Tue, 14 Jul 2015 10:45:24 +0900 (JST) Received: from epochmail.jp.panasonic.com ([157.8.1.130]) by mail.jp.panasonic.com (8.11.6p2/3.7W/kc-maili15) with ESMTP id t6E1jNB17013; Tue, 14 Jul 2015 10:45:23 +0900 Received: by epochmail.jp.panasonic.com (8.12.11.20060308/3.7W/lomi17) id t6E1jNaV011580; Tue, 14 Jul 2015 10:45:23 +0900 Received: from localhost by lomi17.jp.panasonic.com (8.12.11.20060308/3.7W) with ESMTP id t6E1jNHZ011557; Tue, 14 Jul 2015 10:45:23 +0900 Date: Tue, 14 Jul 2015 10:45:22 +0900 (JST) Message-Id: <20150714.104522.1727426323279944326.okuno.kohji@jp.panasonic.com> To: mav@FreeBSD.org Cc: freebsd-current@freebsd.org, ken@freebsd.org, okuno.kohji@jp.panasonic.com Subject: Re: Why shoud we cause panic in scsi_da.c? From: Kohji Okuno In-Reply-To: <55A3D960.5000704@FreeBSD.org> References: <55A37933.3000802@selasky.org> <20150713.175143.290106286605820529.okuno.kohji@jp.panasonic.com> <55A3D960.5000704@FreeBSD.org> Organization: Panasonic Corporation X-Mailer: Mew version 6.5 on Emacs 24.3 / Mule 6.0 (HANACHIRUSATO) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Jul 2015 01:45:26 -0000 Hi, From: Alexander Motin Date: Mon, 13 Jul 2015 18:29:36 +0300 > Hi. > > On 13.07.2015 11:51, Kohji Okuno wrote: >>> On 07/13/15 10:11, Kohji Okuno wrote: >>>> Could you comment on my quesion? >>>> >>>>> I found panic() in scsi_da.c. Please find the following. >>>>> I think we should return with error without panic(). >>>>> What do you think about this? >>>>> >>>>> scsi_da.c: >>>>> 3018 } else if (bp != NULL) { >>>>> 3019 if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) >>>>> 3020 panic("REQ_CMP with QFRZN"); >>>>> >>> >>> It looks to me more like an KASSERT() is appropriate here. > > As I can see, this panic() call was added by ken@ about 15 years ago. > I've added him to CC in case he has some idea why it was done. From my > personal opinion I don't see much reasons to allow CAM_DEV_QFRZN to be > returned only together with error. While is may have little sense in > case of successful command completion, I don't think it should be > treated as error. Simply removing this panic is probably a bad idea, > since if it happens device will just remain frozen forever, that will be > will be difficult to diagnose, but I would better just dropped device > freeze in that case same as in case of completion with error. Thank you for your comment. I have a strange USB HDD. When I access the specified sector, the kernel causes panic("REQ_CMP with QFRZN") always. After I modified the following, I think that I can recover from this state, although the specified sector access fails. This recovery means that I can access other sectors after this recovery. What do you think about my idea? @@ -3016,8 +3016,17 @@ dadone(struct cam_periph *periph, union ccb *done_ccb) /*timeout*/0, /*getcount_only*/0); } else if (bp != NULL) { +#if 0 if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) panic("REQ_CMP with QFRZN"); +#else + if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) + cam_release_devq(done_ccb->ccb_h.path, + /*relsim_flags*/0, + /*reduction*/0, + /*timeout*/0, + /*getcount_only*/0); +#endif if (state == DA_CCB_DELETE) bp->bio_resid = 0; else Best regards, Kohji Okuno