| Message ID | 161339392367.1222219.9529223857148298127.stgit@pasha-ThinkPad-X280 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | replay: notify CPU on event | expand |
On 15/02/21 13:58, Pavel Dovgalyuk wrote: > This patch enables vCPU notification to wake it up > when new async event comes in replay mode. > > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru> > --- > replay/replay-events.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/replay/replay-events.c b/replay/replay-events.c > index a1c6bb934e..92dc800219 100644 > --- a/replay/replay-events.c > +++ b/replay/replay-events.c > @@ -126,6 +126,7 @@ void replay_add_event(ReplayAsyncEventKind event_kind, > > g_assert(replay_mutex_locked()); > QTAILQ_INSERT_TAIL(&events_list, event, events); > + qemu_clock_notify(QEMU_CLOCK_VIRTUAL); > } > > void replay_bh_schedule_event(QEMUBH *bh) > Do you really want to notify the vCPU, or rather the main loop (which will run as a side effect of the lockstep behavior that RR puts in place)? The reason I doubt you need to notify the vCPU, is that I'm not sure why an incoming event would cause you to recalculate the QEMU_CLOCK_VIRTUAL deadline. Rather, perhaps the problem is that a bottom half cannot be run right away? And if so, probably the issue only happens for a running vCPU (not a sleeping one) so you only need qemu_cpu_kick(current_cpu)? Either way, your commit message does not explain why it is needed and how event are missed or delayed without the patch. This is especially important for something like RR, which is pretty invasive and understood only by very few people (I don't put myself in the group, in fact). Thanks, Paolo
On 15.02.2021 16:29, Paolo Bonzini wrote: > On 15/02/21 13:58, Pavel Dovgalyuk wrote: >> This patch enables vCPU notification to wake it up >> when new async event comes in replay mode. >> >> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru> >> --- >> replay/replay-events.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/replay/replay-events.c b/replay/replay-events.c >> index a1c6bb934e..92dc800219 100644 >> --- a/replay/replay-events.c >> +++ b/replay/replay-events.c >> @@ -126,6 +126,7 @@ void replay_add_event(ReplayAsyncEventKind >> event_kind, >> g_assert(replay_mutex_locked()); >> QTAILQ_INSERT_TAIL(&events_list, event, events); >> + qemu_clock_notify(QEMU_CLOCK_VIRTUAL); >> } >> void replay_bh_schedule_event(QEMUBH *bh) >> > > Do you really want to notify the vCPU, or rather the main loop (which > will run as a side effect of the lockstep behavior that RR puts in place)? > > The reason I doubt you need to notify the vCPU, is that I'm not sure why > an incoming event would cause you to recalculate the QEMU_CLOCK_VIRTUAL > deadline. Rather, perhaps the problem is that a bottom half cannot be > run right away? And if so, probably the issue only happens for a > running vCPU (not a sleeping one) so you only need > qemu_cpu_kick(current_cpu)? I've got the following issue: vCPU thread is sleeping in rr_wait_io_event. The next event is block async callback linked with CHECKPOINT_CLOCK_WARP_ACCOUNT. Therefore this event should be processed by vCPU, which is sleeping. > > Either way, your commit message does not explain why it is needed and > how event are missed or delayed without the patch. This is especially > important for something like RR, which is pretty invasive and understood > only by very few people (I don't put myself in the group, in fact). Ok, I'll update the comment. Pavel Dovgalyuk
diff --git a/replay/replay-events.c b/replay/replay-events.c index a1c6bb934e..92dc800219 100644 --- a/replay/replay-events.c +++ b/replay/replay-events.c @@ -126,6 +126,7 @@ void replay_add_event(ReplayAsyncEventKind event_kind, g_assert(replay_mutex_locked()); QTAILQ_INSERT_TAIL(&events_list, event, events); + qemu_clock_notify(QEMU_CLOCK_VIRTUAL); } void replay_bh_schedule_event(QEMUBH *bh)
This patch enables vCPU notification to wake it up when new async event comes in replay mode. Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru> --- replay/replay-events.c | 1 + 1 file changed, 1 insertion(+)