| Message ID | 20220811234401.1957911-1-bvanassche@acm.org |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: ufs: Reduce the power mode change timeout | expand |
On Fri, Aug 12, 2022 at 7:52 AM Bart Van Assche <bvanassche@acm.org> wrote: > > The current power mode change timeout (180 s) is so large that it can > cause a watchdog timer to fire. Reduce the power mode change timeout to > 10 seconds. > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/ufs/core/ufshcd.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
> The current power mode change timeout (180 s) is so large that it can > cause a watchdog timer to fire. Reduce the power mode change timeout to > 10 seconds. Maybe also worth noting that it was invented 20 years ago for scsi ioctl, And is less applicable for nowadays ufs devices > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/ufs/core/ufshcd.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index e32b6b834b7f..2dd355a5da54 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -8776,6 +8776,8 @@ static int ufshcd_set_dev_pwr_mode(struct > ufs_hba *hba, > struct scsi_device *sdp; > unsigned long flags; > int ret, retries; > + unsigned long deadline; > + int32_t remaining; Maybe use ktime api, like its done throughout the driver? Thanks, Avri > > spin_lock_irqsave(hba->host->host_lock, flags); > sdp = hba->ufs_device_wlun; > @@ -8808,9 +8810,14 @@ static int ufshcd_set_dev_pwr_mode(struct > ufs_hba *hba, > * callbacks hence set the RQF_PM flag so that it doesn't resume the > * already suspended childs. > */ > + deadline = jiffies + 10 * HZ; > for (retries = 3; retries > 0; --retries) { > + ret = -ETIMEDOUT; > + remaining = deadline - jiffies; > + if (remaining <= 0) > + break; > ret = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr, > - START_STOP_TIMEOUT, 0, 0, RQF_PM, NULL); > + remaining / HZ, 0, 0, RQF_PM, NULL); > if (!scsi_status_is_check_condition(ret) || > !scsi_sense_valid(&sshdr) || > sshdr.sense_key != UNIT_ATTENTION)
On 8/14/22 00:45, Avri Altman wrote: >> The current power mode change timeout (180 s) is so large that it can >> cause a watchdog timer to fire. Reduce the power mode change timeout to >> 10 seconds. > Maybe also worth noting that it was invented 20 years ago for scsi ioctl, > And is less applicable for nowadays ufs devices > >> >> Signed-off-by: Bart Van Assche <bvanassche@acm.org> >> --- >> drivers/ufs/core/ufshcd.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c >> index e32b6b834b7f..2dd355a5da54 100644 >> --- a/drivers/ufs/core/ufshcd.c >> +++ b/drivers/ufs/core/ufshcd.c >> @@ -8776,6 +8776,8 @@ static int ufshcd_set_dev_pwr_mode(struct >> ufs_hba *hba, >> struct scsi_device *sdp; >> unsigned long flags; >> int ret, retries; >> + unsigned long deadline; >> + int32_t remaining; > > Maybe use ktime api, like its done throughout the driver? Hi Avri, Calling ktime_get() is much more expensive than reading the jiffies counter. That's why I prefer to use the jiffies counter if the timeout is significantly larger than 1 / HZ and if the accuracy of 1 / HZ is sufficient. Thanks, Bart.
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index e32b6b834b7f..2dd355a5da54 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -8776,6 +8776,8 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba, struct scsi_device *sdp; unsigned long flags; int ret, retries; + unsigned long deadline; + int32_t remaining; spin_lock_irqsave(hba->host->host_lock, flags); sdp = hba->ufs_device_wlun; @@ -8808,9 +8810,14 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba, * callbacks hence set the RQF_PM flag so that it doesn't resume the * already suspended childs. */ + deadline = jiffies + 10 * HZ; for (retries = 3; retries > 0; --retries) { + ret = -ETIMEDOUT; + remaining = deadline - jiffies; + if (remaining <= 0) + break; ret = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr, - START_STOP_TIMEOUT, 0, 0, RQF_PM, NULL); + remaining / HZ, 0, 0, RQF_PM, NULL); if (!scsi_status_is_check_condition(ret) || !scsi_sense_valid(&sshdr) || sshdr.sense_key != UNIT_ATTENTION)
The current power mode change timeout (180 s) is so large that it can cause a watchdog timer to fire. Reduce the power mode change timeout to 10 seconds. Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/ufs/core/ufshcd.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)