Re: [PATCH] replay: notify CPU on event

[Paolo Bonzini]

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