Linux block layer
 help / color / mirror / Atom feed
* [PATCH] block: Make WBT latency writes honor enable state
@ 2026-06-21  1:40 guzebing
  2026-07-01 23:27 ` guzebing
  2026-07-02  1:07 ` Jens Axboe
  0 siblings, 2 replies; 3+ messages in thread
From: guzebing @ 2026-06-21  1:40 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Guzebing, linux-block, linux-kernel

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);
 
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] block: Make WBT latency writes honor enable state
  2026-06-21  1:40 [PATCH] block: Make WBT latency writes honor enable state guzebing
@ 2026-07-01 23:27 ` guzebing
  2026-07-02  1:07 ` Jens Axboe
  1 sibling, 0 replies; 3+ messages in thread
From: guzebing @ 2026-07-01 23:27 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, linux-kernel

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);
>  


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] block: Make WBT latency writes honor enable state
  2026-06-21  1:40 [PATCH] block: Make WBT latency writes honor enable state guzebing
  2026-07-01 23:27 ` guzebing
@ 2026-07-02  1:07 ` Jens Axboe
  1 sibling, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2026-07-02  1:07 UTC (permalink / raw)
  To: guzebing; +Cc: linux-block, linux-kernel


On Sun, 21 Jun 2026 09:40:30 +0800, guzebing wrote:
> 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.
> 
> [...]

Applied, thanks!

[1/1] block: Make WBT latency writes honor enable state
      commit: 1e56f30a73f304fe26a272742c398aedd88a1a6c

Best regards,
-- 
Jens Axboe




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-02  1:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-21  1:40 [PATCH] block: Make WBT latency writes honor enable state guzebing
2026-07-01 23:27 ` guzebing
2026-07-02  1:07 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox