[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Claudio Fontana
Subject: Re: [PATCH] replay: fix replay of the interrupts
Date: Mon, 25 Jan 2021 16:17:36 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0

On 1/25/21 3:26 PM, Alex Bennée wrote:
> Claudio Fontana <cfontana@suse.de> writes:
>> On 1/25/21 1:43 PM, Claudio Fontana wrote:
>>> On 1/25/21 1:12 PM, Alex Bennée wrote:
>>>> 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?
>>> Seems that the TCG build faces a similar issue to the issue I was facing 
>>> with the non-TCG build,
>>> before the non-TCG build got functional again (for x86).
>>> We solved the non-TCG build problem, by not compiling replay at all for 
>>> non-TCG, plus closing our nose and stubbing away what couldn't be 
>>> completely removed (yet).
>>> But the CONFIG_TCG build has the same legitimate requirement towards a 
>>> non-CONFIG_REPLAY build.
>>> ie, like we have tcg_enabled(), should we have replay_enabled()? Maybe it 
>>> could be reworked starting from replay_events_enabled()?
>>> And then when things are refactored properly for replay_enabled(), a 
>>> non-REPLAY TCG build can basically ignore all the inner workings of replay.
>> I guess to summarize the above, should there be a CONFIG_REPLAY, dependent 
>> on CONFIG_TCG, by default on,
>> but which could be disabled with
>> --disable-replay
>> ?
> I'm not sure - fundamentally having replay is one of those cool things
> you can do when you have TCG and I suspect there is less pressure to
> have a finely tuned TCG enabled build compared to our HW accelerated
> brethren using hypervisors. TCG plugins are a configure option purely
> because there is a small but non-marginal cost in having it enabled. I
> doubt replay figures that much if you are not using it.
> That said if it makes it easier to make sure our abstractions are clean
> and the layering is good then maybe it is worth having a build that

I think that cleaning up these aspects would be beneficial in replay itself,
but maybe this could be done without forcing the --disable-replay option.

> allows us to check that. But if it's going to be super fiddly and
> involve large amounts of code motion I doubt the "win" is big enough for
> the effort.
> Also I suspect the config option would be CONFIG_ICOUNT because replay
> is just one of the features on the spectrum of:
>  deterministic icount -> record/replay -> reverse debugging
> which all require the base support for icount.

Right, icount_enabled() is already fundamentally able to check whether replay 
functionality is available or not.
Probably there is already some cleanup possible by applying this consistently.

Other clean up opportunities I see are in making consistent checks for whether 
the functionality is active, and consolidate all the spread state,
including (but not limited to):

replay_mode, events_enabled, replay_is_debugging, in_checkpoint, ... (into a 
single struct replay ?)

..and then I guess clean up the namespace from all the replay_ functions for 
which we need a gazillion stubs for non-TCG code..



reply via email to

[Prev in Thread] Current Thread [Next in Thread]