Re: [PATCH v4 09/10] scsi: ufs: Introduce the function ufshcd_execute_start_stop()

Bart Van Assche
2022-10-19T21:13:23Z

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@acm.org>
>> ---
>> 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.

Thanks,

Bart.