diff mbox

[1/3] icount: implement a new icount_no_rt mode without real time cpu sleeping

Message ID 1432727523-2008-2-git-send-email-victor.clement@openwide.fr
State New
Headers show

Commit Message

Victor CLEMENT May 27, 2015, 11:52 a.m. UTC
In this new icount_no_rt mode, the QEMU_VIRTUAL_CLOCK runs at the
maximum possible speed by warping the sleep times of the virtual cpu to the
soonest clock deadline. The virtual clock will be updated only according
the instruction counter.

Signed-off-by: Victor CLEMENT <victor.clement@openwide.fr>
---
 cpus.c | 64 ++++++++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 40 insertions(+), 24 deletions(-)

Comments

Paolo Bonzini May 27, 2015, 12:52 p.m. UTC | #1
On 27/05/2015 13:52, Victor CLEMENT wrote:
> In this new icount_no_rt mode, the QEMU_VIRTUAL_CLOCK runs at the
> maximum possible speed by warping the sleep times of the virtual cpu to the
> soonest clock deadline. The virtual clock will be updated only according
> the instruction counter.
> 
> Signed-off-by: Victor CLEMENT <victor.clement@openwide.fr>
> ---
>  cpus.c | 64 ++++++++++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 40 insertions(+), 24 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index de6469f..012d14b 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -105,6 +105,7 @@ static bool all_cpu_threads_idle(void)
>  
>  /* Protected by TimersState seqlock */
>  
> +static bool icount_no_rt;

It is somewhat hard to read the code due to double negations.  What
about "-icount sleep=[yes|no]" and naming the variable "icount_sleep"?

>  static int64_t vm_clock_warp_start = -1;
>  /* Conversion factor from emulated instructions to virtual clock ticks.  */
>  static int icount_time_shift;
> @@ -393,15 +394,18 @@ void qemu_clock_warp(QEMUClockType type)
>          return;
>      }
>  
> -    /*
> -     * If the CPUs have been sleeping, advance QEMU_CLOCK_VIRTUAL timer now.
> -     * This ensures that the deadline for the timer is computed correctly below.
> -     * This also makes sure that the insn counter is synchronized before the
> -     * CPU starts running, in case the CPU is woken by an event other than
> -     * the earliest QEMU_CLOCK_VIRTUAL timer.
> -     */
> -    icount_warp_rt(NULL);
> -    timer_del(icount_warp_timer);
> +    if (!icount_no_rt) {
> +        /*
> +         * If the CPUs have been sleeping, advance QEMU_CLOCK_VIRTUAL timer now.
> +         * This ensures that the deadline for the timer is computed correctly
> +         * below.
> +         * This also makes sure that the insn counter is synchronized before
> +         * the CPU starts running, in case the CPU is woken by an event other
> +         * than the earliest QEMU_CLOCK_VIRTUAL timer.
> +         */
> +        icount_warp_rt(NULL);
> +        timer_del(icount_warp_timer);
> +    }
>      if (!all_cpu_threads_idle()) {
>          return;
>      }
> @@ -425,23 +429,35 @@ void qemu_clock_warp(QEMUClockType type)
>           * interrupt to wake it up, but the interrupt never comes because
>           * the vCPU isn't running any insns and thus doesn't advance the
>           * QEMU_CLOCK_VIRTUAL.
> -         *
> -         * An extreme solution for this problem would be to never let VCPUs
> -         * sleep in icount mode if there is a pending QEMU_CLOCK_VIRTUAL
> -         * timer; rather time could just advance to the next QEMU_CLOCK_VIRTUAL
> -         * event.  Instead, we do stop VCPUs and only advance QEMU_CLOCK_VIRTUAL
> -         * after some "real" time, (related to the time left until the next
> -         * event) has passed. The QEMU_CLOCK_VIRTUAL_RT clock will do this.
> -         * This avoids that the warps are visible externally; for example,
> -         * you will not be sending network packets continuously instead of
> -         * every 100ms.
>           */
> -        seqlock_write_lock(&timers_state.vm_clock_seqlock);
> -        if (vm_clock_warp_start == -1 || vm_clock_warp_start > clock) {
> -            vm_clock_warp_start = clock;
> +        if (icount_no_rt) {
> +            /*
> +             * We never let VCPUs sleep in async icount mode.

s/async icount/sleep=no/

?

Otherwise the series looks good.

Paolo

> +             * If there is a pending QEMU_CLOCK_VIRTUAL timer we just advance
> +             * to the next QEMU_CLOCK_VIRTUAL event and notify it.
> +             * It is useful when we want a deterministic execution time,
> +             * isolated from host latencies.
> +             */
> +            seqlock_write_lock(&timers_state.vm_clock_seqlock);
> +            timers_state.qemu_icount_bias += deadline;
> +            seqlock_write_unlock(&timers_state.vm_clock_seqlock);
> +            qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
> +        } else {
> +            /*
> +             * We do stop VCPUs and only advance QEMU_CLOCK_VIRTUAL after some
> +             * "real" time, (related to the time left until the next event) has
> +             * passed. The QEMU_CLOCK_VIRTUAL_RT clock will do this.
> +             * This avoids that the warps are visible externally; for example,
> +             * you will not be sending network packets continuously instead of
> +             * every 100ms.
> +             */
> +            seqlock_write_lock(&timers_state.vm_clock_seqlock);
> +            if (vm_clock_warp_start == -1 || vm_clock_warp_start > clock) {
> +                vm_clock_warp_start = clock;
> +            }
> +            seqlock_write_unlock(&timers_state.vm_clock_seqlock);
> +            timer_mod_anticipate(icount_warp_timer, clock + deadline);
>          }
> -        seqlock_write_unlock(&timers_state.vm_clock_seqlock);
> -        timer_mod_anticipate(icount_warp_timer, clock + deadline);
>      } else if (deadline == 0) {
>          qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
>      }
>
Victor CLEMENT May 29, 2015, 8:59 a.m. UTC | #2
----- Mail original -----
> De: "Paolo Bonzini" <pbonzini@redhat.com>
> À: "Victor CLEMENT" <victor.clement@openwide.fr>, qemu-devel@nongnu.org
> Cc: "francois guerret" <francois.guerret@hotmail.fr>
> Envoyé: Mercredi 27 Mai 2015 14:52:00
> Objet: Re: [Qemu-devel] [PATCH 1/3] icount: implement a new icount_no_rt mode without real time cpu sleeping
> 
> 
> 
> On 27/05/2015 13:52, Victor CLEMENT wrote:
> > In this new icount_no_rt mode, the QEMU_VIRTUAL_CLOCK runs at the
> > maximum possible speed by warping the sleep times of the virtual
> > cpu to the
> > soonest clock deadline. The virtual clock will be updated only
> > according
> > the instruction counter.
> > 
> > Signed-off-by: Victor CLEMENT <victor.clement@openwide.fr>
> > ---
> >  cpus.c | 64
> >  ++++++++++++++++++++++++++++++++++++++++------------------------
> >  1 file changed, 40 insertions(+), 24 deletions(-)
> > 
> > diff --git a/cpus.c b/cpus.c
> > index de6469f..012d14b 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -105,6 +105,7 @@ static bool all_cpu_threads_idle(void)
> >  
> >  /* Protected by TimersState seqlock */
> >  
> > +static bool icount_no_rt;
> 
> It is somewhat hard to read the code due to double negations.  What
> about "-icount sleep=[yes|no]" and naming the variable
> "icount_sleep"?

