On 10/7/20 2:20 PM, Pavel Dovgalyuk wrote:
On 07.10.2020 14:22, Alex Bennée wrote:
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
On 10/7/20 10:51 AM, Pavel Dovgalyuk wrote:
On 07.10.2020 11:23, Thomas Huth wrote:
On 07/10/2020 09.13, Philippe Mathieu-Daudé wrote:
Thanks, that was helpful. ... and the winner is:
commit 55adb3c45620c31f29978f209e2a44a08d34e2da
Author: John Snow <jsnow@redhat.com>
Date: Fri Jul 24 01:23:00 2020 -0400
Subject: ide: cancel pending callbacks on SRST
... starting with this commit, the tests starts failing. John, any
idea what
might be causing this?
This patch includes the following lines:
+ aio_bh_schedule_oneshot(qemu_get_aio_context(),
+ ide_bus_perform_srst, bus);
replay_bh_schedule_oneshot_event should be used instead of this
function, because it synchronizes non-deterministic BHs.
Why do we have 2 different functions? BH are already complex
enough, and we need to also think about the replay API...
What about the other cases such vhost-user (blk/net), virtio-blk?
This does seem like something that should be wrapped up inside
aio_bh_schedule_oneshot itself or maybe we need a
aio_bh_schedule_transaction_oneshot to distinguish it from the other
uses the function has.
Maybe there should be two functions:
- one for the guest modification
aio_bh_schedule_oneshot_deterministic()?
- one for internal qemu things
Not sure why there is a difference, BH are used to
avoid delaying the guest, so there always something
related to "guest modification".
The first one may be implemented though the rr+second one.
Maybe replay_ prefix is confusing and the whole BH interface should look
like that?
Yes, but it would be safer/clearer if we don't need to use
a replay_ API.
Can we embed these functions?
- replay_bh_schedule_event
- replay_bh_schedule_oneshot_event
If compiled without rr, events_enabled=false and
compiler can optimize:
-- >8 --
diff --git a/util/async.c b/util/async.c
index f758354c6a..376b6d4e27 100644
--- a/util/async.c
+++ b/util/async.c
@@ -109,14 +109,19 @@ static QEMUBH *aio_bh_dequeue(BHList *head,
unsigned *flags)
void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
{
- QEMUBH *bh;
- bh = g_new(QEMUBH, 1);
- *bh = (QEMUBH){
- .ctx = ctx,
- .cb = cb,
- .opaque = opaque,
- };
- aio_bh_enqueue(bh, BH_SCHEDULED | BH_ONESHOT);
+ if (events_enabled) {
+ replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb,
+ opaque, replay_get_current_icount());
+ } else {
+ QEMUBH *bh;
+ bh = g_new(QEMUBH, 1);
+ *bh = (QEMUBH){
+ .ctx = ctx,
+ .cb = cb,
+ .opaque = opaque,
+ };
+ aio_bh_enqueue(bh, BH_SCHEDULED | BH_ONESHOT);
+ }
}
QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
@@ -178,7 +183,12 @@ void qemu_bh_schedule_idle(QEMUBH *bh)
void qemu_bh_schedule(QEMUBH *bh)
{
- aio_bh_enqueue(bh, BH_SCHEDULED);
+ if (events_enabled) {
+ replay_add_event(REPLAY_ASYNC_EVENT_BH, bh, NULL,
+ replay_get_current_icount());
+ } else {
+ aio_bh_enqueue(bh, BH_SCHEDULED);
+ }
}