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

[guzebing <guzebing1612@gmail.com>]

Gentle ping.

This patch addresses a case where writing the current default WBT
latency to queue/wbt_lat_usec returns success but does not enable WBT
when WBT was disabled by default, for example after selecting BFQ.

If this is intended behavior, then perhaps no fix is needed. Please let
me know if I missed the intended semantics here.

Thanks,
Guzebing

On 6/21/26 9:40 AM, guzebing wrote:
> From: Guzebing <guzebing1612@gmail.com>
> 
> 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@gmail.com>
> ---
> 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);
>