[Qemu-devel] Re: [7241] qemu: refactor main_loop (Marcelo Tosatti)

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 3109 bytes --]

Anthony Liguori wrote:
> Revision: 7241
>           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=7241
> Author:   aliguori
> Date:     2009-04-24 18:03:33 +0000 (Fri, 24 Apr 2009)
> Log Message:
> -----------
> qemu: refactor main_loop (Marcelo Tosatti)
> 
> Break main loop into 3 main functions.

I suspect this patch comes with a race between SIGALRM and
qemu_calculate_timeout() so that I see occasional freezes of exactly 5
seconds if the IO thread is disabled. I do not yet understand what
happens precisely or if this patch only widens an already existing race
window in the old code, but I'm on it.

Besides that...

> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> Modified Paths:
> --------------
>     trunk/vl.c
> 
> Modified: trunk/vl.c
> ===================================================================
> --- trunk/vl.c	2009-04-24 18:03:29 UTC (rev 7240)
> +++ trunk/vl.c	2009-04-24 18:03:33 UTC (rev 7241)
> @@ -273,7 +273,7 @@
>  
>  static CPUState *cur_cpu;
>  static CPUState *next_cpu;
> -static int event_pending = 1;
> +static int timer_alarm_pending = 1;
>  /* Conversion factor from emulated instructions to virtual clock ticks.  */
>  static int icount_time_shift;
>  /* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
> @@ -1360,7 +1360,7 @@
>              }
>  #endif
>          }
> -        event_pending = 1;
> +        timer_alarm_pending = 1;
>          qemu_notify_event();
>      }
>  }
> @@ -3879,153 +3879,175 @@
>  
>  }
>  
> -static int main_loop(void)
> +static int qemu_cpu_exec(CPUState *env)
>  {
> -    int ret, timeout;
> +    int ret;
>  #ifdef CONFIG_PROFILER
>      int64_t ti;
>  #endif
> +
> +#ifdef CONFIG_PROFILER
> +    ti = profile_getclock();
> +#endif
> +    if (use_icount) {
> +        int64_t count;
> +        int decr;
> +        qemu_icount -= (env->icount_decr.u16.low + env->icount_extra);
> +        env->icount_decr.u16.low = 0;
> +        env->icount_extra = 0;
> +        count = qemu_next_deadline();
> +        count = (count + (1 << icount_time_shift) - 1)
> +                >> icount_time_shift;
> +        qemu_icount += count;
> +        decr = (count > 0xffff) ? 0xffff : count;
> +        count -= decr;
> +        env->icount_decr.u16.low = decr;
> +        env->icount_extra = count;
> +    }
> +    ret = cpu_exec(env);
> +#ifdef CONFIG_PROFILER
> +    qemu_time += profile_getclock() - ti;
> +#endif
> +    if (use_icount) {
> +        /* Fold pending instructions back into the
> +           instruction counter, and clear the interrupt flag.  */
> +        qemu_icount -= (env->icount_decr.u16.low
> +                        + env->icount_extra);
> +        env->icount_decr.u32 = 0;
> +        env->icount_extra = 0;
> +    }
> +    return ret;
> +}
> +
> +static int cpu_has_work(CPUState *env)

...this naming is suboptimal. There is already cpu_has_work() in
target-*/exec.h which is at least confusing. Please rename.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]