1. Patchew
  2. QEMU
  3. View series

[PATCH] icount: fix shift=auto for record/replay

Pavel Dovgalyuk posted 1 patch 5 years, 6 months ago
Test asan passed
Test docker-mingw@fedora passed
Test checkpatch passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/158875154623.957.4036561733593052357.stgit@pasha-ThinkPad-X280
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>
There is a newer version of this series
cpus.c |    4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH] icount: fix shift=auto for record/replay
Posted by Pavel Dovgalyuk 5 years, 6 months ago 
This patch fixes shift=auto when record/replay is enabled.
Now user does not need to guess the best shift value.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 cpus.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 5670c96bcf..dfb9f4717f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -379,7 +379,8 @@ static void icount_adjust(void)
 
     seqlock_write_lock(&timers_state.vm_clock_seqlock,
                        &timers_state.vm_clock_lock);
-    cur_time = cpu_get_clock_locked();
+    cur_time = REPLAY_CLOCK_LOCKED(REPLAY_CLOCK_VIRTUAL_RT,
+                                   cpu_get_clock_locked());
     cur_icount = cpu_get_icount_locked();
 
     delta = cur_icount - cur_time;
@@ -685,6 +686,7 @@ static const VMStateDescription icount_vmstate_timers = {
     .fields = (VMStateField[]) {
         VMSTATE_INT64(qemu_icount_bias, TimersState),
         VMSTATE_INT64(qemu_icount, TimersState),
+        VMSTATE_INT16(icount_time_shift, TimersState),
         VMSTATE_END_OF_LIST()
     },
     .subsections = (const VMStateDescription*[]) {
Re: [PATCH] icount: fix shift=auto for record/replay
Posted by Pavel Dovgalyuk 5 years, 6 months ago 
ping


On 06.05.2020 10:52, Pavel Dovgalyuk wrote:
> This patch fixes shift=auto when record/replay is enabled.
> Now user does not need to guess the best shift value.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> ---
>   cpus.c |    4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/cpus.c b/cpus.c
> index 5670c96bcf..dfb9f4717f 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -379,7 +379,8 @@ static void icount_adjust(void)
>   
>       seqlock_write_lock(&timers_state.vm_clock_seqlock,
>                          &timers_state.vm_clock_lock);
> -    cur_time = cpu_get_clock_locked();
> +    cur_time = REPLAY_CLOCK_LOCKED(REPLAY_CLOCK_VIRTUAL_RT,
> +                                   cpu_get_clock_locked());
>       cur_icount = cpu_get_icount_locked();
>   
>       delta = cur_icount - cur_time;
> @@ -685,6 +686,7 @@ static const VMStateDescription icount_vmstate_timers = {
>       .fields = (VMStateField[]) {
>           VMSTATE_INT64(qemu_icount_bias, TimersState),
>           VMSTATE_INT64(qemu_icount, TimersState),
> +        VMSTATE_INT16(icount_time_shift, TimersState),
>           VMSTATE_END_OF_LIST()
>       },
>       .subsections = (const VMStateDescription*[]) {
>
Re: [PATCH] icount: fix shift=auto for record/replay
Posted by Philippe Mathieu-Daudé 5 years, 6 months ago 
+Alex

On 5/6/20 9:52 AM, Pavel Dovgalyuk wrote:
> This patch fixes shift=auto when record/replay is enabled.
> Now user does not need to guess the best shift value.
> 
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> ---
>   cpus.c |    4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 5670c96bcf..dfb9f4717f 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -379,7 +379,8 @@ static void icount_adjust(void)
>   
>       seqlock_write_lock(&timers_state.vm_clock_seqlock,
>                          &timers_state.vm_clock_lock);
> -    cur_time = cpu_get_clock_locked();
> +    cur_time = REPLAY_CLOCK_LOCKED(REPLAY_CLOCK_VIRTUAL_RT,
> +                                   cpu_get_clock_locked());
>       cur_icount = cpu_get_icount_locked();
>   
>       delta = cur_icount - cur_time;
> @@ -685,6 +686,7 @@ static const VMStateDescription icount_vmstate_timers = {
>       .fields = (VMStateField[]) {
>           VMSTATE_INT64(qemu_icount_bias, TimersState),
>           VMSTATE_INT64(qemu_icount, TimersState),
> +        VMSTATE_INT16(icount_time_shift, TimersState),
>           VMSTATE_END_OF_LIST()
>       },
>       .subsections = (const VMStateDescription*[]) {
> 
>
Re: [PATCH] icount: fix shift=auto for record/replay
Posted by Alex Bennée 5 years, 5 months ago 
Pavel Dovgalyuk <Pavel.Dovgaluk@gmail.com> writes:

> This patch fixes shift=auto when record/replay is enabled.
> Now user does not need to guess the best shift value.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> ---
>  cpus.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/cpus.c b/cpus.c
> index 5670c96bcf..dfb9f4717f 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -379,7 +379,8 @@ static void icount_adjust(void)
>  
>      seqlock_write_lock(&timers_state.vm_clock_seqlock,
>                         &timers_state.vm_clock_lock);
> -    cur_time = cpu_get_clock_locked();
> +    cur_time = REPLAY_CLOCK_LOCKED(REPLAY_CLOCK_VIRTUAL_RT,
> +                                   cpu_get_clock_locked());
>      cur_icount = cpu_get_icount_locked();
>  
>      delta = cur_icount - cur_time;
> @@ -685,6 +686,7 @@ static const VMStateDescription icount_vmstate_timers = {
>      .fields = (VMStateField[]) {
>          VMSTATE_INT64(qemu_icount_bias, TimersState),
>          VMSTATE_INT64(qemu_icount, TimersState),
> +        VMSTATE_INT16(icount_time_shift, TimersState),

Surely we should be bumping .version_id/.minimum_version_id here so we
error out gracefully. No recordings pre this change would work right?

>          VMSTATE_END_OF_LIST()
>      },
>      .subsections = (const VMStateDescription*[]) {


-- 
Alex Bennée
Re: [PATCH] icount: fix shift=auto for record/replay
Posted by Pavel Dovgalyuk 5 years, 5 months ago 
On 18.05.2020 19:14, Alex Bennée wrote:
> Pavel Dovgalyuk <Pavel.Dovgaluk@gmail.com> writes:
>
>> This patch fixes shift=auto when record/replay is enabled.
>> Now user does not need to guess the best shift value.
>>
>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>> ---
>>   cpus.c |    4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index 5670c96bcf..dfb9f4717f 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -379,7 +379,8 @@ static void icount_adjust(void)
>>   
>>       seqlock_write_lock(&timers_state.vm_clock_seqlock,
>>                          &timers_state.vm_clock_lock);
>> -    cur_time = cpu_get_clock_locked();
>> +    cur_time = REPLAY_CLOCK_LOCKED(REPLAY_CLOCK_VIRTUAL_RT,
>> +                                   cpu_get_clock_locked());
>>       cur_icount = cpu_get_icount_locked();
>>   
>>       delta = cur_icount - cur_time;
>> @@ -685,6 +686,7 @@ static const VMStateDescription icount_vmstate_timers = {
>>       .fields = (VMStateField[]) {
>>           VMSTATE_INT64(qemu_icount_bias, TimersState),
>>           VMSTATE_INT64(qemu_icount, TimersState),
>> +        VMSTATE_INT16(icount_time_shift, TimersState),
> Surely we should be bumping .version_id/.minimum_version_id here so we
> error out gracefully. No recordings pre this change would work right?


You're right. But before this patch shift=auto doesn't work for RR at 
all. Therefore no recordings should exist.

I think, for other icount use cases would be better to move 
icount_time_shift to subsection and use it only when auto is enabled.

>
>>           VMSTATE_END_OF_LIST()
>>       },
>>       .subsections = (const VMStateDescription*[]) {
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.