* [PATCH] replay: fix icount request when replaying clock access
@ 2021-02-16 12:51 Pavel Dovgalyuk
2021-02-16 16:14 ` Paolo Bonzini
0 siblings, 1 reply; 2+ messages in thread
From: Pavel Dovgalyuk @ 2021-02-16 12:51 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, pbonzini, pavel.dovgalyuk
Record/replay provides REPLAY_CLOCK_LOCKED macro to access
the clock when vm_clock_seqlock is locked. This macro is
needed because replay internals operate icount. In locked case
replay use icount_get_raw_locked for icount request, which prevents
excess locking which leads to deadlock. But previously only
record code used *_locked function and replay did not.
Therefore sometimes clock access lead to deadlocks.
This patch fixes clock access for replay too and uses *_locked
icount access function.
Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
---
include/sysemu/replay.h | 14 ++++++++------
replay/replay-internal.c | 29 +++++++++++++++++++++++++----
replay/replay-time.c | 4 ++--
replay/replay.c | 23 +----------------------
stubs/replay-tools.c | 2 +-
5 files changed, 37 insertions(+), 35 deletions(-)
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 56c0c17c30..0f3b0f7eac 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -128,18 +128,20 @@ bool replay_has_interrupt(void);
int64_t replay_save_clock(ReplayClockKind kind, int64_t clock,
int64_t raw_icount);
/*! Read the specified clock from the log or return cached data */
-int64_t replay_read_clock(ReplayClockKind kind);
+int64_t replay_read_clock(ReplayClockKind kind, int64_t raw_icount);
/*! Saves or reads the clock depending on the current replay mode. */
#define REPLAY_CLOCK(clock, value) \
- (replay_mode == REPLAY_MODE_PLAY ? replay_read_clock((clock)) \
+ (replay_mode == REPLAY_MODE_PLAY \
+ ? replay_read_clock((clock), icount_get_raw()) \
: replay_mode == REPLAY_MODE_RECORD \
- ? replay_save_clock((clock), (value), icount_get_raw()) \
- : (value))
+ ? replay_save_clock((clock), (value), icount_get_raw()) \
+ : (value))
#define REPLAY_CLOCK_LOCKED(clock, value) \
- (replay_mode == REPLAY_MODE_PLAY ? replay_read_clock((clock)) \
+ (replay_mode == REPLAY_MODE_PLAY \
+ ? replay_read_clock((clock), icount_get_raw_locked()) \
: replay_mode == REPLAY_MODE_RECORD \
? replay_save_clock((clock), (value), icount_get_raw_locked()) \
- : (value))
+ : (value))
/* Processing data from random generators */
diff --git a/replay/replay-internal.c b/replay/replay-internal.c
index 2e8a3e947a..77d0c82327 100644
--- a/replay/replay-internal.c
+++ b/replay/replay-internal.c
@@ -247,10 +247,31 @@ void replay_advance_current_icount(uint64_t current_icount)
/* Time can only go forward */
assert(diff >= 0);
- if (diff > 0) {
- replay_put_event(EVENT_INSTRUCTION);
- replay_put_dword(diff);
- replay_state.current_icount += diff;
+ if (replay_mode == REPLAY_MODE_RECORD) {
+ if (diff > 0) {
+ replay_put_event(EVENT_INSTRUCTION);
+ replay_put_dword(diff);
+ replay_state.current_icount += diff;
+ }
+ } else if (replay_mode == REPLAY_MODE_PLAY) {
+ if (diff > 0) {
+ replay_state.instruction_count -= diff;
+ replay_state.current_icount += diff;
+ if (replay_state.instruction_count == 0) {
+ assert(replay_state.data_kind == EVENT_INSTRUCTION);
+ replay_finish_event();
+ /* Wake up iothread. This is required because
+ timers will not expire until clock counters
+ will be read from the log. */
+ qemu_notify_event();
+ }
+ }
+ /* Execution reached the break step */
+ if (replay_break_icount == replay_state.current_icount) {
+ /* Cannot make callback directly from the vCPU thread */
+ timer_mod_ns(replay_break_timer,
+ qemu_clock_get_ns(QEMU_CLOCK_REALTIME));
+ }
}
}
diff --git a/replay/replay-time.c b/replay/replay-time.c
index 43357c9f24..00ebcb7a49 100644
--- a/replay/replay-time.c
+++ b/replay/replay-time.c
@@ -46,12 +46,12 @@ void replay_read_next_clock(ReplayClockKind kind)
}
/*! Reads next clock event from the input. */
-int64_t replay_read_clock(ReplayClockKind kind)
+int64_t replay_read_clock(ReplayClockKind kind, int64_t raw_icount)
{
int64_t ret;
g_assert(replay_file && replay_mutex_locked());
- replay_account_executed_instructions();
+ replay_advance_current_icount(raw_icount);
if (replay_next_event_is(EVENT_CLOCK + kind)) {
replay_read_next_clock(kind);
diff --git a/replay/replay.c b/replay/replay.c
index d4c228ab28..c806fec69a 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -94,28 +94,7 @@ void replay_account_executed_instructions(void)
if (replay_mode == REPLAY_MODE_PLAY) {
g_assert(replay_mutex_locked());
if (replay_state.instruction_count > 0) {
- int count = (int)(replay_get_current_icount()
- - replay_state.current_icount);
-
- /* Time can only go forward */
- assert(count >= 0);
-
- replay_state.instruction_count -= count;
- replay_state.current_icount += count;
- if (replay_state.instruction_count == 0) {
- assert(replay_state.data_kind == EVENT_INSTRUCTION);
- replay_finish_event();
- /* Wake up iothread. This is required because
- timers will not expire until clock counters
- will be read from the log. */
- qemu_notify_event();
- }
- /* Execution reached the break step */
- if (replay_break_icount == replay_state.current_icount) {
- /* Cannot make callback directly from the vCPU thread */
- timer_mod_ns(replay_break_timer,
- qemu_clock_get_ns(QEMU_CLOCK_REALTIME));
- }
+ replay_advance_current_icount(replay_get_current_icount());
}
}
}
diff --git a/stubs/replay-tools.c b/stubs/replay-tools.c
index c06b360e22..43296b3d4e 100644
--- a/stubs/replay-tools.c
+++ b/stubs/replay-tools.c
@@ -13,7 +13,7 @@ int64_t replay_save_clock(unsigned int kind, int64_t clock, int64_t raw_icount)
return 0;
}
-int64_t replay_read_clock(unsigned int kind)
+int64_t replay_read_clock(unsigned int kind, int64_t raw_icount)
{
abort();
return 0;
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] replay: fix icount request when replaying clock access
2021-02-16 12:51 [PATCH] replay: fix icount request when replaying clock access Pavel Dovgalyuk
@ 2021-02-16 16:14 ` Paolo Bonzini
0 siblings, 0 replies; 2+ messages in thread
From: Paolo Bonzini @ 2021-02-16 16:14 UTC (permalink / raw)
To: Pavel Dovgalyuk, qemu-devel; +Cc: alex.bennee
On 16/02/21 13:51, Pavel Dovgalyuk wrote:
> Record/replay provides REPLAY_CLOCK_LOCKED macro to access
> the clock when vm_clock_seqlock is locked. This macro is
> needed because replay internals operate icount. In locked case
> replay use icount_get_raw_locked for icount request, which prevents
> excess locking which leads to deadlock. But previously only
> record code used *_locked function and replay did not.
> Therefore sometimes clock access lead to deadlocks.
> This patch fixes clock access for replay too and uses *_locked
> icount access function.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> ---
> include/sysemu/replay.h | 14 ++++++++------
> replay/replay-internal.c | 29 +++++++++++++++++++++++++----
> replay/replay-time.c | 4 ++--
> replay/replay.c | 23 +----------------------
> stubs/replay-tools.c | 2 +-
> 5 files changed, 37 insertions(+), 35 deletions(-)
>
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index 56c0c17c30..0f3b0f7eac 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -128,18 +128,20 @@ bool replay_has_interrupt(void);
> int64_t replay_save_clock(ReplayClockKind kind, int64_t clock,
> int64_t raw_icount);
> /*! Read the specified clock from the log or return cached data */
> -int64_t replay_read_clock(ReplayClockKind kind);
> +int64_t replay_read_clock(ReplayClockKind kind, int64_t raw_icount);
> /*! Saves or reads the clock depending on the current replay mode. */
> #define REPLAY_CLOCK(clock, value) \
> - (replay_mode == REPLAY_MODE_PLAY ? replay_read_clock((clock)) \
> + (replay_mode == REPLAY_MODE_PLAY \
> + ? replay_read_clock((clock), icount_get_raw()) \
> : replay_mode == REPLAY_MODE_RECORD \
> - ? replay_save_clock((clock), (value), icount_get_raw()) \
> - : (value))
> + ? replay_save_clock((clock), (value), icount_get_raw()) \
> + : (value))
> #define REPLAY_CLOCK_LOCKED(clock, value) \
> - (replay_mode == REPLAY_MODE_PLAY ? replay_read_clock((clock)) \
> + (replay_mode == REPLAY_MODE_PLAY \
> + ? replay_read_clock((clock), icount_get_raw_locked()) \
> : replay_mode == REPLAY_MODE_RECORD \
> ? replay_save_clock((clock), (value), icount_get_raw_locked()) \
> - : (value))
> + : (value))
>
> /* Processing data from random generators */
>
> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
> index 2e8a3e947a..77d0c82327 100644
> --- a/replay/replay-internal.c
> +++ b/replay/replay-internal.c
> @@ -247,10 +247,31 @@ void replay_advance_current_icount(uint64_t current_icount)
> /* Time can only go forward */
> assert(diff >= 0);
>
> - if (diff > 0) {
> - replay_put_event(EVENT_INSTRUCTION);
> - replay_put_dword(diff);
> - replay_state.current_icount += diff;
> + if (replay_mode == REPLAY_MODE_RECORD) {
> + if (diff > 0) {
> + replay_put_event(EVENT_INSTRUCTION);
> + replay_put_dword(diff);
> + replay_state.current_icount += diff;
> + }
> + } else if (replay_mode == REPLAY_MODE_PLAY) {
> + if (diff > 0) {
> + replay_state.instruction_count -= diff;
> + replay_state.current_icount += diff;
> + if (replay_state.instruction_count == 0) {
> + assert(replay_state.data_kind == EVENT_INSTRUCTION);
> + replay_finish_event();
> + /* Wake up iothread. This is required because
> + timers will not expire until clock counters
> + will be read from the log. */
> + qemu_notify_event();
> + }
> + }
> + /* Execution reached the break step */
> + if (replay_break_icount == replay_state.current_icount) {
> + /* Cannot make callback directly from the vCPU thread */
> + timer_mod_ns(replay_break_timer,
> + qemu_clock_get_ns(QEMU_CLOCK_REALTIME));
> + }
> }
> }
>
> diff --git a/replay/replay-time.c b/replay/replay-time.c
> index 43357c9f24..00ebcb7a49 100644
> --- a/replay/replay-time.c
> +++ b/replay/replay-time.c
> @@ -46,12 +46,12 @@ void replay_read_next_clock(ReplayClockKind kind)
> }
>
> /*! Reads next clock event from the input. */
> -int64_t replay_read_clock(ReplayClockKind kind)
> +int64_t replay_read_clock(ReplayClockKind kind, int64_t raw_icount)
> {
> int64_t ret;
> g_assert(replay_file && replay_mutex_locked());
>
> - replay_account_executed_instructions();
> + replay_advance_current_icount(raw_icount);
>
> if (replay_next_event_is(EVENT_CLOCK + kind)) {
> replay_read_next_clock(kind);
> diff --git a/replay/replay.c b/replay/replay.c
> index d4c228ab28..c806fec69a 100644
> --- a/replay/replay.c
> +++ b/replay/replay.c
> @@ -94,28 +94,7 @@ void replay_account_executed_instructions(void)
> if (replay_mode == REPLAY_MODE_PLAY) {
> g_assert(replay_mutex_locked());
> if (replay_state.instruction_count > 0) {
> - int count = (int)(replay_get_current_icount()
> - - replay_state.current_icount);
> -
> - /* Time can only go forward */
> - assert(count >= 0);
> -
> - replay_state.instruction_count -= count;
> - replay_state.current_icount += count;
> - if (replay_state.instruction_count == 0) {
> - assert(replay_state.data_kind == EVENT_INSTRUCTION);
> - replay_finish_event();
> - /* Wake up iothread. This is required because
> - timers will not expire until clock counters
> - will be read from the log. */
> - qemu_notify_event();
> - }
> - /* Execution reached the break step */
> - if (replay_break_icount == replay_state.current_icount) {
> - /* Cannot make callback directly from the vCPU thread */
> - timer_mod_ns(replay_break_timer,
> - qemu_clock_get_ns(QEMU_CLOCK_REALTIME));
> - }
> + replay_advance_current_icount(replay_get_current_icount());
> }
> }
> }
> diff --git a/stubs/replay-tools.c b/stubs/replay-tools.c
> index c06b360e22..43296b3d4e 100644
> --- a/stubs/replay-tools.c
> +++ b/stubs/replay-tools.c
> @@ -13,7 +13,7 @@ int64_t replay_save_clock(unsigned int kind, int64_t clock, int64_t raw_icount)
> return 0;
> }
>
> -int64_t replay_read_clock(unsigned int kind)
> +int64_t replay_read_clock(unsigned int kind, int64_t raw_icount)
> {
> abort();
> return 0;
>
I can't honestly say I understand this very well, but it's all in
replay_mode != REPLAY_MODE_NONE so queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-02-16 16:16 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-16 12:51 [PATCH] replay: fix icount request when replaying clock access Pavel Dovgalyuk
2021-02-16 16:14 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.