LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] scsi: ufs: Allow async suspend/resume callbacks
@ 2021-07-28  1:27 Vincent Palomares
  2021-07-28 17:57 ` Bart Van Assche
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Vincent Palomares @ 2021-07-28  1:27 UTC (permalink / raw)
  To: linux-kernel, linux-scsi
  Cc: Vincent Palomares, Bjorn Helgaas, Jaegeuk Kim, Bart Van Assche,
	Adrian Hunter, Stanley Chu, Can Guo, Asutosh Das, Avri Altman,
	Martin K . Petersen

Allow UFS suspend/resume callbacks to run in parallel with other
suspend/resume callbacks. This can recoup dozens of milliseconds on the
resume path if UFS hardware needs to be powered back on.

Suspending and resuming asynchronously is safe to do so long as the driver
callbacks only depend on resources made available by either a) parent
devices or b) devices explicitly marked as suppliers with device_link_add.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Bart Van Assche <bvanassche@acm.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Cc: Avri Altman <avri.altman@wdc.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Vincent Palomares <paillon@google.com>
---

Are there any suspend/resume dependencies for UFS drivers not tracked by
the device parent relationship?

drivers/scsi/ufs/ufshcd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b87ff68aa9aa..9ec5c308a0ea 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -9625,6 +9625,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	async_schedule(ufshcd_async_scan, hba);
 	ufs_sysfs_add_nodes(hba->dev);
 
+	device_enable_async_suspend(dev);
 	return 0;
 
 free_tmf_queue:
-- 
2.32.0.432.gabb21c7263-goog


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: ufs: Allow async suspend/resume callbacks
  2021-07-28  1:27 [PATCH] scsi: ufs: Allow async suspend/resume callbacks Vincent Palomares
@ 2021-07-28 17:57 ` Bart Van Assche
  2021-07-29  6:48 ` Avri Altman
  2021-07-31  3:48 ` Martin K. Petersen
  2 siblings, 0 replies; 6+ messages in thread
From: Bart Van Assche @ 2021-07-28 17:57 UTC (permalink / raw)
  To: Vincent Palomares, linux-kernel, linux-scsi
  Cc: Bjorn Helgaas, Jaegeuk Kim, Adrian Hunter, Stanley Chu, Can Guo,
	Asutosh Das, Avri Altman, Martin K . Petersen

On 7/27/21 6:27 PM, Vincent Palomares wrote:
> Are there any suspend/resume dependencies for UFS drivers not tracked by
> the device parent relationship?

Not that I know of. Since I like this patch:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] scsi: ufs: Allow async suspend/resume callbacks
  2021-07-28  1:27 [PATCH] scsi: ufs: Allow async suspend/resume callbacks Vincent Palomares
  2021-07-28 17:57 ` Bart Van Assche
@ 2021-07-29  6:48 ` Avri Altman
  2021-07-30 16:05   ` Bart Van Assche
  2021-07-31  3:48 ` Martin K. Petersen
  2 siblings, 1 reply; 6+ messages in thread
From: Avri Altman @ 2021-07-29  6:48 UTC (permalink / raw)
  To: Vincent Palomares, linux-kernel, linux-scsi
  Cc: Bjorn Helgaas, Jaegeuk Kim, Bart Van Assche, Adrian Hunter,
	Stanley Chu, Can Guo, Asutosh Das, Martin K . Petersen

 
> Allow UFS suspend/resume callbacks to run in parallel with other
> suspend/resume callbacks. This can recoup dozens of milliseconds on the
> resume path if UFS hardware needs to be powered back on.
> 
> Suspending and resuming asynchronously is safe to do so long as the driver
> callbacks only depend on resources made available by either a) parent
> devices or b) devices explicitly marked as suppliers with device_link_add.
> 
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Cc: Bart Van Assche <bvanassche@acm.org>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Stanley Chu <stanley.chu@mediatek.com>
> Cc: Can Guo <cang@codeaurora.org>
> Cc: Asutosh Das <asutoshd@codeaurora.org>
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: Vincent Palomares <paillon@google.com>
> ---
> 
> Are there any suspend/resume dependencies for UFS drivers not tracked by
> the device parent relationship?
> 
> drivers/scsi/ufs/ufshcd.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b87ff68aa9aa..9ec5c308a0ea 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -9625,6 +9625,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem
> *mmio_base, unsigned int irq)
>         async_schedule(ufshcd_async_scan, hba);
>         ufs_sysfs_add_nodes(hba->dev);
> 
> +       device_enable_async_suspend(dev);
>         return 0;
Isn't device_enable_async_suspend is being called for each lun in scsi_sysfs_add_sdev Anyway?

