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

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_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.
> 
> 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