All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v1] scsi: ufs: Quiesce all scsi devices before shutdown
@ 2020-07-02  1:32 ` Stanley Chu
  0 siblings, 0 replies; 9+ messages in thread
From: Stanley Chu @ 2020-07-02  1:32 UTC (permalink / raw)
  To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb, bvanassche
  Cc: beanhuo, asutoshd, cang, matthias.bgg, linux-mediatek,
	linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
	chun-hung.wu, andy.teng, chaotian.jing, cc.chou, Stanley Chu

Currently I/O request could be still submitted to UFS device while
UFS is working on shutdown flow. This may lead to racing as below
scenarios and finally system may crash due to unclocked register
accesses.

To fix this kind of issues, specifically quiesce all SCSI devices
before UFS shutdown to block all I/O request sending from block
layer.

Example of racing scenario: While UFS device is runtime-suspended

Thread #1: Executing UFS shutdown flow, e.g.,
           ufshcd_suspend(UFS_SHUTDOWN_PM)
Thread #2: Executing runtime resume flow triggered by I/O request,
           e.g., ufshcd_resume(UFS_RUNTIME_PM)

This breaks the assumption that UFS PM flows can not be running
concurrently and thus some unexpected racing behavior may happen.

Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 59358bb75014..cadfa9006972 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8599,10 +8599,14 @@ EXPORT_SYMBOL(ufshcd_runtime_idle);
 int ufshcd_shutdown(struct ufs_hba *hba)
 {
 	int ret = 0;
+	struct scsi_target *starget;
 
 	if (!hba->is_powered)
 		goto out;
 
+	list_for_each_entry(starget, &hba->host->__targets, siblings)
+		scsi_target_quiesce(starget);
+
 	if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
 		goto out;
 
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [RFC PATCH v1] scsi: ufs: Quiesce all scsi devices before shutdown
@ 2020-07-02  1:32 ` Stanley Chu
  0 siblings, 0 replies; 9+ messages in thread
From: Stanley Chu @ 2020-07-02  1:32 UTC (permalink / raw)
  To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb, bvanassche
  Cc: Stanley Chu, andy.teng, cc.chou, chun-hung.wu, kuohong.wang,
	linux-kernel, cang, linux-mediatek, peter.wang, matthias.bgg,
	beanhuo, chaotian.jing, linux-arm-kernel, asutoshd

Currently I/O request could be still submitted to UFS device while
UFS is working on shutdown flow. This may lead to racing as below
scenarios and finally system may crash due to unclocked register
accesses.

To fix this kind of issues, specifically quiesce all SCSI devices
before UFS shutdown to block all I/O request sending from block
layer.

Example of racing scenario: While UFS device is runtime-suspended

Thread #1: Executing UFS shutdown flow, e.g.,
           ufshcd_suspend(UFS_SHUTDOWN_PM)
Thread #2: Executing runtime resume flow triggered by I/O request,
           e.g., ufshcd_resume(UFS_RUNTIME_PM)

