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

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

[Tang Yizhou]

From: Tang Yizhou <yizhou.tang@shopee.com>

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 <yukuai@fygo.io>
Reviewed-by: Guzebing <guzebing1612@gmail.com>
Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>
---
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

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

[Tao Cui]

在 2026/8/21 00:15, Tang Yizhou 写道:
> From: Tang Yizhou <yizhou.tang@shopee.com>
> 
> 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 <yukuai@fygo.io>
> Reviewed-by: Guzebing <guzebing1612@gmail.com>
> Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>
> ---
> 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;
>  	}

Reviewed-by: Tao Cui <cuitao@kylinos.cn>