diff mbox series

[5/8] scsi: ufs: Try harder to change the power mode

Message ID 20220923201138.2113123-6-bvanassche@acm.org (mailing list archive)
State Superseded
Headers show
Series Fix a deadlock in the UFS driver | expand

Commit Message

Bart Van Assche Sept. 23, 2022, 8:11 p.m. UTC
Instead of only retrying the START STOP UNIT command if a unit attention
is reported, repeat it if any SCSI error is reported by the device or if
the command timed out.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Avri Altman Sept. 27, 2022, 10:41 a.m. UTC | #1
> Instead of only retrying the START STOP UNIT command if a unit attention
> is reported, repeat it if any SCSI error is reported by the device or if
> the command timed out.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/ufs/core/ufshcd.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 02e73208b921..e8c0504e9e83 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8784,9 +8784,9 @@ static int ufshcd_set_dev_pwr_mode(struct
> ufs_hba *hba,
>         for (retries = 3; retries > 0; --retries) {
>                 ret = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr,
>                                 START_STOP_TIMEOUT, 0, 0, RQF_PM, NULL);
> -               if (!scsi_status_is_check_condition(ret) ||
> -                               !scsi_sense_valid(&sshdr) ||
> -                               sshdr.sense_key != UNIT_ATTENTION)
> +               if (ret < 0)
> +                       break;
continue?

Thanks,
Avri
> +               if (host_byte(ret) == 0 && scsi_status_is_good(ret))
>                         break;
>         }
>         if (ret) {
Bart Van Assche Sept. 27, 2022, 4:44 p.m. UTC | #2
On 9/27/22 03:41, Avri Altman wrote:
>> Instead of only retrying the START STOP UNIT command if a unit attention
>> is reported, repeat it if any SCSI error is reported by the device or if
>> the command timed out.
>>
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>> ---
>>   drivers/ufs/core/ufshcd.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 02e73208b921..e8c0504e9e83 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -8784,9 +8784,9 @@ static int ufshcd_set_dev_pwr_mode(struct
>> ufs_hba *hba,
>>          for (retries = 3; retries > 0; --retries) {
>>                  ret = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr,
>>                                  START_STOP_TIMEOUT, 0, 0, RQF_PM, NULL);
>> -               if (!scsi_status_is_check_condition(ret) ||
>> -                               !scsi_sense_valid(&sshdr) ||
>> -                               sshdr.sense_key != UNIT_ATTENTION)
>> +               if (ret < 0)
>> +                       break;
>
> continue?

Hi Avri,

Thanks for having taken a look. I chose "break" on purpose since the 
only case for which I expect scsi_execute() to return a negative value 
is request queue shutdown. If the request queue is being shutdown I 
think we should break out of the loop.

Thanks,

Bart.
diff mbox series

Patch

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 02e73208b921..e8c0504e9e83 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8784,9 +8784,9 @@  static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
 	for (retries = 3; retries > 0; --retries) {
 		ret = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr,
 				START_STOP_TIMEOUT, 0, 0, RQF_PM, NULL);
-		if (!scsi_status_is_check_condition(ret) ||
-				!scsi_sense_valid(&sshdr) ||
-				sshdr.sense_key != UNIT_ATTENTION)
+		if (ret < 0)
+			break;
+		if (host_byte(ret) == 0 && scsi_status_is_good(ret))
 			break;
 	}
 	if (ret) {