diff mbox

[CFT,06/12] exit round-robin vcpu loop if cpu->stopped is true

Message ID 1297185509-20996-7-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Feb. 8, 2011, 5:18 p.m. UTC 
Sometimes vcpus are stopped directly without going through ->stop = 1.
Exit the VCPU execution loop in this case as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Jan Kiszka Feb. 8, 2011, 8:24 p.m. UTC | #1 
On 2011-02-08 18:18, Paolo Bonzini wrote:
> Sometimes vcpus are stopped directly without going through ->stop = 1.
> Exit the VCPU execution loop in this case as well.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cpus.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index c1d0ceb..5b13961 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1061,7 +1061,7 @@ bool cpu_exec_all(void)
>              if (r == EXCP_DEBUG) {
>                  break;
>              }
> -        } else if (env->stop) {
> +        } else if (env->stop || env->stopped) {
>              break;
>          }
>      }

Hmm, does this path actually trigger? If yes, does it happen to obsolete
the global exit_request hack?

Jan
Paolo Bonzini Feb. 9, 2011, 7:24 a.m. UTC | #2 
On 02/08/2011 09:24 PM, Jan Kiszka wrote:
> Hmm, does this path actually trigger? If yes, does it happen to obsolete
> the global exit_request hack?

No idea, I wanted to make the initial work as mechanical as possible. 
By inspection, cpu_stop_current is doing

          cpu_single_env->stopped = 1;
          cpu_exit(cpu_single_env);

without setting ->stop.  (I'm changing that in patch 7, but I'm setting 
->stop to 0, not 1).

Paolo
Paolo Bonzini Feb. 9, 2011, 7:24 a.m. UTC | #3 
On 02/08/2011 09:24 PM, Jan Kiszka wrote:
> Hmm, does this path actually trigger? If yes, does it happen to obsolete
> the global exit_request hack?

No idea, I wanted to make the initial work as mechanical as possible. 
By inspection, cpu_stop_current is doing

          cpu_single_env->stopped = 1;
          cpu_exit(cpu_single_env);

without setting ->stop.  (I'm changing that in patch 7, but I'm setting 
->stop to 0, not 1).

Paolo
Jan Kiszka Feb. 9, 2011, 8:40 a.m. UTC | #4 
On 2011-02-09 08:24, Paolo Bonzini wrote:
> On 02/08/2011 09:24 PM, Jan Kiszka wrote:
>> Hmm, does this path actually trigger? If yes, does it happen to obsolete
>> the global exit_request hack?
> 
> No idea, I wanted to make the initial work as mechanical as possible. By
> inspection, cpu_stop_current is doing
> 
>          cpu_single_env->stopped = 1;
>          cpu_exit(cpu_single_env);
> 
> without setting ->stop.  (I'm changing that in patch 7, but I'm setting
> ->stop to 0, not 1).

Checked my own patches again: :) exit_request is not obsoleted this way,
at least as long as we have !CONFIG_IOTHREAD hanging around.

Also, I don't see any compelling reason now why that test should be
unneeded. Rather, this looks like a sleeping race between asynchronous
and synchronous vcpu stop, probably papered over by polling the cond
variables so far.

Jan
diff mbox

Patch

diff --git a/cpus.c b/cpus.c
index c1d0ceb..5b13961 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1061,7 +1061,7 @@  bool cpu_exec_all(void)
             if (r == EXCP_DEBUG) {
                 break;
             }
-        } else if (env->stop) {
+        } else if (env->stop || env->stopped) {
             break;
         }
     }