@samitouri / QOSamiQemu / commits / 16f94ef4c6

block/qapi: take stats->lock when reading BlockAcctStats for query-blockstats

bdrv_query_blk_stats() reads BlockAcctStats's counters, latency histogram, and per-interval TimedAverage stats without stats->lock, while block_account_one_io() updates the same fields under that lock from an iothread. timed_average_min()/max()/avg() make this worse than a stale read: they call check_expirations(), which can reset a window's sum/count/min/max -- a write, not just a read -- so this is a genuine race with a concurrent writer, not merely a slower reader like the scalar counters. Take stats->lock for the whole call, both to close the race and to make the returned snapshot internally consistent (previously each field could reflect a different instant relative to concurrent updates). block_acct_queue_depth() used to take the lock itself on every call; since bdrv_query_blk_stats() is its only caller and now already holds the lock, that would self-deadlock. Make it require the caller to hold stats->lock instead (documented and asserted). Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Kevin Wolf <kwolf@redhat.com> CC: Hanna Reitz <hreitz@redhat.com> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> Message-ID: <20260724111311.4086859-3-den@openvz.org> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Denis V. Lunev committed Jul 24, 2026 at 13:13 UTC 16f94ef4c6ce3e7ca6b02b796580e2cfc41e9789

3 files changed +5 -2

block/accounting.c

+1 -2

@@ -318,10 +318,9 @@ double block_acct_queue_depth(BlockAcctTimedStats *stats,

318 uint64_t sum, elapsed;

319

320 assert(type < BLOCK_MAX_IOTYPE);

321 + assert(qemu_mutex_trylock(&stats->stats->lock) == -EBUSY);

322

322 - qemu_mutex_lock(&stats->stats->lock);

323 sum = timed_average_sum(&stats->latency[type], &elapsed);

324 - qemu_mutex_unlock(&stats->stats->lock);

324

325 return (double) sum / elapsed;

326 }

block/qapi.c

@@ -535,6 +535,8 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)

535 BlockAcctTimedStats *ts = NULL;

536 BlockLatencyHistogram *hgram;

537

538 + qemu_mutex_lock(&stats->lock);

539 +

540 ds->rd_bytes = stats->nr_bytes[BLOCK_ACCT_READ];

541 ds->wr_bytes = stats->nr_bytes[BLOCK_ACCT_WRITE];

542 ds->zone_append_bytes = stats->nr_bytes[BLOCK_ACCT_ZONE_APPEND];

@@ -624,6 +626,7 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)

626 = bdrv_latency_histogram_stats(&hgram[BLOCK_ACCT_ZONE_APPEND]);

627 ds->flush_latency_histogram

628 = bdrv_latency_histogram_stats(&hgram[BLOCK_ACCT_FLUSH]);

629 + qemu_mutex_unlock(&stats->lock);

630 }

631

632 static BlockStats * GRAPH_RDLOCK

include/block/accounting.h

@@ -116,6 +116,7 @@ void block_acct_invalid(BlockAcctStats *stats, enum BlockAcctType type);

116 void block_acct_merge_done(BlockAcctStats *stats, enum BlockAcctType type,

117 int num_requests);

118 int64_t block_acct_idle_time_ns(BlockAcctStats *stats);

119 +/* Caller must hold stats->stats->lock. */

120 double block_acct_queue_depth(BlockAcctTimedStats *stats,

121 enum BlockAcctType type);

122 int block_latency_histogram_set(BlockAcctStats *stats, enum BlockAcctType type,