* [RFC PATCH 0/2] async: rework async event API for replay
@ 2024-08-15 13:28 Nicholas Piggin
2024-08-15 13:28 ` [RFC PATCH 1/2] " Nicholas Piggin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Nicholas Piggin @ 2024-08-15 13:28 UTC (permalink / raw)
To: qemu-devel
Cc: Nicholas Piggin, Pavel Dovgalyuk, Philippe Mathieu-Daudé,
Richard Henderson, Alex Bennée, Paolo Bonzini, John Snow,
Cleber Rosa, Wainer dos Santos Moschetta, Beraldo Leal,
Michael Tokarev, Michael S . Tsirkin
Continuing the conversation from the thread about record/replay
virtio fix. Here is a sketch of how we could improve the naming
convention so users of bh don't have to know about record/replay.
Thanks,
Nick
Nicholas Piggin (2):
async: rework async event API for replay
async: add debugging assertions for record/replay in bh APIs
docs/devel/replay.rst | 7 ++--
include/block/aio.h | 35 +++++++++++++++--
include/sysemu/replay.h | 2 +-
block.c | 4 +-
block/block-backend.c | 24 +++++++-----
block/io.c | 5 ++-
block/iscsi.c | 5 ++-
block/nfs.c | 10 +++--
block/null.c | 4 +-
block/nvme.c | 8 ++--
hw/ide/core.c | 7 ++--
hw/scsi/scsi-bus.c | 6 +--
monitor/monitor.c | 2 +-
monitor/qmp.c | 5 ++-
qapi/qmp-dispatch.c | 4 +-
replay/replay-events.c | 25 ++++++------
stubs/replay-tools.c | 2 +-
util/aio-wait.c | 2 +-
util/async.c | 63 ++++++++++++++++++++++++++++--
util/main-loop.c | 2 +-
util/thread-pool.c | 8 ++--
scripts/block-coroutine-wrapper.py | 2 +-
22 files changed, 164 insertions(+), 68 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 1/2] async: rework async event API for replay
2024-08-15 13:28 [RFC PATCH 0/2] async: rework async event API for replay Nicholas Piggin
@ 2024-08-15 13:28 ` Nicholas Piggin
2024-08-15 13:28 ` [RFC PATCH 2/2] async: add debugging assertions for record/replay in bh APIs Nicholas Piggin
2024-08-15 15:30 ` [RFC PATCH 0/2] async: rework async event API for replay Michael S. Tsirkin
2 siblings, 0 replies; 6+ messages in thread
From: Nicholas Piggin @ 2024-08-15 13:28 UTC (permalink / raw)
To: qemu-devel
Cc: Nicholas Piggin, Pavel Dovgalyuk, Philippe Mathieu-Daudé,
Richard Henderson, Alex Bennée, Paolo Bonzini, John Snow,
Cleber Rosa, Wainer dos Santos Moschetta, Beraldo Leal,
Michael Tokarev, Michael S . Tsirkin
Replace the functions replay_bh_schedule_event() and
replay_bh_schedule_oneshot_event() with qemu_bh_schedule_event() and
aio_bh_schedule_oneshot_event(), respectively.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
docs/devel/replay.rst | 7 ++++---
include/block/aio.h | 35 +++++++++++++++++++++++++++++++----
include/sysemu/replay.h | 2 +-
block/block-backend.c | 24 ++++++++++++++----------
block/io.c | 5 +++--
block/iscsi.c | 5 +++--
block/nfs.c | 10 ++++++----
block/null.c | 4 ++--
block/nvme.c | 8 +++++---
hw/ide/core.c | 5 +++--
stubs/replay-tools.c | 2 +-
util/async.c | 40 ++++++++++++++++++++++++++++++++++++++--
12 files changed, 111 insertions(+), 36 deletions(-)
diff --git a/docs/devel/replay.rst b/docs/devel/replay.rst
index effd856f0c..ea1ee0a008 100644
--- a/docs/devel/replay.rst
+++ b/docs/devel/replay.rst
@@ -171,9 +171,10 @@ Bottom halves
-------------
Bottom half callbacks, that affect the guest state, should be invoked through
-``replay_bh_schedule_event`` or ``replay_bh_schedule_oneshot_event`` functions.
-Their invocations are saved in record mode and synchronized with the existing
-log in replay mode.
+``qemu_bh_schedule_event`` or ``aio_bh_schedule_oneshot_event`` functions
+the with the appropriate clock type (e.g., QEMU_CLOCK_VIRTUAL. Their
+invocations are saved in record mode and synchronized with the existing log in
+replay mode.
Disk I/O events are completely deterministic in our model, because
in both record and replay modes we start virtual machine from the same
diff --git a/include/block/aio.h b/include/block/aio.h
index 4ee81936ed..23c5543506 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -285,17 +285,30 @@ void aio_context_unref(AioContext *ctx);
* @name: A human-readable identifier for debugging purposes.
*/
void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
- const char *name);
+ const char *name, QEMUClockType clock_type);
/**
- * aio_bh_schedule_oneshot: Allocate a new bottom half structure that will run
- * only once and as soon as possible.
+ * aio_bh_schedule_oneshot_event: Allocate a new bottom half structure that
+ * will run only once and as soon as possible.
*
* A convenience wrapper for aio_bh_schedule_oneshot_full() that uses cb as the
* name string.
*/
+#define aio_bh_schedule_oneshot_event(ctx, cb, opaque, clock_type) \
+ aio_bh_schedule_oneshot_full((ctx), (cb), (opaque), (stringify(cb)), \
+ clock_type)
+
+/**
+ * aio_bh_schedule_oneshot: Allocate a new bottom half structure that will run
+ * only once and as soon as possible.
+ *
+ * A legacy wrapper for aio_bh_schedule_oneshot_event() that uses realtime
+ * as the clock type. Callers should specify the clock time in order to be
+ * compatible with record/replay.
+ */
#define aio_bh_schedule_oneshot(ctx, cb, opaque) \
- aio_bh_schedule_oneshot_full((ctx), (cb), (opaque), (stringify(cb)))
+ aio_bh_schedule_oneshot_full((ctx), (cb), (opaque), (stringify(cb)), \
+ QEMU_CLOCK_REALTIME)
/**
* aio_bh_new_full: Allocate a new bottom half structure.
@@ -377,6 +390,20 @@ void aio_bh_call(QEMUBH *bh);
*/
int aio_bh_poll(AioContext *ctx);
+/**
+ * qemu_bh_schedule_event: Schedule a bottom half.
+ *
+ * Scheduling a bottom half interrupts the main loop and causes the
+ * execution of the callback that was passed to qemu_bh_new.
+ *
+ * Bottom halves that are scheduled from a bottom half handler are instantly
+ * invoked. This can create an infinite loop if a bottom half handler
+ * schedules itself.
+ *
+ * @bh: The bottom half to be scheduled.
+ */
+void qemu_bh_schedule_event(QEMUBH *bh, QEMUClockType clock_type);
+
/**
* qemu_bh_schedule: Schedule a bottom half.
*
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 8102fa54f0..1fb66e4254 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -128,7 +128,7 @@ void replay_flush_events(void);
/*! Adds bottom half event to the queue */
void replay_bh_schedule_event(QEMUBH *bh);
/* Adds oneshot bottom half event to the queue */
-void replay_bh_schedule_oneshot_event(AioContext *ctx,
+void replay_bh_oneshot_event(AioContext *ctx,
QEMUBHFunc *cb, void *opaque);
/*! Adds input event to the queue */
void replay_input_event(QemuConsole *src, InputEvent *evt);
diff --git a/block/block-backend.c b/block/block-backend.c
index db6f9b92a3..a19a32030d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1533,8 +1533,8 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk,
acb->blk = blk;
acb->ret = ret;
- replay_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
- error_callback_bh, acb);
+ aio_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
+ error_callback_bh, acb, QEMU_CLOCK_VIRTUAL);
return &acb->common;
}
@@ -1591,8 +1591,9 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset,
acb->has_returned = true;
if (acb->rwco.ret != NOT_DONE) {
- replay_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
- blk_aio_complete_bh, acb);
+ aio_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
+ blk_aio_complete_bh, acb,
+ QEMU_CLOCK_VIRTUAL);
}
return &acb->common;
@@ -1897,8 +1898,9 @@ BlockAIOCB *blk_aio_zone_report(BlockBackend *blk, int64_t offset,
acb->has_returned = true;
if (acb->rwco.ret != NOT_DONE) {
- replay_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
- blk_aio_complete_bh, acb);
+ aio_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
+ blk_aio_complete_bh, acb,
+ QEMU_CLOCK_VIRTUAL);
}
return &acb->common;
@@ -1938,8 +1940,9 @@ BlockAIOCB *blk_aio_zone_mgmt(BlockBackend *blk, BlockZoneOp op,
acb->has_returned = true;
if (acb->rwco.ret != NOT_DONE) {
- replay_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
- blk_aio_complete_bh, acb);
+ aio_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
+ blk_aio_complete_bh, acb,
+ QEMU_CLOCK_VIRTUAL);
}
return &acb->common;
@@ -1977,8 +1980,9 @@ BlockAIOCB *blk_aio_zone_append(BlockBackend *blk, int64_t *offset,
aio_co_enter(qemu_get_current_aio_context(), co);
acb->has_returned = true;
if (acb->rwco.ret != NOT_DONE) {
- replay_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
- blk_aio_complete_bh, acb);
+ aio_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
+ blk_aio_complete_bh, acb,
+ QEMU_CLOCK_VIRTUAL);
}
return &acb->common;
diff --git a/block/io.c b/block/io.c
index 301514c880..fcce071082 100644
--- a/block/io.c
+++ b/block/io.c
@@ -335,8 +335,9 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
bdrv_inc_in_flight(bs);
}
- replay_bh_schedule_oneshot_event(qemu_get_aio_context(),
- bdrv_co_drain_bh_cb, &data);
+ aio_bh_schedule_oneshot_event(qemu_get_aio_context(),
+ bdrv_co_drain_bh_cb, &data,
+ QEMU_CLOCK_VIRTUAL);
qemu_coroutine_yield();
/* If we are resumed from some other event (such as an aio completion or a
diff --git a/block/iscsi.c b/block/iscsi.c
index 979bf90cb7..98ed86b983 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -285,8 +285,9 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
}
if (iTask->co) {
- replay_bh_schedule_oneshot_event(iTask->iscsilun->aio_context,
- iscsi_co_generic_bh_cb, iTask);
+ aio_bh_schedule_oneshot_event(iTask->iscsilun->aio_context,
+ iscsi_co_generic_bh_cb, iTask,
+ QEMU_CLOCK_VIRTUAL);
} else {
iTask->complete = 1;
}
diff --git a/block/nfs.c b/block/nfs.c
index 0500f60c08..66d9df0d89 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -256,8 +256,9 @@ nfs_co_generic_cb(int ret, struct nfs_context *nfs, void *data,
if (task->ret < 0) {
error_report("NFS Error: %s", nfs_get_error(nfs));
}
- replay_bh_schedule_oneshot_event(task->client->aio_context,
- nfs_co_generic_bh_cb, task);
+ aio_bh_schedule_oneshot_event(task->client->aio_context,
+ nfs_co_generic_bh_cb, task,
+ QEMU_CLOCK_VIRTUAL);
}
static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, int64_t offset,
@@ -723,8 +724,9 @@ nfs_get_allocated_file_size_cb(int ret, struct nfs_context *nfs, void *data,
if (task->ret < 0) {
error_report("NFS Error: %s", nfs_get_error(nfs));
}
- replay_bh_schedule_oneshot_event(task->client->aio_context,
- nfs_co_generic_bh_cb, task);
+ aio_bh_schedule_oneshot_event(task->client->aio_context,
+ nfs_co_generic_bh_cb, task,
+ QEMU_CLOCK_VIRTUAL);
}
static int64_t coroutine_fn nfs_co_get_allocated_file_size(BlockDriverState *bs)
diff --git a/block/null.c b/block/null.c
index 4730acc1eb..d35b42cbb4 100644
--- a/block/null.c
+++ b/block/null.c
@@ -183,8 +183,8 @@ static inline BlockAIOCB *null_aio_common(BlockDriverState *bs,
timer_mod_ns(&acb->timer,
qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + s->latency_ns);
} else {
- replay_bh_schedule_oneshot_event(bdrv_get_aio_context(bs),
- null_bh_cb, acb);
+ aio_bh_schedule_oneshot_event(bdrv_get_aio_context(bs),
+ null_bh_cb, acb, QEMU_CLOCK_VIRTUAL);
}
return &acb->common;
}
diff --git a/block/nvme.c b/block/nvme.c
index 3b588b139f..4069639dc7 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -344,8 +344,9 @@ static void nvme_put_free_req_locked(NVMeQueuePair *q, NVMeRequest *req)
static void nvme_wake_free_req_locked(NVMeQueuePair *q)
{
if (!qemu_co_queue_empty(&q->free_req_queue)) {
- replay_bh_schedule_oneshot_event(q->s->aio_context,
- nvme_free_req_queue_cb, q);
+ aio_bh_schedule_oneshot_event(q->s->aio_context,
+ nvme_free_req_queue_cb, q,
+ QEMU_CLOCK_VIRTUAL);
}
}
@@ -1177,7 +1178,8 @@ static void nvme_rw_cb(void *opaque, int ret)
/* The rw coroutine hasn't yielded, don't try to enter. */
return;
}
- replay_bh_schedule_oneshot_event(data->ctx, nvme_rw_cb_bh, data);
+ aio_bh_schedule_oneshot_event(data->ctx, nvme_rw_cb_bh, data,
+ QEMU_CLOCK_VIRTUAL);
}
static coroutine_fn int nvme_co_prw_aligned(BlockDriverState *bs,
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 08d9218455..fa7fee61d9 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2368,8 +2368,9 @@ void ide_ctrl_write(void *opaque, uint32_t addr, uint32_t val)
s = &bus->ifs[i];
s->status |= BUSY_STAT;
}
- replay_bh_schedule_oneshot_event(qemu_get_aio_context(),
- ide_bus_perform_srst, bus);
+ aio_bh_schedule_oneshot_event(qemu_get_aio_context(),
+ ide_bus_perform_srst, bus,
+ QEMU_CLOCK_VIRTUAL);
}
bus->cmd = val;
diff --git a/stubs/replay-tools.c b/stubs/replay-tools.c
index 3e8ca3212d..31985af35f 100644
--- a/stubs/replay-tools.c
+++ b/stubs/replay-tools.c
@@ -30,7 +30,7 @@ void replay_bh_schedule_event(QEMUBH *bh)
qemu_bh_schedule(bh);
}
-void replay_bh_schedule_oneshot_event(AioContext *ctx,
+void replay_bh_oneshot_event(AioContext *ctx,
QEMUBHFunc *cb, void *opaque)
{
aio_bh_schedule_oneshot(ctx, cb, opaque);
diff --git a/util/async.c b/util/async.c
index 3e3e4fc712..97ed40048d 100644
--- a/util/async.c
+++ b/util/async.c
@@ -35,6 +35,7 @@
#include "qemu/coroutine_int.h"
#include "qemu/coroutine-tls.h"
#include "sysemu/cpu-timers.h"
+#include "sysemu/replay.h"
#include "trace.h"
/***********************************************************/
@@ -125,8 +126,8 @@ static QEMUBH *aio_bh_dequeue(BHList *head, unsigned *flags)
return bh;
}
-void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb,
- void *opaque, const char *name)
+static void do_aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb,
+ void *opaque, const char *name)
{
QEMUBH *bh;
bh = g_new(QEMUBH, 1);
@@ -139,6 +140,24 @@ void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb,
aio_bh_enqueue(bh, BH_SCHEDULED | BH_ONESHOT);
}
+void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb,
+ void *opaque, const char *name,
+ QEMUClockType clock_type)
+{
+ switch (clock_type) {
+ case QEMU_CLOCK_VIRTUAL:
+ case QEMU_CLOCK_VIRTUAL_RT:
+ if (replay_mode != REPLAY_MODE_NONE) {
+ /* Record/replay must intercept bh events */
+ replay_bh_oneshot_event(ctx, cb, opaque);
+ break;
+ }
+ /* fallthrough */
+ default:
+ do_aio_bh_schedule_oneshot_full(ctx, cb, opaque, name);
+ }
+}
+
QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
const char *name, MemReentrancyGuard *reentrancy_guard)
{
@@ -225,6 +244,23 @@ int aio_bh_poll(AioContext *ctx)
return ret;
}
+void qemu_bh_schedule_event(QEMUBH *bh, QEMUClockType clock_type)
+{
+ switch (clock_type) {
+ case QEMU_CLOCK_VIRTUAL:
+ case QEMU_CLOCK_VIRTUAL_RT:
+ if (replay_mode != REPLAY_MODE_NONE) {
+ /* Record/replay must intercept bh events */
+ qatomic_fetch_or(&bh->flags, BH_REPLAY);
+ replay_bh_schedule_event(bh);
+ break;
+ }
+ /* fallthrough */
+ default:
+ aio_bh_enqueue(bh, BH_SCHEDULED);
+ }
+}
+
void qemu_bh_schedule_idle(QEMUBH *bh)
{
aio_bh_enqueue(bh, BH_SCHEDULED | BH_IDLE);
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH 2/2] async: add debugging assertions for record/replay in bh APIs
2024-08-15 13:28 [RFC PATCH 0/2] async: rework async event API for replay Nicholas Piggin
2024-08-15 13:28 ` [RFC PATCH 1/2] " Nicholas Piggin
@ 2024-08-15 13:28 ` Nicholas Piggin
2024-08-15 15:30 ` [RFC PATCH 0/2] async: rework async event API for replay Michael S. Tsirkin
2 siblings, 0 replies; 6+ messages in thread
From: Nicholas Piggin @ 2024-08-15 13:28 UTC (permalink / raw)
To: qemu-devel
Cc: Nicholas Piggin, Pavel Dovgalyuk, Philippe Mathieu-Daudé,
Richard Henderson, Alex Bennée, Paolo Bonzini, John Snow,
Cleber Rosa, Wainer dos Santos Moschetta, Beraldo Leal,
Michael Tokarev, Michael S . Tsirkin
Code using old bh APIs should be updated to account for whether the
bh is related to the target machine model or host QEMU operation,
for record/replay to work properly.
Add some assertions in the old APIs to catch unconverted code when
record/replay is active. Some of the bh APIs like cancel and delete
also don't seem to be implemented in the replay-event driver, so
try to catch these being used too.
This fixes one bug in IDE code that was caught by the assertions.
That fixes the x86-64 q35 non-virtio record/replay avocado test for
me, so not entirely academic.
---
include/block/aio.h | 2 +-
block.c | 4 +++-
hw/ide/core.c | 2 +-
hw/scsi/scsi-bus.c | 6 +++---
monitor/monitor.c | 2 +-
monitor/qmp.c | 5 +++--
qapi/qmp-dispatch.c | 4 ++--
replay/replay-events.c | 25 +++++++++++--------------
util/aio-wait.c | 2 +-
util/async.c | 23 ++++++++++++++++++++++-
util/main-loop.c | 2 +-
util/thread-pool.c | 8 ++++----
scripts/block-coroutine-wrapper.py | 2 +-
13 files changed, 54 insertions(+), 33 deletions(-)
diff --git a/include/block/aio.h b/include/block/aio.h
index 23c5543506..b877c6070d 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -308,7 +308,7 @@ void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
*/
#define aio_bh_schedule_oneshot(ctx, cb, opaque) \
aio_bh_schedule_oneshot_full((ctx), (cb), (opaque), (stringify(cb)), \
- QEMU_CLOCK_REALTIME)
+ QEMU_CLOCK_MAX)
/**
* aio_bh_new_full: Allocate a new bottom half structure.
diff --git a/block.c b/block.c
index c317de9eaa..67c88e8c68 100644
--- a/block.c
+++ b/block.c
@@ -7144,7 +7144,9 @@ void bdrv_schedule_unref(BlockDriverState *bs)
if (!bs) {
return;
}
- aio_bh_schedule_oneshot(qemu_get_aio_context(), bdrv_schedule_unref_bh, bs);
+ aio_bh_schedule_oneshot_event(qemu_get_aio_context(),
+ bdrv_schedule_unref_bh, bs,
+ QEMU_CLOCK_REALTIME);
}
struct BdrvOpBlocker {
diff --git a/hw/ide/core.c b/hw/ide/core.c
index fa7fee61d9..ee77200f77 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2780,7 +2780,7 @@ static void ide_restart_cb(void *opaque, bool running, RunState state)
if (!bus->bh) {
bus->bh = qemu_bh_new(ide_restart_bh, bus);
- qemu_bh_schedule(bus->bh);
+ qemu_bh_schedule_event(bus->bh, QEMU_CLOCK_VIRTUAL);
}
}
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 53eff5dd3d..ef36f12031 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -166,9 +166,9 @@ static void scsi_device_for_each_req_async(SCSIDevice *s,
/* Paired with blk_dec_in_flight() in scsi_device_for_each_req_async_bh() */
blk_inc_in_flight(s->conf.blk);
- aio_bh_schedule_oneshot(blk_get_aio_context(s->conf.blk),
- scsi_device_for_each_req_async_bh,
- data);
+ aio_bh_schedule_oneshot_event(blk_get_aio_context(s->conf.blk),
+ scsi_device_for_each_req_async_bh,
+ data, QEMU_CLOCK_REALTIME);
}
static void scsi_device_realize(SCSIDevice *s, Error **errp)
diff --git a/monitor/monitor.c b/monitor/monitor.c
index db52a9c7ef..eae4d4e7f4 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -580,7 +580,7 @@ void monitor_resume(Monitor *mon)
ctx = qemu_get_aio_context();
}
- aio_bh_schedule_oneshot(ctx, monitor_accept_input, mon);
+ aio_bh_schedule_oneshot_event(ctx, monitor_accept_input, mon, QEMU_CLOCK_REALTIME);
}
trace_monitor_suspend(mon, -1);
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 5e538f34c0..c6fec04860 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -541,8 +541,9 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
* since chardev might be running in the monitor I/O
* thread. Schedule a bottom half.
*/
- aio_bh_schedule_oneshot(iothread_get_aio_context(mon_iothread),
- monitor_qmp_setup_handlers_bh, mon);
+ aio_bh_schedule_oneshot_event(iothread_get_aio_context(mon_iothread),
+ monitor_qmp_setup_handlers_bh, mon,
+ QEMU_CLOCK_REALTIME);
/* The bottom half will add @mon to @mon_list */
} else {
qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 176b549473..cd46a0830c 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -254,8 +254,8 @@ QDict *coroutine_mixed_fn qmp_dispatch(const QmpCommandList *cmds, QObject *requ
.errp = &err,
.co = qemu_coroutine_self(),
};
- aio_bh_schedule_oneshot(iohandler_get_aio_context(), do_qmp_dispatch_bh,
- &data);
+ aio_bh_schedule_oneshot_event(iohandler_get_aio_context(), do_qmp_dispatch_bh,
+ &data, QEMU_CLOCK_REALTIME);
qemu_coroutine_yield();
}
qobject_unref(args);
diff --git a/replay/replay-events.c b/replay/replay-events.c
index af0721cc1a..5ac974fcac 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -132,23 +132,17 @@ void replay_add_event(ReplayAsyncEventKind event_kind,
void replay_bh_schedule_event(QEMUBH *bh)
{
- if (events_enabled) {
- uint64_t id = replay_get_current_icount();
- replay_add_event(REPLAY_ASYNC_EVENT_BH, bh, NULL, id);
- } else {
- qemu_bh_schedule(bh);
- }
+ uint64_t id = replay_get_current_icount();
+ assert(events_enabled);
+ replay_add_event(REPLAY_ASYNC_EVENT_BH, bh, NULL, id);
}
-void replay_bh_schedule_oneshot_event(AioContext *ctx,
+void replay_bh_oneshot_event(AioContext *ctx,
QEMUBHFunc *cb, void *opaque)
{
- if (events_enabled) {
- uint64_t id = replay_get_current_icount();
- replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
- } else {
- aio_bh_schedule_oneshot(ctx, cb, opaque);
- }
+ uint64_t id = replay_get_current_icount();
+ assert(events_enabled);
+ replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
}
void replay_add_input_event(struct InputEvent *event)
@@ -166,7 +160,10 @@ void replay_block_event(QEMUBH *bh, uint64_t id)
if (events_enabled) {
replay_add_event(REPLAY_ASYNC_EVENT_BLOCK, bh, NULL, id);
} else {
- qemu_bh_schedule(bh);
+ /*
+ * Block can be used before events come up.
+ */
+ qemu_bh_schedule_event(bh, QEMU_CLOCK_REALTIME);
}
}
diff --git a/util/aio-wait.c b/util/aio-wait.c
index b5336cf5fd..32298d41b2 100644
--- a/util/aio-wait.c
+++ b/util/aio-wait.c
@@ -51,7 +51,7 @@ void aio_wait_kick(void)
smp_mb();
if (qatomic_read(&global_aio_wait.num_waiters)) {
- aio_bh_schedule_oneshot(qemu_get_aio_context(), dummy_bh_cb, NULL);
+ aio_bh_schedule_oneshot_event(qemu_get_aio_context(), dummy_bh_cb, NULL, QEMU_CLOCK_REALTIME);
}
}
diff --git a/util/async.c b/util/async.c
index 97ed40048d..6893609d8c 100644
--- a/util/async.c
+++ b/util/async.c
@@ -57,6 +57,9 @@ enum {
/* Schedule periodically when the event loop is idle */
BH_IDLE = (1 << 4),
+
+ /* BH being handled by replay machinery */
+ BH_REPLAY = (1 << 4),
};
struct QEMUBH {
@@ -144,6 +147,10 @@ void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb,
void *opaque, const char *name,
QEMUClockType clock_type)
{
+ if (clock_type == QEMU_CLOCK_MAX) {
+ assert(replay_mode == REPLAY_MODE_NONE);
+ clock_type = QEMU_CLOCK_REALTIME;
+ }
switch (clock_type) {
case QEMU_CLOCK_VIRTUAL:
case QEMU_CLOCK_VIRTUAL_RT:
@@ -177,6 +184,12 @@ void aio_bh_call(QEMUBH *bh)
{
bool last_engaged_in_io = false;
+ if (bh->flags & BH_REPLAY) {
+ assert(!(bh->flags & BH_SCHEDULED));
+ assert(!(bh->flags & BH_DELETED));
+ assert(!(bh->flags & BH_PENDING));
+ bh->flags &= ~BH_REPLAY;
+ }
/* Make a copy of the guard-pointer as cb may free the bh */
MemReentrancyGuard *reentrancy_guard = bh->reentrancy_guard;
if (reentrancy_guard) {
@@ -263,11 +276,15 @@ void qemu_bh_schedule_event(QEMUBH *bh, QEMUClockType clock_type)
void qemu_bh_schedule_idle(QEMUBH *bh)
{
+ /* No mechanism for scheduling idle replay-scheduled bh at the moment */
+ assert(replay_mode == REPLAY_MODE_NONE);
aio_bh_enqueue(bh, BH_SCHEDULED | BH_IDLE);
}
void qemu_bh_schedule(QEMUBH *bh)
{
+ /* Callers should be converted to use qemu_bh_schedule_event */
+ assert(replay_mode == REPLAY_MODE_NONE);
aio_bh_enqueue(bh, BH_SCHEDULED);
}
@@ -275,6 +292,8 @@ void qemu_bh_schedule(QEMUBH *bh)
*/
void qemu_bh_cancel(QEMUBH *bh)
{
+ /* No mechanism for canceling replay-scheduled bh at the moment */
+ assert(!(bh->flags & BH_REPLAY));
qatomic_and(&bh->flags, ~BH_SCHEDULED);
}
@@ -283,6 +302,8 @@ void qemu_bh_cancel(QEMUBH *bh)
*/
void qemu_bh_delete(QEMUBH *bh)
{
+ /* No mechanism for deleting replay-scheduled bh at the moment */
+ assert(!(bh->flags & BH_REPLAY));
aio_bh_enqueue(bh, BH_DELETED);
}
@@ -683,7 +704,7 @@ void aio_co_schedule(AioContext *ctx, Coroutine *co)
QSLIST_INSERT_HEAD_ATOMIC(&ctx->scheduled_coroutines,
co, co_scheduled_next);
- qemu_bh_schedule(ctx->co_schedule_bh);
+ qemu_bh_schedule_event(ctx->co_schedule_bh, QEMU_CLOCK_REALTIME);
aio_context_unref(ctx);
}
diff --git a/util/main-loop.c b/util/main-loop.c
index a0386cfeb6..6180a183f5 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -148,7 +148,7 @@ void qemu_notify_event(void)
if (!qemu_aio_context) {
return;
}
- qemu_bh_schedule(qemu_notify_bh);
+ qemu_bh_schedule_event(qemu_notify_bh, QEMU_CLOCK_REALTIME);
}
static GArray *gpollfds;
diff --git a/util/thread-pool.c b/util/thread-pool.c
index 27eb777e85..010eb4ad9a 100644
--- a/util/thread-pool.c
+++ b/util/thread-pool.c
@@ -115,7 +115,7 @@ static void *worker_thread(void *opaque)
smp_wmb();
req->state = THREAD_DONE;
- qemu_bh_schedule(pool->completion_bh);
+ qemu_bh_schedule_event(pool->completion_bh, QEMU_CLOCK_REALTIME);
qemu_mutex_lock(&pool->lock);
}
@@ -167,7 +167,7 @@ static void spawn_thread(ThreadPool *pool)
* inherit the correct affinity instead of the vcpu affinity.
*/
if (!pool->pending_threads) {
- qemu_bh_schedule(pool->new_thread_bh);
+ qemu_bh_schedule_event(pool->new_thread_bh, QEMU_CLOCK_REALTIME);
}
}
@@ -195,7 +195,7 @@ restart:
/* Schedule ourselves in case elem->common.cb() calls aio_poll() to
* wait for another request that completed at the same time.
*/
- qemu_bh_schedule(pool->completion_bh);
+ qemu_bh_schedule_event(pool->completion_bh, QEMU_CLOCK_REALTIME);
elem->common.cb(elem->common.opaque, elem->ret);
@@ -225,7 +225,7 @@ static void thread_pool_cancel(BlockAIOCB *acb)
QEMU_LOCK_GUARD(&pool->lock);
if (elem->state == THREAD_QUEUED) {
QTAILQ_REMOVE(&pool->request_list, elem, reqs);
- qemu_bh_schedule(pool->completion_bh);
+ qemu_bh_schedule_event(pool->completion_bh, QEMU_CLOCK_REALTIME);
elem->state = THREAD_DONE;
elem->ret = -ECANCELED;
diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index dbbde99e39..f28e7b9200 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -292,7 +292,7 @@ def gen_no_co_wrapper(func: FuncDecl) -> str:
}};
assert(qemu_in_coroutine());
- aio_bh_schedule_oneshot(qemu_get_aio_context(), {name}_bh, &s);
+ aio_bh_schedule_oneshot_event(qemu_get_aio_context(), {name}_bh, &s, QEMU_CLOCK_REALTIME);
qemu_coroutine_yield();
{func.ret}
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 0/2] async: rework async event API for replay
2024-08-15 13:28 [RFC PATCH 0/2] async: rework async event API for replay Nicholas Piggin
2024-08-15 13:28 ` [RFC PATCH 1/2] " Nicholas Piggin
2024-08-15 13:28 ` [RFC PATCH 2/2] async: add debugging assertions for record/replay in bh APIs Nicholas Piggin
@ 2024-08-15 15:30 ` Michael S. Tsirkin
2024-08-16 2:23 ` Nicholas Piggin
2 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2024-08-15 15:30 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-devel, Pavel Dovgalyuk, Philippe Mathieu-Daudé,
Richard Henderson, Alex Bennée, Paolo Bonzini, John Snow,
Cleber Rosa, Wainer dos Santos Moschetta, Beraldo Leal,
Michael Tokarev
On Thu, Aug 15, 2024 at 11:28:35PM +1000, Nicholas Piggin wrote:
> Continuing the conversation from the thread about record/replay
> virtio fix. Here is a sketch of how we could improve the naming
> convention so users of bh don't have to know about record/replay.
>
> Thanks,
> Nick
The API looks ok to me.
> Nicholas Piggin (2):
> async: rework async event API for replay
> async: add debugging assertions for record/replay in bh APIs
>
> docs/devel/replay.rst | 7 ++--
> include/block/aio.h | 35 +++++++++++++++--
> include/sysemu/replay.h | 2 +-
> block.c | 4 +-
> block/block-backend.c | 24 +++++++-----
> block/io.c | 5 ++-
> block/iscsi.c | 5 ++-
> block/nfs.c | 10 +++--
> block/null.c | 4 +-
> block/nvme.c | 8 ++--
> hw/ide/core.c | 7 ++--
> hw/scsi/scsi-bus.c | 6 +--
> monitor/monitor.c | 2 +-
> monitor/qmp.c | 5 ++-
> qapi/qmp-dispatch.c | 4 +-
> replay/replay-events.c | 25 ++++++------
> stubs/replay-tools.c | 2 +-
> util/aio-wait.c | 2 +-
> util/async.c | 63 ++++++++++++++++++++++++++++--
> util/main-loop.c | 2 +-
> util/thread-pool.c | 8 ++--
> scripts/block-coroutine-wrapper.py | 2 +-
> 22 files changed, 164 insertions(+), 68 deletions(-)
>
> --
> 2.45.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 0/2] async: rework async event API for replay
2024-08-15 15:30 ` [RFC PATCH 0/2] async: rework async event API for replay Michael S. Tsirkin
@ 2024-08-16 2:23 ` Nicholas Piggin
2024-08-16 7:34 ` Michael S. Tsirkin
0 siblings, 1 reply; 6+ messages in thread
From: Nicholas Piggin @ 2024-08-16 2:23 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Pavel Dovgalyuk, Philippe Mathieu-Daudé,
Richard Henderson, Alex Bennée, Paolo Bonzini, John Snow,
Cleber Rosa, Wainer dos Santos Moschetta, Beraldo Leal,
Michael Tokarev
On Fri Aug 16, 2024 at 1:30 AM AEST, Michael S. Tsirkin wrote:
> On Thu, Aug 15, 2024 at 11:28:35PM +1000, Nicholas Piggin wrote:
> > Continuing the conversation from the thread about record/replay
> > virtio fix. Here is a sketch of how we could improve the naming
> > convention so users of bh don't have to know about record/replay.
> >
> > Thanks,
> > Nick
>
> The API looks ok to me.
Thanks for taking a look. In that case let's go with the fixes for
now so we have rr regression tests in a bit better state, and I will
resend for 9.2.
Thanks,
Nick
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 0/2] async: rework async event API for replay
2024-08-16 2:23 ` Nicholas Piggin
@ 2024-08-16 7:34 ` Michael S. Tsirkin
0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2024-08-16 7:34 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-devel, Pavel Dovgalyuk, Philippe Mathieu-Daudé,
Richard Henderson, Alex Bennée, Paolo Bonzini, John Snow,
Cleber Rosa, Wainer dos Santos Moschetta, Beraldo Leal,
Michael Tokarev
On Fri, Aug 16, 2024 at 12:23:50PM +1000, Nicholas Piggin wrote:
> On Fri Aug 16, 2024 at 1:30 AM AEST, Michael S. Tsirkin wrote:
> > On Thu, Aug 15, 2024 at 11:28:35PM +1000, Nicholas Piggin wrote:
> > > Continuing the conversation from the thread about record/replay
> > > virtio fix. Here is a sketch of how we could improve the naming
> > > convention so users of bh don't have to know about record/replay.
> > >
> > > Thanks,
> > > Nick
> >
> > The API looks ok to me.
>
> Thanks for taking a look. In that case let's go with the fixes for
> now so we have rr regression tests in a bit better state, and I will
> resend for 9.2.
>
> Thanks,
> Nick
No, this is big because you are structuring this wrong.
Work, then rework ... I do not really like this kind of thing.
Add sane APIs and use them keeping old ones around, reworking old code
can wait for 9.2.
--
MST
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-08-16 7:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-15 13:28 [RFC PATCH 0/2] async: rework async event API for replay Nicholas Piggin
2024-08-15 13:28 ` [RFC PATCH 1/2] " Nicholas Piggin
2024-08-15 13:28 ` [RFC PATCH 2/2] async: add debugging assertions for record/replay in bh APIs Nicholas Piggin
2024-08-15 15:30 ` [RFC PATCH 0/2] async: rework async event API for replay Michael S. Tsirkin
2024-08-16 2:23 ` Nicholas Piggin
2024-08-16 7:34 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).