From: "Denis V. Lunev" <den@openvz.org>
To: qemu-block@nongnu.org, qemu-devel@nongnu.org
Cc: den@openvz.org, Kevin Wolf <kwolf@redhat.com>,
Hanna Reitz <hreitz@redhat.com>,
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Subject: [PATCH 3/3] tests/unit: add reproducer for BlockAcctStats histogram locking race
Date: Thu, 23 Jul 2026 14:37:45 +0200 [thread overview]
Message-ID: <20260723123745.3294227-4-den@openvz.org> (raw)
In-Reply-To: <20260723123745.3294227-1-den@openvz.org>
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>
---
tests/unit/meson.build | 1 +
tests/unit/test-block-accounting.c | 114 +++++++++++++++++++++++++++++
2 files changed, 115 insertions(+)
create mode 100644 tests/unit/test-block-accounting.c
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 5ba6b1a230..dc3fb954c0 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -75,6 +75,7 @@ if have_block
'test-blockjob': [testblock],
'test-blockjob-txn': [testblock],
'test-block-backend': [testblock],
+ 'test-block-accounting': [testblock],
'test-block-iothread': [testblock],
'test-write-threshold': [testblock],
'test-crypto-hash': [crypto],
diff --git a/tests/unit/test-block-accounting.c b/tests/unit/test-block-accounting.c
new file mode 100644
index 0000000000..e749b73749
--- /dev/null
+++ b/tests/unit/test-block-accounting.c
@@ -0,0 +1,114 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * BlockAcctStats latency histogram locking regression test
+ *
+ * Copyright (c) 2026 Virtuozzo International GmbH.
+ *
+ * Regression test for missing stats->lock in
+ * block_latency_histogram_set()/block_latency_histograms_clear(),
+ * racing block_account_one_io() reading the same fields from an
+ * iothread. Aborts reliably before the fix, passes after it.
+ */
+
+#include "qemu/osdep.h"
+#include "block/block.h"
+#include "block/accounting.h"
+#include "system/block-backend.h"
+#include "system/block-backend-io.h"
+#include "qapi/error.h"
+#include "qemu/main-loop.h"
+#include "qemu/thread.h"
+
+#define RACE_DURATION_MS 2000
+#define NUM_READER_THREADS 8
+
+static bool stop_workers;
+
+/*
+ * Different bin counts, so the writer's g_free()/g_new() churn can be
+ * caught mid-update. Values are small enough (nanoseconds) that plain
+ * back-to-back start/done calls exercise every bin without sleeping.
+ */
+static uint64List boundaries_a[] = {
+ { .next = &boundaries_a[1], .value = 1000 },
+ { .next = &boundaries_a[2], .value = 5000 },
+ { .next = NULL, .value = 50000 },
+};
+
+static uint64List boundaries_b[] = {
+ { .next = &boundaries_b[1], .value = 800 },
+ { .next = &boundaries_b[2], .value = 3000 },
+ { .next = &boundaries_b[3], .value = 20000 },
+ { .next = NULL, .value = 200000 },
+};
+
+static void *writer_thread(void *opaque)
+{
+ BlockAcctStats *stats = opaque;
+
+ while (!qatomic_read(&stop_workers)) {
+ block_latency_histogram_set(stats, BLOCK_ACCT_READ, boundaries_a);
+ block_latency_histogram_set(stats, BLOCK_ACCT_READ, boundaries_b);
+ }
+
+ return NULL;
+}
+
+static void *reader_thread(void *opaque)
+{
+ BlockAcctStats *stats = opaque;
+
+ while (!qatomic_read(&stop_workers)) {
+ BlockAcctCookie cookie;
+
+ block_acct_start(stats, &cookie, 4096, BLOCK_ACCT_READ);
+ block_acct_done(stats, &cookie);
+ }
+
+ return NULL;
+}
+
+static void test_latency_histogram_race(void)
+{
+ BlockBackend *blk = blk_new(qemu_get_aio_context(),
+ BLK_PERM_ALL, BLK_PERM_ALL);
+ BlockAcctStats *stats = blk_get_stats(blk);
+ QemuThread writer, readers[NUM_READER_THREADS];
+ int i;
+
+ /* Histogram has to be enabled (bins != NULL) before racing it. */
+ g_assert(block_latency_histogram_set(stats, BLOCK_ACCT_READ,
+ boundaries_a) == 0);
+
+ stop_workers = false;
+ qemu_thread_create(&writer, "hist-writer", writer_thread, stats,
+ QEMU_THREAD_JOINABLE);
+ for (i = 0; i < NUM_READER_THREADS; i++) {
+ qemu_thread_create(&readers[i], "hist-reader", reader_thread, stats,
+ QEMU_THREAD_JOINABLE);
+ }
+
+ g_usleep(RACE_DURATION_MS * 1000);
+ qatomic_set(&stop_workers, true);
+
+ qemu_thread_join(&writer);
+ for (i = 0; i < NUM_READER_THREADS; i++) {
+ qemu_thread_join(&readers[i]);
+ }
+
+ blk_unref(blk);
+}
+
+int main(int argc, char **argv)
+{
+ bdrv_init();
+ qemu_init_main_loop(&error_abort);
+
+ g_test_init(&argc, &argv, NULL);
+
+ g_test_add_func("/block-accounting/latency_histogram_race",
+ test_latency_histogram_race);
+
+ return g_test_run();
+}
--
2.53.0
next prev parent reply other threads:[~2026-07-23 12:38 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-23 12:37 [PATCH 0/3] block/accounting: fix unlocked latency histogram reconfiguration races Denis V. Lunev
2026-07-23 12:37 ` [PATCH 1/3] block/accounting: take stats->lock in latency histogram setters Denis V. Lunev
2026-07-23 12:37 ` [PATCH 2/3] block/qapi: take stats->lock when reading BlockAcctStats for query-blockstats Denis V. Lunev
2026-07-23 14:02 ` Andrey Drobyshev
2026-07-23 12:37 ` Denis V. Lunev [this message]
2026-07-23 14:04 ` [PATCH 3/3] tests/unit: add reproducer for BlockAcctStats histogram locking race Andrey Drobyshev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260723123745.3294227-4-den@openvz.org \
--to=den@openvz.org \
--cc=hreitz@redhat.com \
--cc=kwolf@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@yandex-team.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.