All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gleb Natapov <gleb@redhat.com>
To: Anthony Liguori <aliguori@us.ibm.com>
Cc: qemu-devel@nongnu.org, Chris Wright <chrisw@sous-sol.org>,
	kvm@vger.kernel.org
Subject: Re: [PATCH] qemu-kvm: introduce cpu_start/cpu_stop commands
Date: Tue, 23 Nov 2010 09:29:39 +0200	[thread overview]
Message-ID: <20101123072939.GS7948@redhat.com> (raw)
In-Reply-To: <1290466818-5230-1-git-send-email-aliguori@us.ibm.com>

On Mon, Nov 22, 2010 at 05:00:18PM -0600, Anthony Liguori wrote:
> qemu-kvm vcpu threads don't response to SIGSTOP/SIGCONT.  Instead of teaching
> them to respond to these signals, introduce monitor commands that stop and start
> individual vcpus.
> 
> The purpose of these commands are to implement CPU hard limits using an external
> tool that watches the CPU consumption and stops the CPU as appropriate.
> 
> The monitor commands provide a more elegant solution that signals because it
> ensures that a stopped vcpu isn't holding the qemu_mutex.
> 
Do you really want to stop vcpu while it holds guest lock? Does external tool
have enough info to make smart decision about how to limit vcpu runtime.