This breaks the assumption that UFS PM flows can not be running
concurrently and thus some unexpected racing behavior may happen.

Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 59358bb75014..cadfa9006972 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8599,10 +8599,14 @@ EXPORT_SYMBOL(ufshcd_runtime_idle);
 int ufshcd_shutdown(struct ufs_hba *hba)
 {
 	int ret = 0;
+	struct scsi_target *starget;
 
 	if (!hba->is_powered)
 		goto out;
 
+	list_for_each_entry(starget, &hba->host->__targets, siblings)
+		scsi_target_quiesce(starget);
+
 	if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
 		goto out;
 
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [RFC PATCH v1] scsi: ufs: Quiesce all scsi devices before shutdown
@ 2020-07-02  1:32 ` Stanley Chu
  0 siblings, 0 replies; 9+ messages in thread
From: Stanley Chu @ 2020-07-02  1:32 UTC (permalink / raw)
  To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb, bvanassche
  Cc: Stanley Chu, andy.teng, cc.chou, chun-hung.wu, kuohong.wang,
	linux-kernel, cang, linux-mediatek, peter.wang, matthias.bgg,
	beanhuo, chaotian.jing, linux-arm-kernel, asutoshd

Currently I/O request could be still submitted to UFS device while
UFS is working on shutdown flow. This may lead to racing as below
scenarios and finally system may crash due to unclocked register
accesses.

To fix this kind of issues, specifically quiesce all SCSI devices
before UFS shutdown to block all I/O request sending from block
layer.

Example of racing scenario: While UFS device is runtime-suspended

Thread #1: Executing UFS shutdown flow, e.g.,
           ufshcd_suspend(UFS_SHUTDOWN_PM)
Thread #2: Executing runtime resume flow triggered by I/O request,
           e.g., ufshcd_resume(UFS_RUNTIME_PM)

This breaks the assumption that UFS PM flows can not be running
concurrently and thus some unexpected racing behavior may happen.

Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 59358bb75014..cadfa9006972 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8599,10 +8599,14 @@ EXPORT_SYMBOL(ufshcd_runtime_idle);
 int ufshcd_shutdown(struct ufs_hba *hba)
 {
 	int ret = 0;
+	struct scsi_target *starget;
 
 	if (!hba->is_powered)
 		goto out;
 
+	list_for_each_entry(starget, &hba->host->__targets, siblings)
+		scsi_target_quiesce(starget);
+
 	if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
 		goto out;
 
-- 
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [RFC PATCH v1] scsi: ufs: Quiesce all scsi devices before shutdown
  2020-07-02  1:32 ` Stanley Chu
  (?)
@ 2020-07-02  3:02   ` Bart Van Assche
  -1 siblings, 0 replies; 9+ messages in thread
From: Bart Van Assche @ 2020-07-02  3:02 UTC (permalink / raw)
  To: Stanley Chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
  Cc: beanhuo, asutoshd, cang, matthias.bgg, linux-mediatek,
	linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
	chun-hung.wu, andy.teng, chaotian.jing, cc.chou

On 2020-07-01 18:32, Stanley Chu wrote:
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 59358bb75014..cadfa9006972 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -8599,10 +8599,14 @@ EXPORT_SYMBOL(ufshcd_runtime_idle);
>  int ufshcd_shutdown(struct ufs_hba *hba)
>  {
>  	int ret = 0;
> +	struct scsi_target *starget;
>  
>  	if (!hba->is_powered)
>  		goto out;
>  
> +	list_for_each_entry(starget, &hba->host->__targets, siblings)
> +		scsi_target_quiesce(starget);
> +
>  	if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
>  		goto out;

Please add a comment above the list_for_each_entry() loop that explains
that there is no matching scsi_target_unquiesce() call and also that
SCSI commands queued after the scsi_target_quiesce() call returned will
block until blk_cleanup_queue() is called (see also the blk_queue_dying()
check in blk_queue_enter()).

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [RFC PATCH v1] scsi: ufs: Quiesce all scsi devices before shutdown
@ 2020-07-02  3:02   ` Bart Van Assche
  0 siblings, 0 replies; 9+ messages in thread
From: Bart Van Assche @ 2020-07-02  3:02 UTC (permalink / raw)
  To: Stanley Chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
  Cc: andy.teng, cc.chou, chun-hung.wu, kuohong.wang, linux-kernel,
	cang, linux-mediatek, peter.wang, matthias.bgg, beanhuo,
	chaotian.jing, linux-arm-kernel, asutoshd

On 2020-07-01 18:32, Stanley Chu wrote:
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 59358bb75014..cadfa9006972 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -8599,10 +8599,14 @@ EXPORT_SYMBOL(ufshcd_runtime_idle);
>  int ufshcd_shutdown(struct ufs_hba *hba)
>  {
>  	int ret = 0;
> +	struct scsi_target *starget;
>  
>  	if (!hba->is_powered)
>  		goto out;
>  
> +	list_for_each_entry(starget, &hba->host->__targets, siblings)
> +		scsi_target_quiesce(starget);
> +
>  	if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
>  		goto out;

Please add a comment above the list_for_each_entry() loop that explains
that there is no matching scsi_target_unquiesce() call and also that
SCSI commands queued after the scsi_target_quiesce() call returned will
block until blk_cleanup_queue() is called (see also the blk_queue_dying()
check in blk_queue_enter()).

Thanks,

Bart.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [RFC PATCH v1] scsi: ufs: Quiesce all scsi devices before shutdown
@ 2020-07-02  3:02   ` Bart Van Assche
  0 siblings, 0 replies; 9+ messages in thread
From: Bart Van Assche @ 2020-07-02  3:02 UTC (permalink / raw)
  To: Stanley Chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
  Cc: andy.teng, cc.chou, chun-hung.wu, kuohong.wang, linux-kernel,
	cang, linux-mediatek, peter.wang, matthias.bgg, beanhuo,
	chaotian.jing, linux-arm-kernel, asutoshd

On 2020-07-01 18:32, Stanley Chu wrote:
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 59358bb75014..cadfa9006972 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -8599,10 +8599,14 @@ EXPORT_SYMBOL(ufshcd_runtime_idle);
>  int ufshcd_shutdown(struct ufs_hba *hba)
>  {
>  	int ret = 0;
> +	struct scsi_target *starget;
>  
>  	if (!hba->is_powered)
>  		goto out;
>  
> +	list_for_each_entry(starget, &hba->host->__targets, siblings)
> +		scsi_target_quiesce(starget);
> +
>  	if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
>  		goto out;

Please add a comment above the list_for_each_entry() loop that explains
that there is no matching scsi_target_unquiesce() call and also that
SCSI commands queued after the scsi_target_quiesce() call returned will
block until blk_cleanup_queue() is called (see also the blk_queue_dying()
check in blk_queue_enter()).

Thanks,

Bart.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [RFC PATCH v1] scsi: ufs: Quiesce all scsi devices before shutdown
  2020-07-02  3:02   ` Bart Van Assche
  (?)
@ 2020-07-02  3:36     ` Stanley Chu
  -1 siblings, 0 replies; 9+ messages in thread
From: Stanley Chu @ 2020-07-02  3:36 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
	beanhuo, asutoshd, cang, matthias.bgg, linux-mediatek,
	linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
	chun-hung.wu, andy.teng, chaotian.jing, cc.chou

Hi Bart,

On Wed, 2020-07-01 at 20:02 -0700, Bart Van Assche wrote:
> On 2020-07-01 18:32, Stanley Chu wrote:
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 59358bb75014..cadfa9006972 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -8599,10 +8599,14 @@ EXPORT_SYMBOL(ufshcd_runtime_idle);
> >  int ufshcd_shutdown(struct ufs_hba *hba)
> >  {
> >  	int ret = 0;
> > +	struct scsi_target *starget;
> >  
> >  	if (!hba->is_powered)
> >  		goto out;
> >  
> > +	list_for_each_entry(starget, &hba->host->__targets, siblings)
> > +		scsi_target_quiesce(starget);
> > +
> >  	if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
> >  		goto out;
> 
> Please add a comment above the list_for_each_entry() loop that explains
> that there is no matching scsi_target_unquiesce() call and also that
> SCSI commands queued after the scsi_target_quiesce() call returned will
> block until blk_cleanup_queue() is called (see also the blk_queue_dying()
> check in blk_queue_enter()).

Thanks for the review.
I'll add above comments in RFC v2.

Thanks,
Stanley Chu


^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [RFC PATCH v1] scsi: ufs: Quiesce all scsi devices before shutdown
@ 2020-07-02  3:36     ` Stanley Chu
  0 siblings, 0 replies; 9+ messages in thread
From: Stanley Chu @ 2020-07-02  3:36 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
	kuohong.wang, linux-kernel, avri.altman, cang, linux-mediatek,
	peter.wang, alim.akhtar, matthias.bgg, asutoshd, chaotian.jing,
	cc.chou, linux-arm-kernel, beanhuo

Hi Bart,

On Wed, 2020-07-01 at 20:02 -0700, Bart Van Assche wrote:
> On 2020-07-01 18:32, Stanley Chu wrote:
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 59358bb75014..cadfa9006972 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -8599,10 +8599,14 @@ EXPORT_SYMBOL(ufshcd_runtime_idle);
> >  int ufshcd_shutdown(struct ufs_hba *hba)
> >  {
> >  	int ret = 0;
> > +	struct scsi_target *starget;
> >  
> >  	if (!hba->is_powered)
> >  		goto out;
> >  
> > +	list_for_each_entry(starget, &hba->host->__targets, siblings)
> > +		scsi_target_quiesce(starget);
> > +
> >  	if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
> >  		goto out;
> 
> Please add a comment above the list_for_each_entry() loop that explains
> that there is no matching scsi_target_unquiesce() call and also that
> SCSI commands queued after the scsi_target_quiesce() call returned will
> block until blk_cleanup_queue() is called (see also the blk_queue_dying()
> check in blk_queue_enter()).

Thanks for the review.
I'll add above comments in RFC v2.

Thanks,
Stanley Chu

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [RFC PATCH v1] scsi: ufs: Quiesce all scsi devices before shutdown
@ 2020-07-02  3:36     ` Stanley Chu
  0 siblings, 0 replies; 9+ messages in thread
From: Stanley Chu @ 2020-07-02  3:36 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
	kuohong.wang, linux-kernel, avri.altman, cang, linux-mediatek,
	peter.wang, alim.akhtar, matthias.bgg, asutoshd, chaotian.jing,
	cc.chou, linux-arm-kernel, beanhuo

Hi Bart,

On Wed, 2020-07-01 at 20:02 -0700, Bart Van Assche wrote:
> On 2020-07-01 18:32, Stanley Chu wrote:
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 59358bb75014..cadfa9006972 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -8599,10 +8599,14 @@ EXPORT_SYMBOL(ufshcd_runtime_idle);
> >  int ufshcd_shutdown(struct ufs_hba *hba)
> >  {
> >  	int ret = 0;
> > +	struct scsi_target *starget;
> >  
> >  	if (!hba->is_powered)
> >  		goto out;
> >  
> > +	list_for_each_entry(starget, &hba->host->__targets, siblings)
> > +		scsi_target_quiesce(starget);
> > +
> >  	if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
> >  		goto out;
> 
> Please add a comment above the list_for_each_entry() loop that explains
> that there is no matching scsi_target_unquiesce() call and also that
> SCSI commands queued after the scsi_target_quiesce() call returned will
> block until blk_cleanup_queue() is called (see also the blk_queue_dying()
> check in blk_queue_enter()).

Thanks for the review.
I'll add above comments in RFC v2.

Thanks,
Stanley Chu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-07-02  3:38 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-02  1:32 [RFC PATCH v1] scsi: ufs: Quiesce all scsi devices before shutdown Stanley Chu
2020-07-02  1:32 ` Stanley Chu
2020-07-02  1:32 ` Stanley Chu
2020-07-02  3:02 ` Bart Van Assche
2020-07-02  3:02   ` Bart Van Assche
2020-07-02  3:02   ` Bart Van Assche
2020-07-02  3:36   ` Stanley Chu
2020-07-02  3:36     ` Stanley Chu
2020-07-02  3:36     ` Stanley Chu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.