[PATCH] block: Make WBT latency writes honor enable state

[guzebing]

From: Guzebing <guzebing1612@xxxxxxxxx>

queue/wbt_lat_usec controls both the stored WBT latency target and the
effective WBT enable state.

The old no-op check skipped updates whenever the converted latency
matched the stored min_lat_nsec. That check ignored whether the current
WBT state already matched the state requested by the write. For a queue
disabled by default, attempting to enable WBT by writing the default
value through sysfs could return success while the enable state was left
unchanged.

Treat a write as a no-op only when both the stored latency and the
effective WBT enabled state already match the converted value.

Signed-off-by: Guzebing <guzebing1612@xxxxxxxxx>
---
Background:

The issue can be reproduced on an NVMe namespace when BFQ is available:

  echo bfq > /sys/block/nvme0n1/queue/scheduler
  cat /sys/block/nvme0n1/queue/wbt_lat_usec
  echo 2000 > /sys/block/nvme0n1/queue/wbt_lat_usec
  cat /sys/block/nvme0n1/queue/wbt_lat_usec

After BFQ selects the queue, WBT is disabled by default.  On a
non-rotational NVMe namespace the stored default latency remains
2000000 nsec, while the sysfs file reports 0 because the effective WBT
state is disabled:

  queue/wbt_lat_usec = 0
  debugfs enabled = 3
  debugfs min_lat_nsec = 2000000

Writing the default value succeeds, but the old no-op check skips the
state transition because min_lat_nsec already matches the converted
value:

  echo 2000 > /sys/block/nvme0n1/queue/wbt_lat_usec
  # echo returns success, but:
  queue/wbt_lat_usec = 0
  debugfs enabled = 3
  debugfs min_lat_nsec = 2000000

As a control, writing a non-default value first does work:

  echo 5000 > /sys/block/nvme0n1/queue/wbt_lat_usec
  queue/wbt_lat_usec = 5000
  debugfs enabled = 2
  debugfs min_lat_nsec = 5000000

Writing the default value after that also works, because the stored
latency changes from 5000000 nsec back to 2000000 nsec:

  echo 2000 > /sys/block/nvme0n1/queue/wbt_lat_usec
  queue/wbt_lat_usec = 2000
  debugfs enabled = 2
  debugfs min_lat_nsec = 2000000

With this patch, writing the default value after BFQ default-disables
WBT also re-enables WBT as expected:

  queue/wbt_lat_usec = 2000
  debugfs enabled = 2
  debugfs min_lat_nsec = 2000000

 block/blk-wbt.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index dcc2438ca16dc..953d400fd0137 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -813,6 +813,21 @@ 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)
+{
+	struct rq_qos *rqos = wbt_rq_qos(q);
+	struct rq_wb *rwb;
+
+	if (!rqos)
+		return true;
+
+	rwb = RQWB(rqos);
+	if (rwb->min_lat_nsec != val)
+		return true;
+
+	return rwb_enabled(rwb) != !!val;
+}
+
 static void wbt_exit(struct rq_qos *rqos)
 {
 	struct rq_wb *rwb = RQWB(rqos);
@@ -1005,8 +1020,12 @@ int wbt_set_lat(struct gendisk *disk, s64 val)
 	else if (val >= 0)
 		val *= 1000ULL;
 
-	if (wbt_get_min_lat(q) == val)
+	mutex_lock(&disk->rqos_state_mutex);
+	if (!wbt_set_lat_changed(q, val)) {
+		mutex_unlock(&disk->rqos_state_mutex);
 		goto out;
+	}
+	mutex_unlock(&disk->rqos_state_mutex);
 
 	blk_mq_quiesce_queue(q);
 
-- 
2.20.1