You are right, the "nort" can be ambiguous. icount_sleep seems clear.
Thanks for the suggestion.

> [...]
> > +        if (icount_no_rt) {
> > +            /*
> > +             * We never let VCPUs sleep in async icount mode.
> 
> s/async icount/sleep=no/
> 
> ?

Right ! My bad...

> 
> Otherwise the series looks good.
> 
> Paolo

Ok, I sumbit the v2 with icount_sleep naming.

Victor

> 
> > +             * If there is a pending QEMU_CLOCK_VIRTUAL timer we
> > just advance
> > +             * to the next QEMU_CLOCK_VIRTUAL event and notify it.
> > +             * It is useful when we want a deterministic execution
> > time,
> > +             * isolated from host latencies.
> > +             */
> > +            seqlock_write_lock(&timers_state.vm_clock_seqlock);
> > +            timers_state.qemu_icount_bias += deadline;
> > +            seqlock_write_unlock(&timers_state.vm_clock_seqlock);
> > +            qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
> > +        } else {
> > +            /*
> > +             * We do stop VCPUs and only advance
> > QEMU_CLOCK_VIRTUAL after some
> > +             * "real" time, (related to the time left until the
> > next event) has
> > +             * passed. The QEMU_CLOCK_VIRTUAL_RT clock will do
> > this.
> > +             * This avoids that the warps are visible externally;
> > for example,
> > +             * you will not be sending network packets
> > continuously instead of
> > +             * every 100ms.
> > +             */
> > +            seqlock_write_lock(&timers_state.vm_clock_seqlock);
> > +            if (vm_clock_warp_start == -1 || vm_clock_warp_start >
> > clock) {
> > +                vm_clock_warp_start = clock;
> > +            }
> > +            seqlock_write_unlock(&timers_state.vm_clock_seqlock);
> > +            timer_mod_anticipate(icount_warp_timer, clock +
> > deadline);
> >          }
> > -        seqlock_write_unlock(&timers_state.vm_clock_seqlock);
> > -        timer_mod_anticipate(icount_warp_timer, clock + deadline);
> >      } else if (deadline == 0) {
> >          qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
> >      }
> > 
> 
>
diff mbox

