On 10/19/22 4:13 PM, Bart Van Assche wrote:
> On 10/19/22 12:57, Mike Christie wrote:
>> On 10/18/22 3:29 PM, Bart Van Assche wrote:
>>> Open-code scsi_execute() because a later patch will modify scmd->flags
>>> and because scsi_execute() does not support setting scmd->flags. No
>>> functionality is changed.
>>>
>>> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
>>> ---
>>> drivers/ufs/core/ufshcd.c | 39 ++++++++++++++++++++++++++++++++++-----
>>> 1 file changed, 34 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>>> index 2a32bcc93d2e..c5ccc7ba583b 100644
>>> --- a/drivers/ufs/core/ufshcd.c
>>> +++ b/drivers/ufs/core/ufshcd.c
>>> @@ -8729,6 +8729,39 @@ static void ufshcd_hba_exit(struct ufs_hba *hba)
>>> }
>>> }
>>> +static int ufshcd_execute_start_stop(struct scsi_device *sdev,
>>> + enum ufs_dev_pwr_mode pwr_mode,
>>> + struct scsi_sense_hdr *sshdr)
>>> +{
>>> + unsigned char cdb[6] = { START_STOP, 0, 0, 0, pwr_mode << 4, 0 };
>>> + struct request *req;
>>> + struct scsi_cmnd *scmd;
>>> + int ret;
>>> +
>>> + req = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_IN,
>>> + BLK_MQ_REQ_PM);
>>>
>>
>> Can you hit a case where we have run out of tags (__blk_mq_alloc_requests
>> is hitting the blk_mq_get_tag == BLK_MQ_NO_TAG check), the host has gone
>> into recovery and so commands are completing to add a tag back and then we
>> try to call this and get stuck waiting on a tag? Or for passthrough do we
>> have some special reserve?
>>
>> If so do you need to use BLK_MQ_REQ_NOWAIT here? Maybe do the retry loop
>> yourself like:
>>
>> retry:
>> if host is in recovery
>> return failure
>>
>> req = scsi_alloc_request(.... BLK_MQ_REQ_NOWAIT)
>> if (!req and we have not hit some retry limit)
>> goto retry
>>
>>
>> or have some special reserve command/tag.
>
> Hi Mike,
>
> No other SCSI commands should be in progress when ufshcd_execute_start_stop() is called because that function is only called during system suspend and resume and no other I/O should be in progress at that time. Additionally, there is a mutual exclusion mechanism in the UFS driver to serialize system suspend/resume activity and error handling.
Looks ok to me then.
Reviewed-by: Mike Christie <michael.christie@xxxxxxxxxx>
