All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: qemu-devel@nongnu.org
Cc: "Nicholas Piggin" <npiggin@gmail.com>,
	qemu-block@nongnu.org, "Alex Bennée" <alex.bennee@linaro.org>,
	"Kevin Wolf" <kwolf@redhat.com>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"Pavel Dovgalyuk" <pavel.dovgaluk@ispras.ru>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Fam Zheng" <fam@euphon.net>,
	"Ronnie Sahlberg" <ronniesahlberg@gmail.com>,
	"John Snow" <jsnow@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Vladimir Sementsov-Ogievskiy" <vsementsov@yandex-team.ru>,
	"Peter Xu" <peterx@redhat.com>, "Fabiano Rosas" <farosas@suse.de>,
	"Dr. David Alan Gilbert" <dave@treblig.org>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Michael Roth" <michael.roth@amd.com>,
	"Wainer dos Santos Moschetta" <wainersm@redhat.com>
Subject: [PATCH 14/17] async: add debugging assertions for record/replay in bh APIs
Date: Fri, 20 Dec 2024 20:42:16 +1000	[thread overview]
Message-ID: <20241220104220.2007786-15-npiggin@gmail.com> (raw)
In-Reply-To: <20241220104220.2007786-1-npiggin@gmail.com>

Code using old bh APIs must be updated to account for whether the
bh is related to the target machine model or host QEMU operation,
in order for record/replay to work properly.

Add some assertions in the old APIs to catch unconverted code when
record/replay is active.

This caught the IDE bug when running replay_linux.py avocado test
with the x86-64 q35 non-virtio machine.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 include/block/aio.h    |  2 +-
 replay/replay-events.c | 20 ++++++++------------
 util/async.c           | 29 +++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/include/block/aio.h b/include/block/aio.h
index 26859bd0b93..991aaae707d 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -309,7 +309,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/replay/replay-events.c b/replay/replay-events.c
index 6a7c27cac1e..0b3dbfd46b9 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -123,23 +123,19 @@ 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;
+    g_assert(events_enabled);
+    id = replay_get_current_icount();
+    replay_add_event(REPLAY_ASYNC_EVENT_BH, bh, NULL, id);
 }
 
 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;
+    g_assert(events_enabled);
+    id = replay_get_current_icount();
+    replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
 }
 
 void replay_add_input_event(struct InputEvent *event)
