Re: [PATCH v2 14/20] scsi: ufs: Introduce ufshcd_release_scsi_cmd()

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

On 30/11/2021 20:00, Bart Van Assche wrote:
> On 11/24/21 4:03 AM, Adrian Hunter wrote:
>> On 19/11/2021 21:57, Bart Van Assche wrote:
>>> +/* Release the resources allocated for processing a SCSI command. */
>>> +static void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
>>> +                    struct ufshcd_lrb *lrbp)
>>> +{
>>> +    struct scsi_cmnd *cmd = lrbp->cmd;
>>> +
>>> +    scsi_dma_unmap(cmd);
>>> +    lrbp->cmd = NULL;    /* Mark the command as completed. */
>>> +    ufshcd_release(hba);
>>> +    ufshcd_clk_scaling_update_busy(hba);
>>> +}
>>
>> That seems to leave a gap in the handling of tracing.
>>
>> Wouldn't we be better served to tweak the monitoring code
>> in __ufshcd_transfer_req_compl() and then use
>>   __ufshcd_transfer_req_compl()? i.e.
>>
>>     result = ufshcd_transfer_rsp_status(hba, lrbp);
>>     if (unlikely(!result && ufshcd_should_inform_monitor(hba, lrbp)))
>>         ufshcd_update_monitor(hba, lrbp);
> 
> Which gap are you referring to? 

As in: you are not handling tracing.

ufshcd_abort() only calls
> ufshcd_release_scsi_cmd() after ufshcd_try_to_abort_task() succeeded.
> That means that the command has not completed and hence that
> ufshcd_update_monitor() must not be called.

AFAICT the monitor is for successful commands, which is why I suggested
checking the 'result'.

So make that change to __ufshcd_transfer_req_compl() and then it will
work for ufshcd_abort() and provide tracing.