diff options
| author | Denis V. Lunev <den@openvz.org> | 2026-07-24 13:13:10 +0200 |
|---|---|---|
| committer | Kevin Wolf <kwolf@redhat.com> | 2026-08-03 10:42:29 +0200 |
| commit | 16f94ef4c6ce3e7ca6b02b796580e2cfc41e9789 (patch) | |
| tree | 4d454e415bfa89b0d938f2397ca0d46bd9f3143f /block | |
| parent | 5b0ba385a024ad92c63c175317cd9374de2e6165 (diff) | |
| download | qemu-16f94ef4c6ce3e7ca6b02b796580e2cfc41e9789.tar.gz qemu-16f94ef4c6ce3e7ca6b02b796580e2cfc41e9789.zip | |
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>
Diffstat (limited to 'block')
| -rw-r--r-- | block/accounting.c | 3 | ||||
| -rw-r--r-- | block/qapi.c | 3 |
2 files changed, 4 insertions, 2 deletions
diff --git a/block/accounting.c b/block/accounting.c index 6e06c7609e..038af37017 100644 --- a/block/accounting.c +++ b/block/accounting.c @@ -318,10 +318,9 @@ double block_acct_queue_depth(BlockAcctTimedStats *stats, uint64_t sum, elapsed; assert(type < BLOCK_MAX_IOTYPE); + assert(qemu_mutex_trylock(&stats->stats->lock) == -EBUSY); - qemu_mutex_lock(&stats->stats->lock); sum = timed_average_sum(&stats->latency[type], &elapsed); - qemu_mutex_unlock(&stats->stats->lock); return (double) sum / elapsed; } diff --git a/block/qapi.c b/block/qapi.c index eabfbfc258..1dfac51091 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -535,6 +535,8 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk) BlockAcctTimedStats *ts = NULL; BlockLatencyHistogram *hgram; + qemu_mutex_lock(&stats->lock); + ds->rd_bytes = stats->nr_bytes[BLOCK_ACCT_READ]; ds->wr_bytes = stats->nr_bytes[BLOCK_ACCT_WRITE]; 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) = bdrv_latency_histogram_stats(&hgram[BLOCK_ACCT_ZONE_APPEND]); ds->flush_latency_histogram = bdrv_latency_histogram_stats(&hgram[BLOCK_ACCT_FLUSH]); + qemu_mutex_unlock(&stats->lock); } static BlockStats * GRAPH_RDLOCK |