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

在 2026/8/21 00:15, Tang Yizhou 写道:
> 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;
>  	}

Reviewed-by: Tao Cui <[email protected]>