[PATCH v2] block: fix peeking requests during PM

[PATCH v2] block: fix peeking requests during PM

[Christoph Hellwig]

We need to look for an active PM request until the next softbarrier
instead of looking for the first non-PM request.  Otherwise any cause
of request reordering might starve the PM request(s).

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 14f7674fa0b1..480029e4fb54 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2464,20 +2464,22 @@ void blk_account_io_done(struct request *req)
  * Don't process normal requests when queue is suspended
  * or in the process of suspending/resuming
  */
-static struct request *blk_pm_peek_request(struct request_queue *q,
-					   struct request *rq)
+static bool blk_pm_allow_request(struct request *rq)
 {
-	if (q->dev && (q->rpm_status == RPM_SUSPENDED ||
-	    (q->rpm_status != RPM_ACTIVE && !(rq->rq_flags & RQF_PM))))
-		return NULL;
-	else
-		return rq;
+	switch (rq->q->rpm_status) {
+	case RPM_RESUMING:
+	case RPM_SUSPENDING:
+		return rq->rq_flags & RQF_PM;
+	case RPM_SUSPENDED:
+		return false;
+	}
+
+	return true;
 }
 #else
-static inline struct request *blk_pm_peek_request(struct request_queue *q,
-						  struct request *rq)
+static bool blk_pm_allow_request(struct request *rq)
 {
-	return rq;
+	return true;
 }
 #endif
 
@@ -2525,9 +2527,12 @@ static struct request *elv_next_request(struct request_queue *q)
 	WARN_ON_ONCE(q->mq_ops);
 
 	while (1) {
-		if (!list_empty(&q->queue_head)) {
-			rq = list_entry_rq(q->queue_head.next);
-			return rq;
+		list_for_each_entry(rq, &q->queue_head, queuelist) {
+			if (blk_pm_allow_request(rq))
+				return rq;
+
+			if (rq->rq_flags & RQF_SOFTBARRIER)
+				break;
 		}
 
 		/*
@@ -2578,10 +2583,6 @@ struct request *blk_peek_request(struct request_queue *q)
 	WARN_ON_ONCE(q->mq_ops);
 
 	while ((rq = elv_next_request(q)) != NULL) {
-		rq = blk_pm_peek_request(q, rq);
-		if (!rq)
-			break;
-
 		if (!(rq->rq_flags & RQF_STARTED)) {
 			/*
 			 * This is the first time the device driver
-- 
2.14.2

Re: [PATCH v2] block: fix peeking requests during PM

[Ming Lei]

On Fri, Oct 20, 2017 at 04:45:23PM +0200, Christoph Hellwig wrote:
> We need to look for an active PM request until the next softbarrier
> instead of looking for the first non-PM request.  Otherwise any cause
> of request reordering might starve the PM request(s).

Hi Christoph,

Could you share us how the starve is caused? Looks all PM
request is run via scsi_execute(), and the caller is always
waiting for the completion of PM request.

Especially PM request is run after SCSI device is in quiesce
state, during that time, no normal(non-preempt) request can
enter queue after the making quiesce safe patchset is merged.

--
Ming

Re: [PATCH v2] block: fix peeking requests during PM

[Christoph Hellwig]

On Mon, Oct 23, 2017 at 06:43:16PM +0800, Ming Lei wrote:
> On Fri, Oct 20, 2017 at 04:45:23PM +0200, Christoph Hellwig wrote:
> > We need to look for an active PM request until the next softbarrier
> > instead of looking for the first non-PM request.  Otherwise any cause
> > of request reordering might starve the PM request(s).
> 
> Hi Christoph,
> 
> Could you share us how the starve is caused? Looks all PM
> request is run via scsi_execute(), and the caller is always
> waiting for the completion of PM request.
> 
> Especially PM request is run after SCSI device is in quiesce
> state, during that time, no normal(non-preempt) request can
> enter queue after the making quiesce safe patchset is merged.

I thought I stated that the issue is more theoretical, and the
real reason is to refactor this code for later additions.

Seems like that was only in the cover letter of the previous version,
though.

Re: [PATCH v2] block: fix peeking requests during PM

[Christoph Hellwig]

Jens, is this something worth picking up?

Re: [PATCH v2] block: fix peeking requests during PM

[Jens Axboe]

On 11/04/2017 05:40 AM, Christoph Hellwig wrote:
> Jens, is this something worth picking up?

Sure, looks clean enough. Added for 4.15.

-- 
Jens Axboe