Re: [PATCH] scsi: ufs: Improve SCSI abort handling

[Bart Van Assche <bvanassche@acm.org>]

On 11/4/21 4:39 PM, Daejun Park wrote:
>> On 11/4/21 3:39 PM, Daejun Park wrote:
>>> I found similar code in the ufshcd_err_handler(). I think the following
>>> patch will required to fix another warning.
>>>
>>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>>> index f5ba8f953b87..cce9abc6b879 100644
>>> --- a/drivers/scsi/ufs/ufshcd.c
>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>> @@ -6190,6 +6190,7 @@ static void ufshcd_err_handler(struct work_struct *work)
>>>                   }
>>>                   dev_err(hba->dev, "Aborted tag %d / CDB %#02x\n", tag,
>>>                           hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1);
>>> +               hba->lrb[tag].cmd = NULL;
>>>           }
>>>
>>>           /* Clear pending task management requests */
>>
>> Hmm ... since the error handler calls ufshcd_complete_requests(),
>> shouldn't the completion function clear the 'cmd' member? I'm concerned
>> that the above change would break the completion handler.
> 
> I missed that the error handler calls ufshcd_complete_requests(). Please
> ignore my suggestion.
> 
> By the way, I give my reviewed-by tag.
> 
> Reviewed-by: Daejun Park <daejun7.park@samsung.com>

Thanks Daejun! However, your question made me wonder whether ufshcd_abort()
should clear the 'tag' bit from hba->outstanding_reqs. Although the SCSI
standard requires that a command that is aborted is not completed, the UFSHCI
specification requires that writing into the UTRLCLR register clears the
corresponding bit(s) in the UTRLDBR register. I think bit 'tag' will have to
be cleared from hba->outstanding_reqs to prevent that the aborted request is
completed while the SCSI core is resubmitting it.

Thanks,

Bart.