Patch

diff --git a/cpus.c b/cpus.c
index de6469f..012d14b 100644
--- a/cpus.c
+++ b/cpus.c
@@ -105,6 +105,7 @@  static bool all_cpu_threads_idle(void)
 
 /* Protected by TimersState seqlock */
 
+static bool icount_no_rt;
 static int64_t vm_clock_warp_start = -1;
 /* Conversion factor from emulated instructions to virtual clock ticks.  */
 static int icount_time_shift;
@@ -393,15 +394,18 @@  void qemu_clock_warp(QEMUClockType type)
         return;
     }
 
-    /*
-     * If the CPUs have been sleeping, advance QEMU_CLOCK_VIRTUAL timer now.
-     * This ensures that the deadline for the timer is computed correctly below.
-     * This also makes sure that the insn counter is synchronized before the
-     * CPU starts running, in case the CPU is woken by an event other than
-     * the earliest QEMU_CLOCK_VIRTUAL timer.
-     */
-    icount_warp_rt(NULL);
-    timer_del(icount_warp_timer);
+    if (!icount_no_rt) {
+        /*
+         * If the CPUs have been sleeping, advance QEMU_CLOCK_VIRTUAL timer now.
+         * This ensures that the deadline for the timer is computed correctly
+         * below.
+         * This also makes sure that the insn counter is synchronized before
+         * the CPU starts running, in case the CPU is woken by an event other
+         * than the earliest QEMU_CLOCK_VIRTUAL timer.
+         */
+        icount_warp_rt(NULL);
+        timer_del(icount_warp_timer);
+    }
     if (!all_cpu_threads_idle()) {
         return;
     }
@@ -425,23 +429,35 @@  void qemu_clock_warp(QEMUClockType type)
          * interrupt to wake it up, but the interrupt never comes because
          * the vCPU isn't running any insns and thus doesn't advance the
          * QEMU_CLOCK_VIRTUAL.
-         *
-         * An extreme solution for this problem would be to never let VCPUs
-         * sleep in icount mode if there is a pending QEMU_CLOCK_VIRTUAL
-         * timer; rather time could just advance to the next QEMU_CLOCK_VIRTUAL
-         * event.  Instead, we do stop VCPUs and only advance QEMU_CLOCK_VIRTUAL
-         * after some "real" time, (related to the time left until the next
-         * event) has passed. The QEMU_CLOCK_VIRTUAL_RT clock will do this.
-         * This avoids that the warps are visible externally; for example,
-         * you will not be sending network packets continuously instead of
-         * every 100ms.
          */
-        seqlock_write_lock(&timers_state.vm_clock_seqlock);
-        if (vm_clock_warp_start == -1 || vm_clock_warp_start > clock) {
-            vm_clock_warp_start = clock;
+        if (icount_no_rt) {
+            /*
+             * We never let VCPUs sleep in async icount mode.
+             * If there is a pending QEMU_CLOCK_VIRTUAL timer we just advance
+             * to the next QEMU_CLOCK_VIRTUAL event and notify it.
+             * It is useful when we want a deterministic execution time,
+             * isolated from host latencies.
+             */
+            seqlock_write_lock(&timers_state.vm_clock_seqlock);
+            timers_state.qemu_icount_bias += deadline;
+            seqlock_write_unlock(&timers_state.vm_clock_seqlock);
+            qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
+        } else {
+            /*
+             * We do stop VCPUs and only advance QEMU_CLOCK_VIRTUAL after some
+             * "real" time, (related to the time left until the next event) has
+             * passed. The QEMU_CLOCK_VIRTUAL_RT clock will do this.
+             * This avoids that the warps are visible externally; for example,
+             * you will not be sending network packets continuously instead of
+             * every 100ms.
+             */
+            seqlock_write_lock(&timers_state.vm_clock_seqlock);
+            if (vm_clock_warp_start == -1 || vm_clock_warp_start > clock) {
+                vm_clock_warp_start = clock;
+            }
+            seqlock_write_unlock(&timers_state.vm_clock_seqlock);
+            timer_mod_anticipate(icount_warp_timer, clock + deadline);
         }
-        seqlock_write_unlock(&timers_state.vm_clock_seqlock);
-        timer_mod_anticipate(icount_warp_timer, clock + deadline);
     } else if (deadline == 0) {
         qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
     }