From owner-freebsd-current@freebsd.org Fri Jul 17 04:46:53 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 6EC1D9A2D6D for ; Fri, 17 Jul 2015 04:46:53 +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 0608E10CA; Fri, 17 Jul 2015 04:46:52 +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-maile11) with ESMTP id t6H4kjON006160; Fri, 17 Jul 2015 13:46:45 +0900 (JST) Received: from epochmail.jp.panasonic.com ([157.8.1.130]) by mail.jp.panasonic.com (8.11.6p2/3.7W/kc-maili11) with ESMTP id t6H4kke09217; Fri, 17 Jul 2015 13:46:46 +0900 Received: by epochmail.jp.panasonic.com (8.12.11.20060308/3.7W/lomi17) id t6H4kkDY002202; Fri, 17 Jul 2015 13:46:46 +0900 Received: from localhost by lomi17.jp.panasonic.com (8.12.11.20060308/3.7W) with ESMTP id t6H4kjrl002182; Fri, 17 Jul 2015 13:46:45 +0900 Date: Fri, 17 Jul 2015 13:46:44 +0900 (JST) Message-Id: <20150717.134644.1446845623556333449.okuno.kohji@jp.panasonic.com> To: ken@FreeBSD.ORG Cc: mav@FreeBSD.org, freebsd-current@freebsd.org, okuno.kohji@jp.panasonic.com Subject: Re: Why shoud we cause panic in scsi_da.c? From: Kohji Okuno In-Reply-To: <20150714194929.GA51157@doriath.kdm.org> References: <20150713.175143.290106286605820529.okuno.kohji@jp.panasonic.com> <55A3D960.5000704@FreeBSD.org> <20150714194929.GA51157@doriath.kdm.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: Fri, 17 Jul 2015 04:46:53 -0000 Subject: Re: Why shoud we cause panic in scsi_da.c? Date: Tue, 14 Jul 2015 15:49:29 -0400 > On Mon, Jul 13, 2015 at 18:29:36 +0300, Alexander Motin wrote: >> 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. > > I put it there because it indicates a software error. The queue shouldn't > be frozen if the command is successful. The reason for freezing the queue > is to allow error recovery to happen. The queue will get unfrozen after > error recovery completes. > > We could alternately just print a diagnostic message, unfreeze the queue > and move on, but the idea is to allow the driver writer to detect and > correct his error immediately. > > As for the original poster's problem, he has uncovered a bug that needs to > be fixed. (And I don't mean in the da(4) driver. The bug is in the > component that left the queue frozen. Most likely in the USB driver, but > it will take a little more investigation.) The panic worked as intended. :) I don't know the reaseon. When I accessed the specified sector on the specified HDD, I encounter the panic. But, I can access other sectors on the same HDD. And, I can access all sectors on the other HDD (same model). Even if Ken's logic is correct, I think that we should do panic in da(4) driver. Best regards, Kohji Okuno