All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] qtest: fix qtest_clock_warp() for no deadline case
@ 2014-06-09 16:42 Sergey Fedorov
  2014-06-09 17:36 ` Alex Bligh
  0 siblings, 1 reply; 5+ messages in thread
From: Sergey Fedorov @ 2014-06-09 16:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bligh, Stefan Hajnoczi, Paolo Bonzini, Sergey Fedorov,
	Andreas Färber, Richard Henderson

If there is no deadline across all timerlists attached to the clock
then qemu_clock_deadline_ns_all() returns -1. Cast it to unsinged so
MIN() do not treat it as minimum.

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
---
 cpus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index dd7ac13..3ec15cb 100644
--- a/cpus.c
+++ b/cpus.c
@@ -346,8 +346,8 @@ void qtest_clock_warp(int64_t dest)
     int64_t clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     assert(qtest_enabled());
     while (clock < dest) {
-        int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
-        int64_t warp = MIN(dest - clock, deadline);
+        uint64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
+        uint64_t warp = MIN(dest - clock, deadline);
         seqlock_write_lock(&timers_state.vm_clock_seqlock);
         qemu_icount_bias += warp;
         seqlock_write_unlock(&timers_state.vm_clock_seqlock);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] qtest: fix qtest_clock_warp() for no deadline case
  2014-06-09 16:42 [Qemu-devel] [PATCH] qtest: fix qtest_clock_warp() for no deadline case Sergey Fedorov
@ 2014-06-09 17:36 ` Alex Bligh
  2014-06-10  9:08   ` Sergey Fedorov
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Bligh @ 2014-06-09 17:36 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: Alex Bligh, qemu-devel, Stefan Hajnoczi, Paolo Bonzini,
	Andreas Färber, Richard Henderson


On 9 Jun 2014, at 17:42, Sergey Fedorov wrote:

> If there is no deadline across all timerlists attached to the clock
> then qemu_clock_deadline_ns_all() returns -1. Cast it to unsinged so
> MIN() do not treat it as minimum.
> 
> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> ---
> cpus.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index dd7ac13..3ec15cb 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -346,8 +346,8 @@ void qtest_clock_warp(int64_t dest)
>     int64_t clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>     assert(qtest_enabled());
>     while (clock < dest) {
> -        int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
> -        int64_t warp = MIN(dest - clock, deadline);
> +        uint64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
> +        uint64_t warp = MIN(dest - clock, deadline);

Please don't do that. It looks like a bug where you've not read the return
type.

Instead, just do

int64_t warp = qemu_soonest_timeout(dest - clock, deadline);

which puts all the ugly casting stuff in one nicely documented inline function
and will generate the same code.

>         seqlock_write_lock(&timers_state.vm_clock_seqlock);
>         qemu_icount_bias += warp;
>         seqlock_write_unlock(&timers_state.vm_clock_seqlock);
> -- 
> 1.9.1
> 
> 
> 

-- 
Alex Bligh

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] qtest: fix qtest_clock_warp() for no deadline case
  2014-06-09 17:36 ` Alex Bligh
@ 2014-06-10  9:08   ` Sergey Fedorov
  0 siblings, 0 replies; 5+ messages in thread
From: Sergey Fedorov @ 2014-06-10  9:08 UTC (permalink / raw)
  To: Alex Bligh
  Cc: Paolo Bonzini, Richard Henderson, qemu-devel, Stefan Hajnoczi,
	Andreas Färber

On 09.06.2014 21:36, Alex Bligh wrote:
> On 9 Jun 2014, at 17:42, Sergey Fedorov wrote:
>
>> If there is no deadline across all timerlists attached to the clock
>> then qemu_clock_deadline_ns_all() returns -1. Cast it to unsinged so
>> MIN() do not treat it as minimum.
>>
>> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
>> ---
>> cpus.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index dd7ac13..3ec15cb 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -346,8 +346,8 @@ void qtest_clock_warp(int64_t dest)
>>     int64_t clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>>     assert(qtest_enabled());
>>     while (clock < dest) {
>> -        int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
>> -        int64_t warp = MIN(dest - clock, deadline);
>> +        uint64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
>> +        uint64_t warp = MIN(dest - clock, deadline);
> Please don't do that. It looks like a bug where you've not read the return
> type.
>
> Instead, just do
>
> int64_t warp = qemu_soonest_timeout(dest - clock, deadline);
>
> which puts all the ugly casting stuff in one nicely documented inline function
> and will generate the same code.

Thank you! I'm resubmitting the patch.

>
>>         seqlock_write_lock(&timers_state.vm_clock_seqlock);
>>         qemu_icount_bias += warp;
>>         seqlock_write_unlock(&timers_state.vm_clock_seqlock);
>> -- 
>> 1.9.1
>>
>>
>>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] qtest: fix qtest_clock_warp() for no deadline case
  2014-06-10  9:10 Sergey Fedorov
@ 2014-06-10 11:03 ` Paolo Bonzini
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2014-06-10 11:03 UTC (permalink / raw)
  To: Sergey Fedorov, qemu-devel
  Cc: Richard Henderson, Stefan Hajnoczi, Alex Bligh, Andreas Färber

Il 10/06/2014 11:10, Sergey Fedorov ha scritto:
> Use dedicated qemu_soonest_timeout() instead of MIN().
>
> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> ---
>  cpus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cpus.c b/cpus.c
> index dd7ac13..af06dc0 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -347,7 +347,7 @@ void qtest_clock_warp(int64_t dest)
>      assert(qtest_enabled());
>      while (clock < dest) {
>          int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
> -        int64_t warp = MIN(dest - clock, deadline);
> +        int64_t warp = qemu_soonest_timeout(dest - clock, deadline);
>          seqlock_write_lock(&timers_state.vm_clock_seqlock);
>          qemu_icount_bias += warp;
>          seqlock_write_unlock(&timers_state.vm_clock_seqlock);
>

Thanks, I will make sure this gets into QEMU 2.1.

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Qemu-devel] [PATCH] qtest: fix qtest_clock_warp() for no deadline case
@ 2014-06-10  9:10 Sergey Fedorov
  2014-06-10 11:03 ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Sergey Fedorov @ 2014-06-10  9:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bligh, Stefan Hajnoczi, Paolo Bonzini, Sergey Fedorov,
	Andreas Färber, Richard Henderson

Use dedicated qemu_soonest_timeout() instead of MIN().

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
---
 cpus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index dd7ac13..af06dc0 100644
--- a/cpus.c
+++ b/cpus.c
@@ -347,7 +347,7 @@ void qtest_clock_warp(int64_t dest)
     assert(qtest_enabled());
     while (clock < dest) {
         int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
-        int64_t warp = MIN(dest - clock, deadline);
+        int64_t warp = qemu_soonest_timeout(dest - clock, deadline);
         seqlock_write_lock(&timers_state.vm_clock_seqlock);
         qemu_icount_bias += warp;
         seqlock_write_unlock(&timers_state.vm_clock_seqlock);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-06-10 11:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-09 16:42 [Qemu-devel] [PATCH] qtest: fix qtest_clock_warp() for no deadline case Sergey Fedorov
2014-06-09 17:36 ` Alex Bligh
2014-06-10  9:08   ` Sergey Fedorov
2014-06-10  9:10 Sergey Fedorov
2014-06-10 11:03 ` 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.