[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] kvm: Prevent dynticks race condition for !CONFI
From: |
Jan Kiszka |
Subject: |
[Qemu-devel] Re: [PATCH] kvm: Prevent dynticks race condition for !CONFIG_IOTHREAD |
Date: |
Wed, 26 Jan 2011 14:12:04 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2011-01-26 10:39, Stefan Hajnoczi wrote:
> The dynticks timer arranges for SIGALRM to be raised when the next
> pending timer expires. When building with !CONFIG_IOTHREAD, we need to
> check whether a request to exit the vcpu is pending before re-entering
> the guest.
>
> Unfortunately there is a race condition here because SIGALRM may be
> raised after we check for an exit request but before re-entering the
> guest. In that case the guest is re-entered without the dynticks timer
> being rearmed.
>
> This results in temporary loss of timers until some other event forces a
> vmexit. In the case of a CPU-bound guest it can cause softlockups.
>
> This patch blocks SIGALRM before checking for an exit request and uses
> KVM's sigmask support to atomically unblock it when entering the guest,
> thereby making the exit request check safe.
>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
> cpus.c | 17 ++++++++++++++++-
> kvm-all.c | 16 ++++++++++++++++
> 2 files changed, 32 insertions(+), 1 deletions(-)
>
> Does not affect qemu-kvm.git. Still worth having in qemu.git so we don't get
> odd behavior when building without --enable-io-thread.
>
> diff --git a/cpus.c b/cpus.c
> index 0309189..59dbfab 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -262,14 +262,29 @@ void qemu_main_loop_start(void)
> {
> }
>
> +static void kvm_init_sigmask(CPUState *env)
> +{
> + int r;
> + sigset_t set;
> +
> + pthread_sigmask(SIG_SETMASK, NULL, &set);
> + r = kvm_set_signal_mask(env, &set);
> + if (r) {
> + fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(r));
> + exit(1);
> + }
> +}
> +
> void qemu_init_vcpu(void *_env)
> {
> CPUState *env = _env;
>
> env->nr_cores = smp_cores;
> env->nr_threads = smp_threads;
> - if (kvm_enabled())
> + if (kvm_enabled()) {
> kvm_init_vcpu(env);
> + kvm_init_sigmask(env);
> + }
> return;
> }
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 255b6fa..9cc2553 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -890,19 +890,31 @@ int kvm_cpu_exec(CPUState *env)
> {
> struct kvm_run *run = env->kvm_run;
> int ret;
> +#ifndef CONFIG_IOTHREAD
> + sigset_t set, old_set;
> +
> + sigemptyset(&set);
> + sigaddset(&set, SIGALRM);
> +#endif
>
> DPRINTF("kvm_cpu_exec()\n");
>
> do {
> #ifndef CONFIG_IOTHREAD
> + pthread_sigmask(SIG_BLOCK, &set, &old_set);
> +
> if (env->exit_request) {
> DPRINTF("interrupt exit requested\n");
> + pthread_sigmask(SIG_SETMASK, &old_set, NULL);
> ret = 0;
> break;
> }
> #endif
>
> if (kvm_arch_process_irqchip_events(env)) {
> +#ifndef CONFIG_IOTHREAD
> + pthread_sigmask(SIG_SETMASK, &old_set, NULL);
> +#endif
> ret = 0;
> break;
> }
> @@ -920,6 +932,10 @@ int kvm_cpu_exec(CPUState *env)
> cpu_single_env = env;
> kvm_arch_post_run(env, run);
>
> +#ifndef CONFIG_IOTHREAD
> + pthread_sigmask(SIG_SETMASK, &old_set, NULL);
> +#endif
> +
> if (ret == -EINTR || ret == -EAGAIN) {
> cpu_exit(env);
> DPRINTF("io window exit\n");
Good catch, but the code should be organized differently, probably with
the help of signalfd which we need for SIGBUS anyway. I'm currently
reworking signaling bits. I will pick this up and integrate a
corresponding solution in my queue. Will keep you posted!
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux