[PATCH v2] blk-wbt: Always change enable_state to MANUAL when setting latency

Tang Yizhou <[email protected]>
Newsgroups org.kernel.vger.linux-block,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
From: Tang Yizhou <[email protected]>

Commit 1e56f30a73f3 ("block: Make WBT latency writes honor enable
state") compares only the boolean enabled state in the no-op check.
Writing the current latency value while WBT is in WBT_STATE_ON_DEFAULT
is therefore still skipped, and enable_state is not changed to
WBT_STATE_ON_MANUAL. A subsequent elevator switch to BFQ then disables
WBT through wbt_disable_default(), silently discarding the explicit
sysfs setting:

  # enable_state = WBT_STATE_ON_DEFAULT, min_lat_nsec = 2000000
  echo 2000 > /sys/block/nullb0/queue/wbt_lat_usec  # skipped as a no-op
  echo bfq > /sys/block/nullb0/queue/scheduler
  cat /sys/block/nullb0/queue/wbt_lat_usec          # 0, WBT disabled

Skip the update only if the stored latency matches and enable_state
already equals the exact target state: WBT_STATE_ON_MANUAL for a
non-zero value, WBT_STATE_OFF_MANUAL for zero.

A write that passes the check then reaches wbt_set_min_lat(), which
performs the actual transition to the MANUAL state.

Rename the helper to wbt_lat_changed() to improve readability.

Fixes: 1e56f30a73f3 ("block: Make WBT latency writes honor enable state")
Reviewed-by: Yu Kuai <[email protected]>
Reviewed-by: Guzebing <[email protected]>
Signed-off-by: Tang Yizhou <[email protected]>
---
v2: Pick Yuai and Guzebing's Reviewed-by tag.
 block/blk-wbt.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index 953d400fd013..4f12a064286e 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -813,7 +813,14 @@ static void wbt_queue_depth_changed(struct rq_qos *rqos)
 	wbt_update_limits(RQWB(rqos));
 }
 
-static bool wbt_set_lat_changed(struct request_queue *q, u64 val)
+/*
+ * Return true if writing @val would change the WBT state:
+ * 1) @val differs from the stored min_lat_nsec, or
+ * 2) @val matches, but enable_state is not the corresponding manual state
+ *    (WBT_STATE_ON_MANUAL for non-zero @val, WBT_STATE_OFF_MANUAL for 0),
+ *    so the write still has to update enable_state.
+ */
+static bool wbt_lat_changed(struct request_queue *q, u64 val)
 {
 	struct rq_qos *rqos = wbt_rq_qos(q);
 	struct rq_wb *rwb;
@@ -825,7 +832,8 @@ static bool wbt_set_lat_changed(struct request_queue *q, u64 val)
 	if (rwb->min_lat_nsec != val)
 		return true;
 
-	return rwb_enabled(rwb) != !!val;
+	return rwb->enable_state !=
+	       (val ? WBT_STATE_ON_MANUAL : WBT_STATE_OFF_MANUAL);
 }
 
 static void wbt_exit(struct rq_qos *rqos)
@@ -1021,7 +1029,7 @@ int wbt_set_lat(struct gendisk *disk, s64 val)
 		val *= 1000ULL;
 
 	mutex_lock(&disk->rqos_state_mutex);
-	if (!wbt_set_lat_changed(q, val)) {
+	if (!wbt_lat_changed(q, val)) {
 		mutex_unlock(&disk->rqos_state_mutex);
 		goto out;
 	}
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.