Date: Tue, 14 Jul 2015 10:45:22 +0900 (JST) From: Kohji Okuno <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? Message-ID: <20150714.104522.1727426323279944326.okuno.kohji@jp.panasonic.com> In-Reply-To: <55A3D960.5000704@FreeBSD.org> References: <55A37933.3000802@selasky.org> <20150713.175143.290106286605820529.okuno.kohji@jp.panasonic.com> <55A3D960.5000704@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi,
From: Alexander Motin <mav@FreeBSD.org>
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150714.104522.1727426323279944326.okuno.kohji>