Re: [PATCH] replay: fix replay of the interrupts

From: "Alex Bennée" <alex.bennee@linaro.org>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>,
	Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH] replay: fix replay of the interrupts
Date: Mon, 25 Jan 2021 12:12:52 +0000	[thread overview]
Message-ID: <87sg6pmcsz.fsf@linaro.org> (raw)
In-Reply-To: <CABgObfaFnKztrjc7mpgTxEi9R7jXD-Qed5vVcPBSGcE_nexONg@mail.gmail.com>

Paolo Bonzini <pbonzini@redhat.com> writes:

> In general I agree, but != means that rr disabled returns true. In general
> it seems to me that rr disabled should work more or less the same as record
> mode, because there is no replay log to provide the checkpoints.

Is this not an argument to combine the mode and check into replay.h
inline helpers with some clear semantic documentation and the call sites
become self documenting?

if (deadline == 0 && replay_recording_or_checkpoint())

which also makes things easier to compile away if replay isn't there?

>
> Paolo
>
> Il lun 25 gen 2021, 06:38 Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> ha
> scritto:
>
>> On 23.01.2021 21:15, Paolo Bonzini wrote:
>> > On 19/01/21 13:39, Pavel Dovgalyuk wrote:
>> >> Sometimes interrupt event comes at the same time with
>> >> the virtual timers. In this case replay tries to proceed
>> >> the timers, because deadline for them is zero.
>> >> This patch allows processing interrupts and exceptions
>> >> by entering the vCPU execution loop, when deadline is zero,
>> >> but checkpoint associated with virtual timers is not ready
>> >> to be replayed.
>> >>
>> >> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
>> >> ---
>> >>   accel/tcg/tcg-cpus-icount.c |    8 +++++++-
>> >>   1 file changed, 7 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/accel/tcg/tcg-cpus-icount.c b/accel/tcg/tcg-cpus-icount.c
>> >> index 9f45432275..a6d2bb8a88 100644
>> >> --- a/accel/tcg/tcg-cpus-icount.c
>> >> +++ b/accel/tcg/tcg-cpus-icount.c
>> >> @@ -81,7 +81,13 @@ void icount_handle_deadline(void)
>> >>       int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
>> >>
>> QEMU_TIMER_ATTR_ALL);
>> >> -    if (deadline == 0) {
>> >> +    /*
>> >> +     * Instructions, interrupts, and exceptions are processed in
>> >> cpu-exec.
>> >> +     * Don't interrupt cpu thread, when these events are waiting
>> >> +     * (i.e., there is no checkpoint)
>> >> +     */
>> >> +    if (deadline == 0
>> >> +        && (replay_mode == REPLAY_MODE_RECORD ||
>> >> replay_has_checkpoint())) {
>> >
>> > Should this be replay_mode != REPLAY_MODE_PLAY ||
>> replay_has_checkpoint()?
>>
>> It was the first idea, but I thought, that == is more straightforward
>> to understand than !=.
>>
>> Pavel Dovgalyuk
>>
>>

-- 
Alex Bennée