Re: [PATCH v2 9/9] block: Do not accept any requests while suspended

Can Guo
2020-11-18T01:12:53Z

On 2020-11-16 11:04, Bart Van Assche wrote:
> From: Alan Stern <stern@rowland.harvard.edu>
>
> blk_queue_enter() accepts BLK_MQ_REQ_PREEMPT independent of the runtime
> power management state. Since SCSI domain validation no longer depends
> on
> this behavior, modify the behavior of blk_queue_enter() as follows:
> - Do not accept any requests while suspended.
> - Only process power management requests while suspending or resuming.
>
> Submitting BLK_MQ_REQ_PREEMPT requests to a device that is runtime-
> suspended causes runtime-suspended block devices not to resume as they
> should. The request which should cause a runtime resume instead gets
> issued directly, without resuming the device first. Of course the
> device
> can't handle it properly, the I/O fails, and the device remains
> suspended.
>
> The problem is fixed by checking that the queue's runtime-PM status
> isn't RPM_SUSPENDED before allowing a request to be issued, and
> queuing a runtime-resume request if it is. In particular, the inline
> blk_pm_request_resume() routine is renamed blk_pm_resume_queue() and
> the code is unified by merging the surrounding checks into the
> routine. If the queue isn't set up for runtime PM, or there currently
> is no restriction on allowed requests, the request is allowed.
> Likewise if the BLK_MQ_REQ_PREEMPT flag is set and the status isn't
> RPM_SUSPENDED. Otherwise a runtime resume is queued and the request
> is blocked until conditions are more suitable.
>
> Cc: Can Guo <cang@codeaurora.org>
> Cc: Stanley Chu <stanley.chu@mediatek.com>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reported-and-tested-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Reviewed-by: Can Guo <cang@codeaurora.org>

> [ bvanassche: modified commit message and removed Cc: stable because
> without
> the previous patches from this series this patch would break parallel
> SCSI
> domain validation ]
> ---
> block/blk-core.c | 6 +++---
> block/blk-pm.h | 14 +++++++++-----
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index a00bce9f46d8..230880cbf8c8 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -440,7 +440,8 @@ int blk_queue_enter(struct request_queue *q,
> blk_mq_req_flags_t flags)
> * responsible for ensuring that that counter is
> * globally visible before the queue is unfrozen.
> */
> - if (pm || !blk_queue_pm_only(q)) {
> + if ((pm && q->rpm_status != RPM_SUSPENDED) ||
> + !blk_queue_pm_only(q)) {
> success = true;
> } else {
> percpu_ref_put(&q->q_usage_counter);
> @@ -465,8 +466,7 @@ int blk_queue_enter(struct request_queue *q,
> blk_mq_req_flags_t flags)
>
> wait_event(q->mq_freeze_wq,
> (!q->mq_freeze_depth &&
> - (pm || (blk_pm_request_resume(q),
> - !blk_queue_pm_only(q)))) ||
> + blk_pm_resume_queue(pm, q)) ||
> blk_queue_dying(q));
> if (blk_queue_dying(q))
> return -ENODEV;
> diff --git a/block/blk-pm.h b/block/blk-pm.h
> index ea5507d23e75..a2283cc9f716 100644
> --- a/block/blk-pm.h
> +++ b/block/blk-pm.h
> @@ -6,11 +6,14 @@
> #include <linux/pm_runtime.h>
>
> #ifdef CONFIG_PM
> -static inline void blk_pm_request_resume(struct request_queue *q)
> +static inline int blk_pm_resume_queue(const bool pm, struct
> request_queue *q)
> {
> - if (q->dev && (q->rpm_status == RPM_SUSPENDED ||
> - q->rpm_status == RPM_SUSPENDING))
> - pm_request_resume(q->dev);
> + if (!q->dev || !blk_queue_pm_only(q))
> + return 1; /* Nothing to do */
> + if (pm && q->rpm_status != RPM_SUSPENDED)
> + return 1; /* Request allowed */
> + pm_request_resume(q->dev);
> + return 0;
> }
>
> static inline void blk_pm_mark_last_busy(struct request *rq)
> @@ -44,8 +47,9 @@ static inline void blk_pm_put_request(struct request
> *rq)
> --rq->q->nr_pending;
> }
> #else
> -static inline void blk_pm_request_resume(struct request_queue *q)
> +static inline int blk_pm_resume_queue(const bool pm, struct
> request_queue *q)
> {
> + return 1;
> }
>
> static inline void blk_pm_mark_last_busy(struct request *rq)