Why shoud we cause panic in scsi_da.c?
Kohji Okuno
okuno.kohji at jp.panasonic.com
Tue Jul 14 01:45:26 UTC 2015
Hi,
From: Alexander Motin <mav at 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
More information about the freebsd-current
mailing list