Commit 20de765f authored by Chengming Zhou's avatar Chengming Zhou Committed by Jens Axboe

blk-stat: fix QUEUE_FLAG_STATS clear

We need to set QUEUE_FLAG_STATS for two cases:
1. blk_stat_enable_accounting()
2. blk_stat_add_callback()

So we should clear it only when ((q->stats->accounting == 0) &&
list_empty(&q->stats->callbacks)).

blk_stat_disable_accounting() only check if q->stats->accounting
is 0 before clear the flag, this patch fix it.

Also add list_empty(&q->stats->callbacks)) check when enable, or
the flag is already set.

The bug can be reproduced on kernel without BLK_DEV_THROTTLING
(since it unconditionally enable accounting, see the next patch).

  # cat /sys/block/sr0/queue/scheduler
  none mq-deadline [bfq]

  # cat /sys/kernel/debug/block/sr0/state
  SAME_COMP|IO_STAT|INIT_DONE|STATS|REGISTERED|NOWAIT|30

  # echo none > /sys/block/sr0/queue/scheduler

  # cat /sys/kernel/debug/block/sr0/state
  SAME_COMP|IO_STAT|INIT_DONE|REGISTERED|NOWAIT

  # cat /sys/block/sr0/queue/wbt_lat_usec
  75000

We can see that after changing elevator from "bfq" to "none",
"STATS" flag is lost even though WBT callback still need it.

Fixes: 68497092 ("block: make queue stat accounting a reference")
Cc: <stable@vger.kernel.org> # v5.17+
Signed-off-by: default avatarChengming Zhou <zhouchengming@bytedance.com>
Acked-by: default avatarTejun Heo <tj@kernel.org>
Link: https://lore.kernel.org/r/20230413062805.2081970-1-chengming.zhou@linux.devSigned-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent a13696b8
...@@ -190,7 +190,7 @@ void blk_stat_disable_accounting(struct request_queue *q) ...@@ -190,7 +190,7 @@ void blk_stat_disable_accounting(struct request_queue *q)
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&q->stats->lock, flags); spin_lock_irqsave(&q->stats->lock, flags);
if (!--q->stats->accounting) if (!--q->stats->accounting && list_empty(&q->stats->callbacks))
blk_queue_flag_clear(QUEUE_FLAG_STATS, q); blk_queue_flag_clear(QUEUE_FLAG_STATS, q);
spin_unlock_irqrestore(&q->stats->lock, flags); spin_unlock_irqrestore(&q->stats->lock, flags);
} }
...@@ -201,7 +201,7 @@ void blk_stat_enable_accounting(struct request_queue *q) ...@@ -201,7 +201,7 @@ void blk_stat_enable_accounting(struct request_queue *q)
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&q->stats->lock, flags); spin_lock_irqsave(&q->stats->lock, flags);
if (!q->stats->accounting++) if (!q->stats->accounting++ && list_empty(&q->stats->callbacks))
blk_queue_flag_set(QUEUE_FLAG_STATS, q); blk_queue_flag_set(QUEUE_FLAG_STATS, q);
spin_unlock_irqrestore(&q->stats->lock, flags); spin_unlock_irqrestore(&q->stats->lock, flags);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment