| Message ID | 20220923201138.2113123-6-bvanassche@acm.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix a deadlock in the UFS driver | expand |
> 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) {
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 --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) {
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(-)