Re: [PATCH V3 1/3] scsi: ufs: Fix error handler clear ua deadlock

[Adrian Hunter <adrian.hunter@intel.com>]

On 15/09/21 1:28 am, Bart Van Assche wrote:
> On 9/13/21 9:55 PM, Adrian Hunter wrote:
>> On 13/09/21 11:11 pm, Bart Van Assche wrote: >> What am I missing?
>>
>> You have not responded to the issues raised by
>> "scsi: ufs: Synchronize SCSI and UFS error handling"
> 
> Because one of the follow-up messages to that patch was so cryptic that I
> did not comprehend it. Anyway, based on the patch at the start of this email
> thread I assume that the deadlock is caused by calling blk_get_request()
> without the BLK_MQ_REQ_NOWAIT flag from inside a SCSI error handler. How
> about fixing this by removing the code that submits a REQUEST SENSE command
> and calling scsi_report_bus_reset() or scsi_report_device_reset() instead?
> ufshcd_reset_and_restore() already uses that approach to make sure that the
> unit attention condition triggered by a reset is not reported to the SCSI
> command submitter. I think only if needs_restore == true and
> needs_reset == false that ufshcd_err_handler() can trigger a UA condition
> without calling scsi_report_bus_reset().
> 
> The following code from scsi_error.c makes sure that the UA after a reset
> does not reach the upper-level driver:
> 
>     case NOT_READY:
>     case UNIT_ATTENTION:
>         /*
>          * if we are expecting a cc/ua because of a bus reset that we
>          * performed, treat this just as a retry.  otherwise this is
>          * information that we should pass up to the upper-level driver
>          * so that we can deal with it there.
>          */
>         if (scmd->device->expecting_cc_ua) {
>             /*
>              * Because some device does not queue unit
>              * attentions correctly, we carefully check
>              * additional sense code and qualifier so as
>              * not to squash media change unit attention.
>              */
>             if (sshdr.asc != 0x28 || sshdr.ascq != 0x00) {
>                 scmd->device->expecting_cc_ua = 0;
>                 return NEEDS_RETRY;
>             }
>         }
> 
> Bart.


Thanks for the idea.  Unfortunately it does not work for pass-through
requests, refer scsi_noretry_cmd().  sdev_ufs_device and sdev_rpmb are
used with pass-through requests.