Linux-SCSI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] SCSI and block: Simplify resume handling
@ 2020-07-01 18:37 Alan Stern
  2020-07-06  2:41 ` Bart Van Assche
  2020-07-22  1:46 ` [PATCH v2] " Can Guo
  0 siblings, 2 replies; 8+ messages in thread
From: Alan Stern @ 2020-07-01 18:37 UTC (permalink / raw)
  To: Bart Van Assche, martin.petersen; +Cc: Can Guo, linux-scsi, linux-block

Commit 05d18ae1cc8a ("scsi: pm: Balance pm_only counter of request
queue during system resume") fixed a problem in the block layer's
runtime-PM code: blk_set_runtime_active() failed to call
blk_clear_pm_only().  However, the commit's implementation was
awkward; it forced the SCSI system-resume handler to choose whether to
call blk_post_runtime_resume() or blk_set_runtime_active(), depending
on whether or not the SCSI device had previously been runtime
suspended.

This patch simplifies the situation considerably by adding the missing
function call directly into blk_set_runtime_active() (under the
condition that the queue is not already in the RPM_ACTIVE state).
This allows the SCSI routine to revert back to its original form.
Furthermore, making this change reveals that blk_post_runtime_resume()
(in its success pathway) does exactly the same thing as
blk_set_runtime_active().  The duplicate code is easily removed by
making one routine call the other.

No functional changes are intended.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: Can Guo <cang@codeaurora.org>
CC: Bart Van Assche <bvanassche@acm.org>

---

v2:	Don't call blk_clear_pm_only() if the queue's RPM status was
	already set to RPM_ACTIVE.  This happens during a system resume
	if the device was not in runtime suspend beforehand.

Martin:

Since you merged the oritinal 05d18ae1cc8a commit, I'm submitting this to
you as an update.  If you would prefer to have it go by way of the
block-layer tree, let me know and I'll resend it.


[as1939b]


 block/blk-pm.c         |   30 +++++++++++++++---------------
 drivers/scsi/scsi_pm.c |   10 ++--------
 2 files changed, 17 insertions(+), 23 deletions(-)

Index: usb-devel/block/blk-pm.c
===================================================================
--- usb-devel.orig/block/blk-pm.c
+++ usb-devel/block/blk-pm.c
@@ -164,30 +164,21 @@ EXPORT_SYMBOL(blk_pre_runtime_resume);
  *
  * Description:
  *    Update the queue's runtime status according to the return value of the
- *    device's runtime_resume function. If it is successfully resumed, process
- *    the requests that are queued into the device's queue when it is resuming
- *    and then mark last busy and initiate autosuspend for it.
+ *    device's runtime_resume function. If the resume was successful, call
+ *    blk_set_runtime_active() to do the real work of restarting the queue.
  *
  *    This function should be called near the end of the device's
  *    runtime_resume callback.
  */
 void blk_post_runtime_resume(struct request_queue *q, int err)
 {
-	if (!q->dev)
-		return;
-
-	spin_lock_irq(&q->queue_lock);
 	if (!err) {
-		q->rpm_status = RPM_ACTIVE;
-		pm_runtime_mark_last_busy(q->dev);
-		pm_request_autosuspend(q->dev);
-	} else {
+		blk_set_runtime_active(q);
+	} else if (q->dev) {
+		spin_lock_irq(&q->queue_lock);
 		q->rpm_status = RPM_SUSPENDED;
+		spin_unlock_irq(&q->queue_lock);
 	}
-	spin_unlock_irq(&q->queue_lock);
-
-	if (!err)
-		blk_clear_pm_only(q);
 }
 EXPORT_SYMBOL(blk_post_runtime_resume);
 
@@ -204,15 +195,24 @@ EXPORT_SYMBOL(blk_post_runtime_resume);
  * This function can be used in driver's resume hook to correct queue
  * runtime PM status and re-enable peeking requests from the queue. It
  * should be called before first request is added to the queue.
+ *
+ * This function is also called by blk_post_runtime_resume() for successful
+ * runtime resumes.  It does everything necessary to restart the queue.
  */
 void blk_set_runtime_active(struct request_queue *q)
 {
 	if (q->dev) {
+		int old_status;
+
 		spin_lock_irq(&q->queue_lock);
+		old_status = q->rpm_status;
 		q->rpm_status = RPM_ACTIVE;
 		pm_runtime_mark_last_busy(q->dev);
 		pm_request_autosuspend(q->dev);
 		spin_unlock_irq(&q->queue_lock);
+
+		if (old_status != RPM_ACTIVE)
+			blk_clear_pm_only(q);
 	}
 }
 EXPORT_SYMBOL(blk_set_runtime_active);
Index: usb-devel/drivers/scsi/scsi_pm.c
===================================================================
--- usb-devel.orig/drivers/scsi/scsi_pm.c
+++ usb-devel/drivers/scsi/scsi_pm.c
@@ -80,10 +80,6 @@ static int scsi_dev_type_resume(struct d
 	dev_dbg(dev, "scsi resume: %d\n", err);
 
 	if (err == 0) {
-		bool was_runtime_suspended;
-
-		was_runtime_suspended = pm_runtime_suspended(dev);
-
 		pm_runtime_disable(dev);
 		err = pm_runtime_set_active(dev);
 		pm_runtime_enable(dev);
@@ -97,10 +93,8 @@ static int scsi_dev_type_resume(struct d
 		 */
 		if (!err && scsi_is_sdev_device(dev)) {
 			struct scsi_device *sdev = to_scsi_device(dev);
-			if (was_runtime_suspended)
-				blk_post_runtime_resume(sdev->request_queue, 0);
-			else
-				blk_set_runtime_active(sdev->request_queue);
+
+			blk_set_runtime_active(sdev->request_queue);
 		}
 	}
 


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

* Re: [PATCH v2] SCSI and block: Simplify resume handling
  2020-07-01 18:37 [PATCH v2] SCSI and block: Simplify resume handling Alan Stern
@ 2020-07-06  2:41 ` Bart Van Assche
  2020-07-06 15:14   ` [PATCH v3] " Alan Stern
  2020-07-22  1:46 ` [PATCH v2] " Can Guo
  1 sibling, 1 reply; 8+ messages in thread
From: Bart Van Assche @ 2020-07-06  2:41 UTC (permalink / raw)
  To: Alan Stern, martin.petersen; +Cc: Can Guo, linux-scsi, linux-block

On 2020-07-01 11:37, Alan Stern wrote:
>  void blk_post_runtime_resume(struct request_queue *q, int err)
>  {
> -	if (!q->dev)
> -		return;
> -
> -	spin_lock_irq(&q->queue_lock);
>  	if (!err) {
> -		q->rpm_status = RPM_ACTIVE;
> -		pm_runtime_mark_last_busy(q->dev);
> -		pm_request_autosuspend(q->dev);
> -	} else {
> +		blk_set_runtime_active(q);
> +	} else if (q->dev) {
> +		spin_lock_irq(&q->queue_lock);
>  		q->rpm_status = RPM_SUSPENDED;
> +		spin_unlock_irq(&q->queue_lock);
>  	}
> -	spin_unlock_irq(&q->queue_lock);
> -
> -	if (!err)
> -		blk_clear_pm_only(q);
>  }
>  EXPORT_SYMBOL(blk_post_runtime_resume);

I'd like to keep the if (!q->dev) check at the start of the function instead
of moving it to the middle of the function to keep the symmetry with the
existing runtime power management functions in the same source file.

>  void blk_set_runtime_active(struct request_queue *q)
>  {
>  	if (q->dev) {
> +		int old_status;
> +
>  		spin_lock_irq(&q->queue_lock);
> +		old_status = q->rpm_status;
>  		q->rpm_status = RPM_ACTIVE;
>  		pm_runtime_mark_last_busy(q->dev);
>  		pm_request_autosuspend(q->dev);
>  		spin_unlock_irq(&q->queue_lock);
> +
> +		if (old_status != RPM_ACTIVE)
> +			blk_clear_pm_only(q);
>  	}
>  }

Since this function is being modified, please change the if (q->dev) into
if (!q->dev) return since returning early is the recommended kernel coding
style.

Thanks,

Bart.

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

* [PATCH v3] SCSI and block: Simplify resume handling
  2020-07-06  2:41 ` Bart Van Assche
@ 2020-07-06 15:14   ` Alan Stern
  2020-07-09  3:29     ` Bart Van Assche
                       ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Alan Stern @ 2020-07-06 15:14 UTC (permalink / raw)
  To: Bart Van Assche; +Cc: martin.petersen, Can Guo, linux-scsi, linux-block

Commit 05d18ae1cc8a ("scsi: pm: Balance pm_only counter of request
queue during system resume") fixed a problem in the block layer's
runtime-PM code: blk_set_runtime_active() failed to call
blk_clear_pm_only().  However, the commit's implementation was
awkward; it forced the SCSI system-resume handler to choose whether to
call blk_post_runtime_resume() or blk_set_runtime_active(), depending
on whether or not the SCSI device had previously been runtime
suspended.

This patch simplifies the situation considerably by adding the missing
function call directly into blk_set_runtime_active() (under the
condition that the queue is not already in the RPM_ACTIVE state).
This allows the SCSI routine to revert back to its original form.
Furthermore, making this change reveals that blk_post_runtime_resume()
(in its success pathway) does exactly the same thing as
blk_set_runtime_active().  The duplicate code is easily removed by
making one routine call the other.

No functional changes are intended.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: Can Guo <cang@codeaurora.org>
CC: Bart Van Assche <bvanassche@acm.org>

---

v3:	Arrange the code in blk_pm_post_runtime_resume() and
	blk_set_runtime_active() so that the "if (!q->dev)" tests and
	early returns come first, as in the other nearby functions.

v2:	Don't call blk_clear_pm_only() if the queue's RPM status was
	already set to RPM_ACTIVE.  This happens during a system resume
	if the device was not in runtime suspend beforehand.


[as1939c]


 block/blk-pm.c         |   41 ++++++++++++++++++++++-------------------
 drivers/scsi/scsi_pm.c |   10 ++--------
 2 files changed, 24 insertions(+), 27 deletions(-)

Index: usb-devel/block/blk-pm.c
===================================================================
--- usb-devel.orig/block/blk-pm.c
+++ usb-devel/block/blk-pm.c
@@ -164,9 +164,8 @@ EXPORT_SYMBOL(blk_pre_runtime_resume);
  *
  * Description:
  *    Update the queue's runtime status according to the return value of the
- *    device's runtime_resume function. If it is successfully resumed, process
- *    the requests that are queued into the device's queue when it is resuming
- *    and then mark last busy and initiate autosuspend for it.
+ *    device's runtime_resume function. If the resume was successful, call
+ *    blk_set_runtime_active() to do the real work of restarting the queue.
  *
  *    This function should be called near the end of the device's
  *    runtime_resume callback.
@@ -175,19 +174,13 @@ void blk_post_runtime_resume(struct requ
 {
 	if (!q->dev)
 		return;
-
-	spin_lock_irq(&q->queue_lock);
 	if (!err) {
-		q->rpm_status = RPM_ACTIVE;
-		pm_runtime_mark_last_busy(q->dev);
-		pm_request_autosuspend(q->dev);
+		blk_set_runtime_active(q);
 	} else {
+		spin_lock_irq(&q->queue_lock);
 		q->rpm_status = RPM_SUSPENDED;
+		spin_unlock_irq(&q->queue_lock);
 	}
-	spin_unlock_irq(&q->queue_lock);
-
-	if (!err)
-		blk_clear_pm_only(q);
 }
 EXPORT_SYMBOL(blk_post_runtime_resume);
 
@@ -204,15 +197,25 @@ EXPORT_SYMBOL(blk_post_runtime_resume);
  * This function can be used in driver's resume hook to correct queue
  * runtime PM status and re-enable peeking requests from the queue. It
  * should be called before first request is added to the queue.
+ *
+ * This function is also called by blk_post_runtime_resume() for successful
+ * runtime resumes.  It does everything necessary to restart the queue.
  */
 void blk_set_runtime_active(struct request_queue *q)
 {
-	if (q->dev) {
-		spin_lock_irq(&q->queue_lock);
-		q->rpm_status = RPM_ACTIVE;
-		pm_runtime_mark_last_busy(q->dev);
-		pm_request_autosuspend(q->dev);
-		spin_unlock_irq(&q->queue_lock);
-	}
+	int old_status;
+
+	if (!q->dev)
+		return;
+
+	spin_lock_irq(&q->queue_lock);
+	old_status = q->rpm_status;
+	q->rpm_status = RPM_ACTIVE;
+	pm_runtime_mark_last_busy(q->dev);
+	pm_request_autosuspend(q->dev);
+	spin_unlock_irq(&q->queue_lock);
+
+	if (old_status != RPM_ACTIVE)
+		blk_clear_pm_only(q);
 }
 EXPORT_SYMBOL(blk_set_runtime_active);
Index: usb-devel/drivers/scsi/scsi_pm.c
===================================================================
--- usb-devel.orig/drivers/scsi/scsi_pm.c
+++ usb-devel/drivers/scsi/scsi_pm.c
@@ -80,10 +80,6 @@ static int scsi_dev_type_resume(struct d
 	dev_dbg(dev, "scsi resume: %d\n", err);
 
 	if (err == 0) {
-		bool was_runtime_suspended;
-
-		was_runtime_suspended = pm_runtime_suspended(dev);
-
 		pm_runtime_disable(dev);
 		err = pm_runtime_set_active(dev);
 		pm_runtime_enable(dev);
@@ -97,10 +93,8 @@ static int scsi_dev_type_resume(struct d
 		 */
 		if (!err && scsi_is_sdev_device(dev)) {
 			struct scsi_device *sdev = to_scsi_device(dev);
-			if (was_runtime_suspended)
-				blk_post_runtime_resume(sdev->request_queue, 0);
-			else
-				blk_set_runtime_active(sdev->request_queue);
+
+			blk_set_runtime_active(sdev->request_queue);
 		}
 	}
 

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

* Re: [PATCH v3] SCSI and block: Simplify resume handling
  2020-07-06 15:14   ` [PATCH v3] " Alan Stern
@ 2020-07-09  3:29     ` Bart Van Assche
  2020-07-21  3:01       ` Martin K. Petersen
  2020-07-23  3:53     ` Bart Van Assche
  2020-07-25  2:50     ` Martin K. Petersen
  2 siblings, 1 reply; 8+ messages in thread
From: Bart Van Assche @ 2020-07-09  3:29 UTC (permalink / raw)
  To: Alan Stern; +Cc: martin.petersen, Can Guo, linux-scsi, linux-block

On 2020-07-06 08:14, Alan Stern wrote:
> Commit 05d18ae1cc8a ("scsi: pm: Balance pm_only counter of request
> queue during system resume") fixed a problem in the block layer's
> runtime-PM code: blk_set_runtime_active() failed to call
> blk_clear_pm_only().  However, the commit's implementation was
> awkward; it forced the SCSI system-resume handler to choose whether to
> call blk_post_runtime_resume() or blk_set_runtime_active(), depending
> on whether or not the SCSI device had previously been runtime
> suspended.
> 
> This patch simplifies the situation considerably by adding the missing
> function call directly into blk_set_runtime_active() (under the
> condition that the queue is not already in the RPM_ACTIVE state).
> This allows the SCSI routine to revert back to its original form.
> Furthermore, making this change reveals that blk_post_runtime_resume()
> (in its success pathway) does exactly the same thing as
> blk_set_runtime_active().  The duplicate code is easily removed by
> making one routine call the other.
> 
> No functional changes are intended.
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> CC: Can Guo <cang@codeaurora.org>
> CC: Bart Van Assche <bvanassche@acm.org>

This patch looks good to me. Can, since this patch modifies code in
which you recently fixed a bug, would it be possible to test this patch?

Thanks,

Bart.

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

* Re: [PATCH v3] SCSI and block: Simplify resume handling
  2020-07-09  3:29     ` Bart Van Assche
@ 2020-07-21  3:01       ` Martin K. Petersen
  0 siblings, 0 replies; 8+ messages in thread
From: Martin K. Petersen @ 2020-07-21  3:01 UTC (permalink / raw)
  To: Can Guo
  Cc: Alan Stern, martin.petersen, Bart Van Assche, linux-scsi, linux-block


> This patch looks good to me. Can, since this patch modifies code in
> which you recently fixed a bug, would it be possible to test this
> patch?

Can: Please verify Alan's patch:

	https://patchwork.kernel.org/patch/11646029/
        
Bart: A proper review tag would be appreciated.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: [PATCH v2] SCSI and block: Simplify resume handling
  2020-07-01 18:37 [PATCH v2] SCSI and block: Simplify resume handling Alan Stern
  2020-07-06  2:41 ` Bart Van Assche
@ 2020-07-22  1:46 ` Can Guo
  1 sibling, 0 replies; 8+ messages in thread
From: Can Guo @ 2020-07-22  1:46 UTC (permalink / raw)
  To: Alan Stern; +Cc: Bart Van Assche, martin.petersen, linux-scsi, linux-block

Hi Alan,

On 2020-07-02 02:37, Alan Stern wrote:
> Commit 05d18ae1cc8a ("scsi: pm: Balance pm_only counter of request
> queue during system resume") fixed a problem in the block layer's
> runtime-PM code: blk_set_runtime_active() failed to call
> blk_clear_pm_only().  However, the commit's implementation was
> awkward; it forced the SCSI system-resume handler to choose whether to
> call blk_post_runtime_resume() or blk_set_runtime_active(), depending
> on whether or not the SCSI device had previously been runtime
> suspended.
> 

Yeah, even I thoght it was awkward as I couldn't find a way or a 
existing
API to fix it subtly without changing block layer. I was trying not to
make noise to block layer as I am not an expert of it, so I chose the 
akward
way to keep the fix within driver layer. Anyways, thanks for the change 
and
I will come back after I test it.

Can Guo.

> This patch simplifies the situation considerably by adding the missing
> function call directly into blk_set_runtime_active() (under the
> condition that the queue is not already in the RPM_ACTIVE state).
> This allows the SCSI routine to revert back to its original form.
> Furthermore, making this change reveals that blk_post_runtime_resume()
> (in its success pathway) does exactly the same thing as
> blk_set_runtime_active().  The duplicate code is easily removed by
> making one routine call the other.
> 
> No functional changes are intended.
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> CC: Can Guo <cang@codeaurora.org>
> CC: Bart Van Assche <bvanassche@acm.org>
> 
> ---
> 
> v2:	Don't call blk_clear_pm_only() if the queue's RPM status was
> 	already set to RPM_ACTIVE.  This happens during a system resume
> 	if the device was not in runtime suspend beforehand.
> 
> Martin:
> 
> Since you merged the oritinal 05d18ae1cc8a commit, I'm submitting this 
> to
> you as an update.  If you would prefer to have it go by way of the
> block-layer tree, let me know and I'll resend it.
> 
> 
> [as1939b]
> 
> 
>  block/blk-pm.c         |   30 +++++++++++++++---------------
>  drivers/scsi/scsi_pm.c |   10 ++--------
>  2 files changed, 17 insertions(+), 23 deletions(-)
> 
> Index: usb-devel/block/blk-pm.c
> ===================================================================
> --- usb-devel.orig/block/blk-pm.c
> +++ usb-devel/block/blk-pm.c
> @@ -164,30 +164,21 @@ EXPORT_SYMBOL(blk_pre_runtime_resume);
>   *
>   * Description:
>   *    Update the queue's runtime status according to the return value 
> of the
> - *    device's runtime_resume function. If it is successfully resumed, 
> process
> - *    the requests that are queued into the device's queue when it is 
> resuming
> - *    and then mark last busy and initiate autosuspend for it.
> + *    device's runtime_resume function. If the resume was successful, 
> call
> + *    blk_set_runtime_active() to do the real work of restarting the 
> queue.
>   *
>   *    This function should be called near the end of the device's
>   *    runtime_resume callback.
>   */
>  void blk_post_runtime_resume(struct request_queue *q, int err)
>  {
> -	if (!q->dev)
> -		return;
> -
> -	spin_lock_irq(&q->queue_lock);
>  	if (!err) {
> -		q->rpm_status = RPM_ACTIVE;
> -		pm_runtime_mark_last_busy(q->dev);
> -		pm_request_autosuspend(q->dev);
> -	} else {
> +		blk_set_runtime_active(q);
> +	} else if (q->dev) {
> +		spin_lock_irq(&q->queue_lock);
>  		q->rpm_status = RPM_SUSPENDED;
> +		spin_unlock_irq(&q->queue_lock);
>  	}
> -	spin_unlock_irq(&q->queue_lock);
> -
> -	if (!err)
> -		blk_clear_pm_only(q);
>  }
>  EXPORT_SYMBOL(blk_post_runtime_resume);
> 
> @@ -204,15 +195,24 @@ EXPORT_SYMBOL(blk_post_runtime_resume);
>   * This function can be used in driver's resume hook to correct queue
>   * runtime PM status and re-enable peeking requests from the queue. It
>   * should be called before first request is added to the queue.
> + *
> + * This function is also called by blk_post_runtime_resume() for 
> successful
> + * runtime resumes.  It does everything necessary to restart the 
> queue.
>   */
>  void blk_set_runtime_active(struct request_queue *q)
>  {
>  	if (q->dev) {
> +		int old_status;
> +
>  		spin_lock_irq(&q->queue_lock);
> +		old_status = q->rpm_status;
>  		q->rpm_status = RPM_ACTIVE;
>  		pm_runtime_mark_last_busy(q->dev);
>  		pm_request_autosuspend(q->dev);
>  		spin_unlock_irq(&q->queue_lock);
> +
> +		if (old_status != RPM_ACTIVE)
> +			blk_clear_pm_only(q);
>  	}
>  }
>  EXPORT_SYMBOL(blk_set_runtime_active);
> Index: usb-devel/drivers/scsi/scsi_pm.c
> ===================================================================
> --- usb-devel.orig/drivers/scsi/scsi_pm.c
> +++ usb-devel/drivers/scsi/scsi_pm.c
> @@ -80,10 +80,6 @@ static int scsi_dev_type_resume(struct d
>  	dev_dbg(dev, "scsi resume: %d\n", err);
> 
>  	if (err == 0) {
> -		bool was_runtime_suspended;
> -
> -		was_runtime_suspended = pm_runtime_suspended(dev);
> -
>  		pm_runtime_disable(dev);
>  		err = pm_runtime_set_active(dev);
>  		pm_runtime_enable(dev);
> @@ -97,10 +93,8 @@ static int scsi_dev_type_resume(struct d
>  		 */
>  		if (!err && scsi_is_sdev_device(dev)) {
>  			struct scsi_device *sdev = to_scsi_device(dev);
> -			if (was_runtime_suspended)
> -				blk_post_runtime_resume(sdev->request_queue, 0);
> -			else
> -				blk_set_runtime_active(sdev->request_queue);
> +
> +			blk_set_runtime_active(sdev->request_queue);
>  		}
>  	}

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

* Re: [PATCH v3] SCSI and block: Simplify resume handling
  2020-07-06 15:14   ` [PATCH v3] " Alan Stern
  2020-07-09  3:29     ` Bart Van Assche
@ 2020-07-23  3:53     ` Bart Van Assche
  2020-07-25  2:50     ` Martin K. Petersen
  2 siblings, 0 replies; 8+ messages in thread
From: Bart Van Assche @ 2020-07-23  3:53 UTC (permalink / raw)
  To: Alan Stern; +Cc: martin.petersen, Can Guo, linux-scsi, linux-block

On 2020-07-06 08:14, Alan Stern wrote:
> Commit 05d18ae1cc8a ("scsi: pm: Balance pm_only counter of request
> queue during system resume") fixed a problem in the block layer's
> runtime-PM code: blk_set_runtime_active() failed to call
> blk_clear_pm_only().  However, the commit's implementation was
> awkward; it forced the SCSI system-resume handler to choose whether to
> call blk_post_runtime_resume() or blk_set_runtime_active(), depending
> on whether or not the SCSI device had previously been runtime
> suspended.
> 
> This patch simplifies the situation considerably by adding the missing
> function call directly into blk_set_runtime_active() (under the
> condition that the queue is not already in the RPM_ACTIVE state).
> This allows the SCSI routine to revert back to its original form.
> Furthermore, making this change reveals that blk_post_runtime_resume()
> (in its success pathway) does exactly the same thing as
> blk_set_runtime_active().  The duplicate code is easily removed by
> making one routine call the other.

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

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

* Re: [PATCH v3] SCSI and block: Simplify resume handling
  2020-07-06 15:14   ` [PATCH v3] " Alan Stern
  2020-07-09  3:29     ` Bart Van Assche
  2020-07-23  3:53     ` Bart Van Assche
@ 2020-07-25  2:50     ` Martin K. Petersen
  2 siblings, 0 replies; 8+ messages in thread
From: Martin K. Petersen @ 2020-07-25  2:50 UTC (permalink / raw)
  To: Bart Van Assche, Alan Stern
  Cc: Martin K . Petersen, Can Guo, linux-scsi, linux-block

On Mon, 6 Jul 2020 11:14:36 -0400, Alan Stern wrote:

> Commit 05d18ae1cc8a ("scsi: pm: Balance pm_only counter of request
> queue during system resume") fixed a problem in the block layer's
> runtime-PM code: blk_set_runtime_active() failed to call
> blk_clear_pm_only().  However, the commit's implementation was
> awkward; it forced the SCSI system-resume handler to choose whether to
> call blk_post_runtime_resume() or blk_set_runtime_active(), depending
> on whether or not the SCSI device had previously been runtime
> suspended.
> 
> [...]

Applied to 5.9/scsi-queue, thanks!

[1/1] scsi: block: pm: Simplify resume handling
      https://git.kernel.org/mkp/scsi/c/8f38f8e0a30e

-- 
Martin K. Petersen	Oracle Linux Engineering

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

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01 18:37 [PATCH v2] SCSI and block: Simplify resume handling Alan Stern
2020-07-06  2:41 ` Bart Van Assche
2020-07-06 15:14   ` [PATCH v3] " Alan Stern
2020-07-09  3:29     ` Bart Van Assche
2020-07-21  3:01       ` Martin K. Petersen
2020-07-23  3:53     ` Bart Van Assche
2020-07-25  2:50     ` Martin K. Petersen
2020-07-22  1:46 ` [PATCH v2] " Can Guo

Linux-SCSI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-scsi/0 linux-scsi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-scsi linux-scsi/ https://lore.kernel.org/linux-scsi \
		linux-scsi@vger.kernel.org
	public-inbox-index linux-scsi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-scsi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git