Patchwork fix 100% cpu utilization when cpu is stopped (was Re: d6f4ade214a9f74dca9495b83a24ff9c113e4f9a)

login
register
mail settings
Submitter Paolo Bonzini
Date April 6, 2010, 8:16 p.m.
Message ID <4BBB9685.9090801@redhat.com>
Download mbox | patch
Permalink /patch/49537/
State New
Headers show

Comments

Paolo Bonzini - April 6, 2010, 8:16 p.m.
On 04/05/2010 03:51 PM, malc wrote:
> Hello,
>
> d6f4ade214a9f74dca9495b83a24ff9c113e4f9a: disentangle tcg and deadline calculation
> introduces following regression(s):
>
> 100% cpu utilization when QEMU is invoked like:
> qemu -S -s ...
>
> ditto when gdb takes control over the session via gdb-stub
> (i.e. the breakpoint is hit or C-c is pressed inside gdb to
>   interrupt the attached qemu instance)

The bug is that env->stopped is not really as comprehensive as it seems 
to be (and cpu_has_work thinks); it is only valid with iothread 
basically, and even then it is cleared by reset and it is not set when 
starting qemu with -S.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
malc - April 6, 2010, 9:17 p.m.
On Tue, 6 Apr 2010, Paolo Bonzini wrote:

> On 04/05/2010 03:51 PM, malc wrote:
> > Hello,
> > 
> > d6f4ade214a9f74dca9495b83a24ff9c113e4f9a: disentangle tcg and deadline
> > calculation
> > introduces following regression(s):
> > 
> > 100% cpu utilization when QEMU is invoked like:
> > qemu -S -s ...
> > 
> > ditto when gdb takes control over the session via gdb-stub
> > (i.e. the breakpoint is hit or C-c is pressed inside gdb to
> >   interrupt the attached qemu instance)
> 
> The bug is that env->stopped is not really as comprehensive as it seems to be
> (and cpu_has_work thinks); it is only valid with iothread basically, and even
> then it is cleared by reset and it is not set when starting qemu with -S.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> diff --git a/cpus.c b/cpus.c
> index a2e0642..0debe77 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -100,9 +100,7 @@ static int cpu_can_run(CPUState *env)
>  {
>      if (env->stop)
>          return 0;
> -    if (env->stopped)
> -        return 0;
> -    if (!vm_running)
> +    if (env->stopped || !vm_running)
>          return 0;
>      return 1;
>  }
> @@ -111,7 +109,7 @@ static int cpu_has_work(CPUState *env)
>  {
>      if (env->stop)
>          return 1;
> -    if (env->stopped)
> +    if (env->stopped || !vm_running)
>          return 0;
>      if (!env->halted)
>          return 1;
> 

This doesn't apply for whatever reason, i patched the thing by hand
and things seem to work, thanks, care to resend?

Patch

diff --git a/cpus.c b/cpus.c
index a2e0642..0debe77 100644
--- a/cpus.c
+++ b/cpus.c
@@ -100,9 +100,7 @@  static int cpu_can_run(CPUState *env)
  {
      if (env->stop)
          return 0;
-    if (env->stopped)
-        return 0;
-    if (!vm_running)
+    if (env->stopped || !vm_running)
          return 0;
      return 1;
  }
@@ -111,7 +109,7 @@  static int cpu_has_work(CPUState *env)
  {
      if (env->stop)
          return 1;
-    if (env->stopped)
+    if (env->stopped || !vm_running)
          return 0;
      if (!env->halted)
          return 1;