Thanks,
Avri
> 
>  free_tmf_queue:
> --
> 2.32.0.432.gabb21c7263-goog


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: ufs: Allow async suspend/resume callbacks
  2021-07-29  6:48 ` Avri Altman
@ 2021-07-30 16:05   ` Bart Van Assche
  2021-07-30 18:17     ` Avri Altman
  0 siblings, 1 reply; 6+ messages in thread
From: Bart Van Assche @ 2021-07-30 16:05 UTC (permalink / raw)
  To: Avri Altman, Vincent Palomares, linux-kernel, linux-scsi
  Cc: Bjorn Helgaas, Jaegeuk Kim, Adrian Hunter, Stanley Chu, Can Guo,
	Asutosh Das, Martin K . Petersen

On 7/28/21 11:48 PM, Avri Altman wrote:
> Vincent wrote:
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index b87ff68aa9aa..9ec5c308a0ea 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -9625,6 +9625,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem
>> *mmio_base, unsigned int irq)
>>          async_schedule(ufshcd_async_scan, hba);
>>          ufs_sysfs_add_nodes(hba->dev);
>>
>> +       device_enable_async_suspend(dev);
>>          return 0;
> Isn't device_enable_async_suspend is being called for each lun in scsi_sysfs_add_sdev Anyway?

Hi Avri,

Our measurements have shown that resume takes longer than it should with 
encryption enabled. While suspending we change the power mode of the UFS 
device to a mode in which it loses crypto keys. Restoring crypto keys 
during resume (blk_ksm_reprogram_all_keys()) takes about 31 ms. This is 
the long pole and takes much more time than resuming LUNs. This patch 
makes UFS resume happen concurrently with resuming other devices in the 
system instead of serializing it. Measurements have shown that this 
patch significantly improves the time needed to resume an Android device.

Bart.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] scsi: ufs: Allow async suspend/resume callbacks
  2021-07-30 16:05   ` Bart Van Assche
@ 2021-07-30 18:17     ` Avri Altman
  0 siblings, 0 replies; 6+ messages in thread
From: Avri Altman @ 2021-07-30 18:17 UTC (permalink / raw)
  To: Bart Van Assche, Vincent Palomares, linux-kernel, linux-scsi
  Cc: Bjorn Helgaas, Jaegeuk Kim, Adrian Hunter, Stanley Chu, Can Guo,
	Asutosh Das, Martin K . Petersen

> On 7/28/21 11:48 PM, Avri Altman wrote:
> > Vincent wrote:
> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> >> index b87ff68aa9aa..9ec5c308a0ea 100644
> >> --- a/drivers/scsi/ufs/ufshcd.c
> >> +++ b/drivers/scsi/ufs/ufshcd.c
> >> @@ -9625,6 +9625,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem
> >> *mmio_base, unsigned int irq)
> >>          async_schedule(ufshcd_async_scan, hba);
> >>          ufs_sysfs_add_nodes(hba->dev);
> >>
> >> +       device_enable_async_suspend(dev);
> >>          return 0;
> > Isn't device_enable_async_suspend is being called for each lun in
> scsi_sysfs_add_sdev Anyway?
> 
> Hi Avri,
> 
> Our measurements have shown that resume takes longer than it should with
> encryption enabled. While suspending we change the power mode of the UFS
> device to a mode in which it loses crypto keys. Restoring crypto keys
> during resume (blk_ksm_reprogram_all_keys()) takes about 31 ms. This is
> the long pole and takes much more time than resuming LUNs. This patch
> makes UFS resume happen concurrently with resuming other devices in the
> system instead of serializing it. Measurements have shown that this
> patch significantly improves the time needed to resume an Android device.
OK.
Thanks for the extra info.

Thanks,
Avri

> 
> Bart.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: ufs: Allow async suspend/resume callbacks
  2021-07-28  1:27 [PATCH] scsi: ufs: Allow async suspend/resume callbacks Vincent Palomares
  2021-07-28 17:57 ` Bart Van Assche
  2021-07-29  6:48 ` Avri Altman
@ 2021-07-31  3:48 ` Martin K. Petersen
  2 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2021-07-31  3:48 UTC (permalink / raw)
  To: Vincent Palomares
  Cc: linux-kernel, linux-scsi, Bjorn Helgaas, Jaegeuk Kim,
	Bart Van Assche, Adrian Hunter, Stanley Chu, Can Guo,
	Asutosh Das, Avri Altman, Martin K . Petersen


Vincent,

> Allow UFS suspend/resume callbacks to run in parallel with other
> suspend/resume callbacks. This can recoup dozens of milliseconds on the
> resume path if UFS hardware needs to be powered back on.

Applied to 5.15/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-07-31  3:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-28  1:27 [PATCH] scsi: ufs: Allow async suspend/resume callbacks Vincent Palomares
2021-07-28 17:57 ` Bart Van Assche
2021-07-29  6:48 ` Avri Altman
2021-07-30 16:05   ` Bart Van Assche
2021-07-30 18:17     ` Avri Altman
2021-07-31  3:48 ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).