> I'll reply to this note with an example tool.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index ba6de28..827bd67 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -279,6 +279,24 @@ Resume emulation.
>  ETEXI
>  
>      {
> +        .name       = "cpu_start",
> +        .args_type  = "cpu:i",
> +        .params     = "[cpu]",
> +        .help       = "start cpu emulation",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_vcpu_start,
> +    },
> +
> +    {
> +        .name       = "cpu_stop",
> +        .args_type  = "cpu:i",
> +        .params     = "[cpu]",
> +        .help       = "stop cpu emulation",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_vcpu_stop,
> +    },
> +
> +    {
>          .name       = "gdbserver",
>          .args_type  = "device:s?",
>          .params     = "[device]",
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index 471306b..35121ed 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -1351,6 +1351,65 @@ static void pause_all_threads(void)
>      }
>  }
>  
> +static void vcpu_stop(int cpu)
> +{
> +    CPUState *env = first_cpu;
> +
> +    for (env = first_cpu; env; env = env->next_cpu) {
> +        if (env->cpu_index == cpu) {
> +            break;
> +        }
> +    }
> +
> +    if (env) {
> +        if (env != cpu_single_env) {
> +            env->stop = 1;
> +            pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
> +        } else {
> +            env->stop = 0;
> +            env->stopped = 1;
> +            cpu_exit(env);
> +        }
> +
> +        while (!env->stopped) {
> +            qemu_cond_wait(&qemu_pause_cond);
> +        }
> +    }
> +}
> +
> +static void vcpu_start(int cpu)
> +{
> +    CPUState *env = first_cpu;
> +
> +    assert(!cpu_single_env);
> +
> +    for (env = first_cpu; env; env = env->next_cpu) {
> +        if (env->cpu_index == cpu) {
> +            break;
> +        }
> +    }
> +
> +    if (env) {
> +        env->stop = 0;
> +        env->stopped = 0;
> +        pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
> +    }
> +}
> +
> +int do_vcpu_stop(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +{
> +    int vcpu = qdict_get_int(qdict, "cpu");
> +    vcpu_stop(vcpu);
> +    return 0;
> +}
> +
> +int do_vcpu_start(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +{
> +    int vcpu = qdict_get_int(qdict, "cpu");
> +    vcpu_start(vcpu);
> +    return 0;
> +}
> +
>  static void resume_all_threads(void)
>  {
>      CPUState *penv = first_cpu;
> diff --git a/sysemu.h b/sysemu.h
> index 849dc8c..3ef68dd 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -61,6 +61,9 @@ void qemu_system_reset(void);
>  void qemu_add_exit_notifier(Notifier *notify);
>  void qemu_remove_exit_notifier(Notifier *notify);
>  
> +int do_vcpu_stop(Monitor *mon, const QDict *qdict, QObject **ret_data);
> +int do_vcpu_start(Monitor *mon, const QDict *qdict, QObject **ret_data);
> +
>  void do_savevm(Monitor *mon, const QDict *qdict);
>  int load_vmstate(const char *name);
>  void do_delvm(Monitor *mon, const QDict *qdict);
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.

WARNING: multiple messages have this Message-ID (diff)
From: Gleb Natapov <gleb@redhat.com>
To: Anthony Liguori <aliguori@us.ibm.com>
Cc: Chris Wright <chrisw@sous-sol.org>,
	qemu-devel@nongnu.org, kvm@vger.kernel.org
Subject: [Qemu-devel] Re: [PATCH] qemu-kvm: introduce cpu_start/cpu_stop commands
Date: Tue, 23 Nov 2010 09:29:39 +0200	[thread overview]
Message-ID: <20101123072939.GS7948@redhat.com> (raw)
In-Reply-To: <1290466818-5230-1-git-send-email-aliguori@us.ibm.com>

On Mon, Nov 22, 2010 at 05:00:18PM -0600, Anthony Liguori wrote:
> qemu-kvm vcpu threads don't response to SIGSTOP/SIGCONT.  Instead of teaching
> them to respond to these signals, introduce monitor commands that stop and start
> individual vcpus.
> 
> The purpose of these commands are to implement CPU hard limits using an external
> tool that watches the CPU consumption and stops the CPU as appropriate.
> 
> The monitor commands provide a more elegant solution that signals because it
> ensures that a stopped vcpu isn't holding the qemu_mutex.
> 
Do you really want to stop vcpu while it holds guest lock? Does external tool
have enough info to make smart decision about how to limit vcpu runtime.

> I'll reply to this note with an example tool.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index ba6de28..827bd67 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -279,6 +279,24 @@ Resume emulation.
>  ETEXI
>  
>      {
> +        .name       = "cpu_start",
> +        .args_type  = "cpu:i",
> +        .params     = "[cpu]",
> +        .help       = "start cpu emulation",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_vcpu_start,
> +    },
> +
> +    {
> +        .name       = "cpu_stop",
> +        .args_type  = "cpu:i",
> +        .params     = "[cpu]",
> +        .help       = "stop cpu emulation",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_vcpu_stop,
> +    },
> +
> +    {
>          .name       = "gdbserver",
>          .args_type  = "device:s?",
>          .params     = "[device]",
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index 471306b..35121ed 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -1351,6 +1351,65 @@ static void pause_all_threads(void)
>      }
>  }
>  
> +static void vcpu_stop(int cpu)
> +{
> +    CPUState *env = first_cpu;
> +
> +    for (env = first_cpu; env; env = env->next_cpu) {
> +        if (env->cpu_index == cpu) {
> +            break;
> +        }
> +    }
> +
> +    if (env) {
> +        if (env != cpu_single_env) {
> +            env->stop = 1;
> +            pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
> +        } else {
> +            env->stop = 0;
> +            env->stopped = 1;
> +            cpu_exit(env);
> +        }
> +
> +        while (!env->stopped) {
> +            qemu_cond_wait(&qemu_pause_cond);
> +        }
> +    }
> +}
> +
> +static void vcpu_start(int cpu)
> +{
> +    CPUState *env = first_cpu;
> +
> +    assert(!cpu_single_env);
> +
> +    for (env = first_cpu; env; env = env->next_cpu) {
> +        if (env->cpu_index == cpu) {
> +            break;
> +        }
> +    }
> +
> +    if (env) {
> +        env->stop = 0;
> +        env->stopped = 0;
> +        pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
> +    }
> +}
> +
> +int do_vcpu_stop(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +{
> +    int vcpu = qdict_get_int(qdict, "cpu");
> +    vcpu_stop(vcpu);
> +    return 0;
> +}
> +
> +int do_vcpu_start(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +{
> +    int vcpu = qdict_get_int(qdict, "cpu");
> +    vcpu_start(vcpu);
> +    return 0;
> +}
> +
>  static void resume_all_threads(void)
>  {
>      CPUState *penv = first_cpu;
> diff --git a/sysemu.h b/sysemu.h
> index 849dc8c..3ef68dd 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -61,6 +61,9 @@ void qemu_system_reset(void);
>  void qemu_add_exit_notifier(Notifier *notify);
>  void qemu_remove_exit_notifier(Notifier *notify);
>  
> +int do_vcpu_stop(Monitor *mon, const QDict *qdict, QObject **ret_data);
> +int do_vcpu_start(Monitor *mon, const QDict *qdict, QObject **ret_data);
> +
>  void do_savevm(Monitor *mon, const QDict *qdict);
>  int load_vmstate(const char *name);
>  void do_delvm(Monitor *mon, const QDict *qdict);
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.

  parent reply	other threads:[~2010-11-23  7:29 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-22 23:00 [PATCH] qemu-kvm: introduce cpu_start/cpu_stop commands Anthony Liguori
2010-11-22 23:00 ` [Qemu-devel] " Anthony Liguori
2010-11-22 23:03 ` Anthony Liguori
2010-11-22 23:03   ` [Qemu-devel] " Anthony Liguori
2010-11-22 23:04 ` Chris Wright
2010-11-22 23:04   ` [Qemu-devel] " Chris Wright
2010-11-22 23:44   ` Anthony Liguori
2010-11-22 23:44     ` [Qemu-devel] " Anthony Liguori
2010-11-22 23:56     ` Chris Wright
2010-11-22 23:56       ` [Qemu-devel] " Chris Wright
2010-11-23  0:24       ` Anthony Liguori
2010-11-23  0:24         ` [Qemu-devel] " Anthony Liguori
2010-11-23  6:35   ` Avi Kivity
2010-11-23  6:41 ` [Qemu-devel] " Avi Kivity
2010-11-23  6:41   ` Avi Kivity
2010-11-23  8:16   ` Dor Laor
2010-11-23  8:16     ` Dor Laor
2010-11-23 13:57     ` Anthony Liguori
2010-11-23 13:57       ` Anthony Liguori
2010-11-23 13:51   ` Anthony Liguori
2010-11-23 13:51     ` Anthony Liguori
2010-11-23 14:00     ` Avi Kivity
2010-11-23 14:00       ` Avi Kivity
2010-11-23 14:24       ` Anthony Liguori
2010-11-23 14:24         ` Anthony Liguori
2010-11-23 14:35         ` Avi Kivity
2010-11-23 14:35           ` Avi Kivity
2010-11-23  7:29 ` Gleb Natapov [this message]
2010-11-23  7:29   ` [Qemu-devel] " Gleb Natapov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101123072939.GS7948@redhat.com \
    --to=gleb@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=chrisw@sous-sol.org \
    --cc=kvm@vger.kernel.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.