| Message ID | 20201116030459.13963-10-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Rework runtime suspend and SCSI domain validation | expand |
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
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)
On Mon, 2020-11-16 at 11:04 +0800, 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: Stanley Chu <stanley.chu@mediatek.com>
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)