Skip to contentSkip to conversation

blk-wbt fixes

PATCH New Jens Axboe 1 participant 4 messages Aug 21, 2018 Block layer

lore
Share & tools
lore

Jens Axboe

author maintainer
#0 lore raw

cover blk-wbt fixes

One cleanup, and two fixes:

Patch 2 - Ensure that the wait queue active check includes the necessary memory barrier.

Patch 3 - Ensure that new tasks don't supersede already waiting tasks.

Signature · 1 line
Jens Axboe
author maintainer
#1 lore raw

PATCH 1/3 blk-wbt: move disable check into get_limit()

Check it in one place, instead of in multiple places.

Signed-off-by Jens Axboe <[email protected]>
---
 block/blk-wbt.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)
block/blk-wbt.c +7 -15
index bb93c7c2b182..1dd7edce7a99 100644
@@ -449,6 +449,13 @@ static inline unsigned int get_limit(struct rq_wb *rwb, unsigned long rw)
 {
 	unsigned int limit;
 
+	/*
+	 * If we got disabled, just return UINT_MAX. This ensures that
+	 * we'll properly inc a new IO, and dec+wakeup at the end.
+	 */
+	if (!rwb_enabled(rwb))
+		return UINT_MAX;
+
 	if ((rw & REQ_OP_MASK) == REQ_OP_DISCARD)
 		return rwb->wb_background;
 

@@ -486,16 +493,6 @@ static void __wbt_wait(struct rq_wb *rwb, enum wbt_flags wb_acct,
 	struct rq_wait *rqw = get_rq_wait(rwb, wb_acct);
 	DECLARE_WAITQUEUE(wait, current);
 
-	/*
-	* inc it here even if disabled, since we'll dec it at completion.
-	* this only happens if the task was sleeping in __wbt_wait(),
-	* and someone turned it off at the same time.
-	*/
-	if (!rwb_enabled(rwb)) {
-		atomic_inc(&rqw->inflight);
-		return;
-	}
-
 	if (!waitqueue_active(&rqw->wait)
 		&& rq_wait_inc_below(rqw, get_limit(rwb, rw)))
 		return;

@@ -504,11 +501,6 @@ static void __wbt_wait(struct rq_wb *rwb, enum wbt_flags wb_acct,
 	do {
 		set_current_state(TASK_UNINTERRUPTIBLE);
 
-		if (!rwb_enabled(rwb)) {
-			atomic_inc(&rqw->inflight);
-			break;
-		}
-
 		if (rq_wait_inc_below(rqw, get_limit(rwb, rw)))
 			break;
Signature · 1 line
2.17.1
author maintainer
#2 lore raw

PATCH 2/3 blk-wbt: use wq_has_sleeper() for wq active check

In reply to Jens Axboe

We need the memory barrier before checking the list head, use the appropriate helper for this. The matching queue side memory barrier is provided by set_current_state().

Signed-off-by Jens Axboe <[email protected]>
---
 block/blk-wbt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
block/blk-wbt.c +4 -4
index 1dd7edce7a99..853e22492f4e 100644
@@ -118,7 +118,7 @@ static void rwb_wake_all(struct rq_wb *rwb)
 	for (i = 0; i < WBT_NUM_RWQ; i++) {
 		struct rq_wait *rqw = &rwb->rq_wait[i];
 
-		if (waitqueue_active(&rqw->wait))
+		if (wq_has_sleeper(&rqw->wait))
 			wake_up_all(&rqw->wait);
 	}
 }

@@ -162,7 +162,7 @@ static void __wbt_done(struct rq_qos *rqos, enum wbt_flags wb_acct)
 	if (inflight && inflight >= limit)
 		return;
 
-	if (waitqueue_active(&rqw->wait)) {
+	if (wq_has_sleeper(&rqw->wait)) {
 		int diff = limit - inflight;
 
 		if (!inflight || diff >= rwb->wb_background / 2)

@@ -493,8 +493,8 @@ static void __wbt_wait(struct rq_wb *rwb, enum wbt_flags wb_acct,
 	struct rq_wait *rqw = get_rq_wait(rwb, wb_acct);
 	DECLARE_WAITQUEUE(wait, current);
 
-	if (!waitqueue_active(&rqw->wait)
-		&& rq_wait_inc_below(rqw, get_limit(rwb, rw)))
+	if (!wq_has_sleeper(&rqw->wait) &&
+	    rq_wait_inc_below(rqw, get_limit(rwb, rw)))
 		return;
 
 	add_wait_queue_exclusive(&rqw->wait, &wait);
Signature · 1 line
2.17.1
author maintainer
#3 lore raw

PATCH 3/3 blk-wbt: fix has-sleeper queueing check

In reply to Jens Axboe

We need to do this inside the loop as well, or we can allow new IO to supersede previous IO.

Signed-off-by Jens Axboe <[email protected]>
---
 block/blk-wbt.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
block/blk-wbt.c +5 -3
index 853e22492f4e..c9358f1981fb 100644
@@ -492,16 +492,17 @@ static void __wbt_wait(struct rq_wb *rwb, enum wbt_flags wb_acct,
 {
 	struct rq_wait *rqw = get_rq_wait(rwb, wb_acct);
 	DECLARE_WAITQUEUE(wait, current);
+	bool has_sleeper;
 
-	if (!wq_has_sleeper(&rqw->wait) &&
-	    rq_wait_inc_below(rqw, get_limit(rwb, rw)))
+	has_sleeper = wq_has_sleeper(&rqw->wait);
+	if (!has_sleeper && rq_wait_inc_below(rqw, get_limit(rwb, rw)))
 		return;
 
 	add_wait_queue_exclusive(&rqw->wait, &wait);
 	do {
 		set_current_state(TASK_UNINTERRUPTIBLE);
 
-		if (rq_wait_inc_below(rqw, get_limit(rwb, rw)))
+		if (!has_sleeper && rq_wait_inc_below(rqw, get_limit(rwb, rw)))
 			break;
 
 		if (lock) {

@@ -510,6 +511,7 @@ static void __wbt_wait(struct rq_wb *rwb, enum wbt_flags wb_acct,
 			spin_lock_irq(lock);
 		} else
 			io_schedule();
+		has_sleeper = false;
 	} while (1);
 
 	__set_current_state(TASK_RUNNING);
Signature · 1 line
2.17.1