From: Chengming Zhou zhouchengming@bytedance.com
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: 68497092bde9 ("block: make queue stat accounting a reference") Cc: stable@vger.kernel.org # v5.17+ Signed-off-by: Chengming Zhou zhouchengming@bytedance.com Acked-by: Tejun Heo tj@kernel.org --- block/blk-stat.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/blk-stat.c b/block/blk-stat.c index 74a1a8c32d86..bc7e0ed81642 100644 --- a/block/blk-stat.c +++ b/block/blk-stat.c @@ -190,7 +190,7 @@ void blk_stat_disable_accounting(struct request_queue *q) unsigned long 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); spin_unlock_irqrestore(&q->stats->lock, flags); } @@ -201,7 +201,7 @@ void blk_stat_enable_accounting(struct request_queue *q) unsigned long 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); spin_unlock_irqrestore(&q->stats->lock, flags); }
On Thu, 13 Apr 2023 14:28:04 +0800, chengming.zhou@linux.dev wrote:
We need to set QUEUE_FLAG_STATS for two cases:
- blk_stat_enable_accounting()
- blk_stat_add_callback()
So we should clear it only when ((q->stats->accounting == 0) && list_empty(&q->stats->callbacks)).
[...]
Applied, thanks!
[1/2] blk-stat: fix QUEUE_FLAG_STATS clear commit: 20de765f6d9da0c47b756429c60b41063b990a10 [2/2] blk-throttle: only enable blk-stat when BLK_DEV_THROTTLING_LOW commit: 8e15dfbd9ae21e518979e3823e335073e725f445
Best regards,
linux-stable-mirror@lists.linaro.org