Re: [Qemu-devel] [PATCH 1/3] icount: implement a new icount_no_rt mode without real time cpu sleeping

[Victor Clement <victor.clement@openwide.fr>]

----- 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);
> >      }
> > 
> 
>