Message ID | 1297185509-20996-7-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
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
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
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
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 --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; } }
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(-)