Re: [PATCH 08/11] scsi: ufs: Improve SCSI abort handling further

Adrian Hunter
2021-11-17T07:37:15Z

On 16/11/2021 23:53, Bart Van Assche wrote:
> On 11/16/21 12:16, Adrian Hunter wrote:
>> On 16/11/2021 18:08, Bart Van Assche wrote:
>>> On 11/16/21 1:07 AM, Peter Wang wrote:
>>>> Should we add unmap?
>>>
>>> Hi Peter,
>>>
>>> I will add DMA unmapping code in the abort handler.
>>
>> I would note that __ufshcd_transfer_req_compl() does that, as well as providing
>> a matching ufshcd_release() for the ufshcd_hold() in ufshcd_queuecommand(), so
>> do consider __ufshcd_transfer_req_compl().
>>
>> Using __ufshcd_transfer_req_compl() seems consistent with the error handler which
>> uses __ufshcd_transfer_req_compl() via ufshcd_complete_requests(), which will pick
>> up all the requests that the error handler has just aborted via
>> ufshcd_try_to_abort_task().  Also ufshcd_host_reset_and_restore() uses
>> __ufshcd_transfer_req_compl() via ufshcd_complete_requests(), which will pick
>> up anything still in outstanding_reqs because the doorbell has become zero at
>> that point.
>
> Hi Adrian,
>
> Although I agree with minimizing code duplication, I'm not sure that __ufshcd_transfer_req_compl() should be called from inside ufshcd_abort(). __ufshcd_transfer_req_compl() also calls ufshcd_update_monitor() and ufshcd_add_command_trace(hba, index, UFS_CMD_COMP). Neither function should be called when aborting a command.

But we should trace the abort I would have thought - seems like a separate issue though.