@samitouri / QOSamiQemu / commits / 842c89281a

tests/unit: add reproducer for BlockAcctStats histogram locking race

block_latency_histogram_set() and block_latency_histograms_clear() replace BlockLatencyHistogram's nbins/boundaries/bins without taking stats->lock, while block_account_one_io() reads those same fields under that lock from whatever iothread completes the I/O. Add a test that races two real threads against block_latency_histogram_set() and block_acct_start()/block_acct_done() on the same BlockAcctStats. Applied here it passes, since the previous two commits already take the lock; reverting them locally reproduces the abort this series fixes, in about a second. 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-4-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 842c89281a20c3ea783609c5ac9da226e017e490
2 files changed +116
tests/unit/meson.build
+1
@@ -75,6 +75,7 @@ if have_block
75 'test-blockjob': [testblock],
76 'test-blockjob-txn': [testblock],
77 'test-block-backend': [testblock],
78 + 'test-block-accounting': [testblock],
79 'test-block-iothread': [testblock],
80 'test-write-threshold': [testblock],
81 'test-crypto-hash': [crypto],
tests/unit/test-block-accounting.c new
+115
@@ -0,0 +1,115 @@
1 +/*
2 + * SPDX-License-Identifier: GPL-2.0-or-later
3 + *
4 + * BlockAcctStats latency histogram locking regression test
5 + *
6 + * Copyright (c) 2026 Virtuozzo International GmbH.
7 + *
8 + * Regression test for missing stats->lock in
9 + * block_latency_histogram_set()/block_latency_histograms_clear(),
10 + * racing block_account_one_io() reading the same fields from an
11 + * iothread. Aborts reliably before the fix, passes after it.
12 + */
13 +
14 +#include "qemu/osdep.h"
15 +#include "block/block.h"
16 +#include "block/accounting.h"
17 +#include "system/block-backend.h"
18 +#include "system/block-backend-io.h"
19 +#include "qapi/error.h"
20 +#include "qemu/main-loop.h"
21 +#include "qemu/thread.h"
22 +
23 +#define RACE_DURATION_MS 2000
24 +#define NUM_READER_THREADS 8
25 +
26 +static bool stop_workers;
27 +
28 +/*
29 + * Different bin counts, so the writer's g_free()/g_new() churn can be
30 + * caught mid-update. Values are small enough (nanoseconds) that plain
31 + * back-to-back start/done calls exercise every bin without sleeping.
32 + */
33 +static uint64List boundaries_a[] = {
34 + { .next = &boundaries_a[1], .value = 1000 },
35 + { .next = &boundaries_a[2], .value = 5000 },
36 + { .next = NULL, .value = 50000 },
37 +};
38 +
39 +static uint64List boundaries_b[] = {
40 + { .next = &boundaries_b[1], .value = 800 },
41 + { .next = &boundaries_b[2], .value = 3000 },
42 + { .next = &boundaries_b[3], .value = 20000 },
43 + { .next = NULL, .value = 200000 },
44 +};
45 +
46 +static void *writer_thread(void *opaque)
47 +{
48 + BlockAcctStats *stats = opaque;
49 +
50 + while (!qatomic_read(&stop_workers)) {
51 + block_latency_histogram_set(stats, BLOCK_ACCT_READ, boundaries_a);
52 + block_latency_histogram_set(stats, BLOCK_ACCT_READ, boundaries_b);
53 + block_latency_histograms_clear(stats);
54 + }
55 +
56 + return NULL;
57 +}
58 +
59 +static void *reader_thread(void *opaque)
60 +{
61 + BlockAcctStats *stats = opaque;
62 +
63 + while (!qatomic_read(&stop_workers)) {
64 + BlockAcctCookie cookie;
65 +
66 + block_acct_start(stats, &cookie, 4096, BLOCK_ACCT_READ);
67 + block_acct_done(stats, &cookie);
68 + }
69 +
70 + return NULL;
71 +}
72 +
73 +static void test_latency_histogram_race(void)
74 +{
75 + BlockBackend *blk = blk_new(qemu_get_aio_context(),
76 + BLK_PERM_ALL, BLK_PERM_ALL);
77 + BlockAcctStats *stats = blk_get_stats(blk);
78 + QemuThread writer, readers[NUM_READER_THREADS];
79 + int i;
80 +
81 + /* Histogram has to be enabled (bins != NULL) before racing it. */
82 + g_assert(block_latency_histogram_set(stats, BLOCK_ACCT_READ,
83 + boundaries_a) == 0);
84 +
85 + stop_workers = false;
86 + qemu_thread_create(&writer, "hist-writer", writer_thread, stats,
87 + QEMU_THREAD_JOINABLE);
88 + for (i = 0; i < NUM_READER_THREADS; i++) {
89 + qemu_thread_create(&readers[i], "hist-reader", reader_thread, stats,
90 + QEMU_THREAD_JOINABLE);
91 + }
92 +
93 + g_usleep(RACE_DURATION_MS * 1000);
94 + qatomic_set(&stop_workers, true);
95 +
96 + qemu_thread_join(&writer);
97 + for (i = 0; i < NUM_READER_THREADS; i++) {
98 + qemu_thread_join(&readers[i]);
99 + }
100 +
101 + blk_unref(blk);
102 +}
103 +
104 +int main(int argc, char **argv)
105 +{
106 + bdrv_init();
107 + qemu_init_main_loop(&error_abort);
108 +
109 + g_test_init(&argc, &argv, NULL);
110 +
111 + g_test_add_func("/block-accounting/latency_histogram_race",
112 + test_latency_histogram_race);
113 +
114 + return g_test_run();
115 +}