On 10/7/20 2:20 PM, Pavel Dovgalyuk wrote:
> On 07.10.2020 14:22, Alex Bennée wrote:
>>
>> Philippe Mathieu-Daudé <phi...@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 <js...@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);
+ }
}
---
>
>
> Pavel Dovgalyuk
>