diff --git a/util/async.c b/util/async.c
index 5d2c76dec08..72a9eccffbe 100644
--- a/util/async.c
+++ b/util/async.c
@@ -58,6 +58,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 {
@@ -145,6 +148,17 @@ void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb,
                                   void *opaque, const char *name,
                                   QEMUClockType clock_type)
 {
+    if (clock_type == QEMU_CLOCK_MAX) {
+        /*
+         * aio_bh_schedule_oneshot() uses QEMU_CLOCK_MAX to say it does not
+         * know about clock context to use. It will not work in record/replay.
+         * Callers should be converted to aio_bh_schedule_oneshot_event()
+         * then this can be removed when the old API goes away.
+         */
+        g_assert(replay_mode == REPLAY_MODE_NONE);
+        clock_type = QEMU_CLOCK_REALTIME;
+    }
+
     switch (clock_type) {
     case QEMU_CLOCK_VIRTUAL:
     case QEMU_CLOCK_VIRTUAL_RT:
@@ -178,6 +192,12 @@ void aio_bh_call(QEMUBH *bh)
 {
     bool last_engaged_in_io = false;
 
+    if (bh->flags & BH_REPLAY) {
+        g_assert(!(bh->flags & BH_SCHEDULED));
+        g_assert(!(bh->flags & BH_DELETED));
+        g_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) {
@@ -252,6 +272,7 @@ void qemu_bh_schedule_event(QEMUBH *bh, QEMUClockType clock_type)
     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;
         }
@@ -268,11 +289,15 @@ void qemu_bh_schedule_event_noreplay(QEMUBH *bh)
 
 void qemu_bh_schedule_idle(QEMUBH *bh)
 {
+    /* No mechanism for scheduling idle replay-scheduled bh at the moment */
+    g_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 */
+    g_assert(replay_mode == REPLAY_MODE_NONE);
     aio_bh_enqueue(bh, BH_SCHEDULED);
 }
 
@@ -280,6 +305,8 @@ void qemu_bh_schedule(QEMUBH *bh)
  */
 void qemu_bh_cancel(QEMUBH *bh)
 {
+    /* No mechanism for canceling replay-scheduled bh at the moment */
+    g_assert(!(bh->flags & BH_REPLAY));
     qatomic_and(&bh->flags, ~BH_SCHEDULED);
 }
 
@@ -288,6 +315,8 @@ void qemu_bh_cancel(QEMUBH *bh)
  */
 void qemu_bh_delete(QEMUBH *bh)
 {
+    /* No mechanism for deleting replay-scheduled bh at the moment */
+    g_assert(!(bh->flags & BH_REPLAY));
     aio_bh_enqueue(bh, BH_DELETED);
 }
 
-- 
2.45.2



  parent reply	other threads:[~2024-12-20 10:44 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-20 10:42 [PATCH 00/17] replay: Fixes and avocado test updates Nicholas Piggin
2024-12-20 10:42 ` [PATCH 01/17] replay: Fix migration use of clock for statistics Nicholas Piggin
2024-12-20 16:31   ` Peter Xu
2024-12-21  3:02     ` Nicholas Piggin
2024-12-23 17:26       ` Peter Xu
2024-12-24  7:24         ` Pavel Dovgalyuk
2024-12-24 15:19           ` Peter Xu
2024-12-20 10:42 ` [PATCH 02/17] replay: Fix migration replay_mutex locking Nicholas Piggin
2024-12-20 13:08   ` Fabiano Rosas
2024-12-21  2:54     ` Nicholas Piggin
2024-12-20 10:42 ` [PATCH 03/17] async: rework async event API for replay Nicholas Piggin
2024-12-20 10:42 ` [PATCH 04/17] util/main-loop: Convert to new bh API Nicholas Piggin
2024-12-20 10:42 ` [PATCH 05/17] util/thread-pool: " Nicholas Piggin
2024-12-20 10:42 ` [PATCH 06/17] util/aio-wait: " Nicholas Piggin
2024-12-20 10:42 ` [PATCH 07/17] async/coroutine: " Nicholas Piggin
2024-12-20 10:42 ` [PATCH 08/17] migration: " Nicholas Piggin
2024-12-20 10:42 ` [PATCH 09/17] monitor: " Nicholas Piggin
2024-12-20 10:42 ` [PATCH 10/17] qmp: " Nicholas Piggin
2024-12-20 10:42 ` [PATCH 11/17] block: " Nicholas Piggin
2024-12-20 10:42 ` [PATCH 12/17] hw/ide: Fix record-replay and convert " Nicholas Piggin
2024-12-20 10:42 ` [PATCH 13/17] hw/scsi: Convert " Nicholas Piggin
2024-12-20 23:54   ` Paolo Bonzini
2024-12-21  3:17     ` Nicholas Piggin
2024-12-20 10:42 ` Nicholas Piggin [this message]
2024-12-20 10:42 ` [PATCH 15/17] tests/avocado/replay_linux: Fix compile error Nicholas Piggin
2024-12-20 10:42 ` [PATCH 16/17] tests/avocado/replay_linux: Fix cdrom device setup Nicholas Piggin
2024-12-20 10:42 ` [PATCH 17/17] tests/avocado/replay_linux: remove the timeout expected guards Nicholas Piggin
2024-12-20 11:42 ` [PATCH 00/17] replay: Fixes and avocado test updates Pavel Dovgalyuk

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=20241220104220.2007786-15-npiggin@gmail.com \
    --to=npiggin@gmail.com \
    --cc=alex.bennee@linaro.org \
    --cc=armbru@redhat.com \
    --cc=dave@treblig.org \
    --cc=fam@euphon.net \
    --cc=farosas@suse.de \
    --cc=hreitz@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=mst@redhat.com \
    --cc=pavel.dovgaluk@ispras.ru \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=ronniesahlberg@gmail.com \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@yandex-team.ru \
    --cc=wainersm@redhat.com \
    /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.