Re: Why shoud we cause panic in scsi_da.c?

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_at_ 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?


_at__at_ -3016,8 +3016,17 _at__at_ 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