All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: qemu-trivial@nongnu.org, pbonzini@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH] main-loop: For tools, initialize timers as part of qemu_init_main_loop()
Date: Wed, 01 Feb 2012 16:10:27 -0600	[thread overview]
Message-ID: <4F29B853.4070805@codemonkey.ws> (raw)
In-Reply-To: <1327166033-17922-1-git-send-email-mdroth@linux.vnet.ibm.com>

On 01/21/2012 11:13 AM, Michael Roth wrote:
> In some cases initializing the alarm timers can lead to non-negligable
> overhead from programs that link against qemu-tool.o. At least,
> setting a max-resolution WinMM alarm timer via mm_start_timer() (the
> current default for Windows) can increase the "tick rate" on Windows
> OSs and affect frequency scaling, and in the case of tools that run
> in guest OSs such has qemu-ga, the impact can be fairly dramatic
> (+20%/20% user/sys time on a core 2 processor was observed from an idle
> Windows XP guest).
>
> This patch doesn't address the issue directly (not sure what a good
> solution would be for Windows, or what other situations it might be
> noticeable), but it at least limits the scope of the issue to programs
> that "opt-in" to using the main-loop.c functions by only enabling alarm
> timers when qemu_init_main_loop() is called, which is already required
> to make use of those facilities, so existing users shouldn't be
> affected.
>
> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>

Applied.  Thanks.

Regards,

Anthony Liguori
> ---
>   main-loop.c |    2 +-
>   main-loop.h |   12 ++++++++++++
>   qemu-tool.c |    3 ++-
>   vl.c        |    5 +++++
>   4 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/main-loop.c b/main-loop.c
> index 62d95b9..db23de0 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -199,7 +199,7 @@ static int qemu_signal_init(void)
>   }
>   #endif
>
> -int qemu_init_main_loop(void)
> +int main_loop_init(void)
>   {
>       int ret;
>
> diff --git a/main-loop.h b/main-loop.h
> index f971013..4987041 100644
> --- a/main-loop.h
> +++ b/main-loop.h
> @@ -41,10 +41,22 @@
>    * SIGUSR2, thread signals (SIGFPE, SIGILL, SIGSEGV, SIGBUS) and real-time
>    * signals if available.  Remember that Windows in practice does not have
>    * signals, though.
> + *
> + * In the case of QEMU tools, this will also start/initialize timers.
>    */
>   int qemu_init_main_loop(void);
>
>   /**
> + * main_loop_init: Initializes main loop
> + *
> + * Internal (but shared for compatibility reasons) initialization routine
> + * for the main loop. This should not be used by applications directly,
> + * use qemu_init_main_loop() instead.
> + *
> + */
> +int main_loop_init(void);
> +
> +/**
>    * main_loop_wait: Run one iteration of the main loop.
>    *
>    * If @nonblocking is true, poll for events, otherwise suspend until
> diff --git a/qemu-tool.c b/qemu-tool.c
> index 6b69668..183a583 100644
> --- a/qemu-tool.c
> +++ b/qemu-tool.c
> @@ -83,11 +83,12 @@ void qemu_clock_warp(QEMUClock *clock)
>   {
>   }
>
> -static void __attribute__((constructor)) init_main_loop(void)
> +int qemu_init_main_loop(void)
>   {
>       init_clocks();
>       init_timer_alarm();
>       qemu_clock_enable(vm_clock, false);
> +    return main_loop_init();
>   }
>
>   void slirp_select_fill(int *pnfds, fd_set *readfds,
> diff --git a/vl.c b/vl.c
> index ba55b35..74a47e6 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2136,6 +2136,11 @@ static void free_and_trace(gpointer mem)
>       free(mem);
>   }
>
> +int qemu_init_main_loop(void)
> +{
> +    return main_loop_init();
> +}
> +
>   int main(int argc, char **argv, char **envp)
>   {
>       const char *gdbstub_dev = NULL;



WARNING: multiple messages have this Message-ID (diff)
From: Anthony Liguori <anthony@codemonkey.ws>
To: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: qemu-trivial@nongnu.org, pbonzini@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] main-loop: For tools, initialize timers as part of qemu_init_main_loop()
Date: Wed, 01 Feb 2012 16:10:27 -0600	[thread overview]
Message-ID: <4F29B853.4070805@codemonkey.ws> (raw)
In-Reply-To: <1327166033-17922-1-git-send-email-mdroth@linux.vnet.ibm.com>

On 01/21/2012 11:13 AM, Michael Roth wrote:
> In some cases initializing the alarm timers can lead to non-negligable
> overhead from programs that link against qemu-tool.o. At least,
> setting a max-resolution WinMM alarm timer via mm_start_timer() (the
> current default for Windows) can increase the "tick rate" on Windows
> OSs and affect frequency scaling, and in the case of tools that run
> in guest OSs such has qemu-ga, the impact can be fairly dramatic
> (+20%/20% user/sys time on a core 2 processor was observed from an idle
> Windows XP guest).
>
> This patch doesn't address the issue directly (not sure what a good
> solution would be for Windows, or what other situations it might be
> noticeable), but it at least limits the scope of the issue to programs
> that "opt-in" to using the main-loop.c functions by only enabling alarm
> timers when qemu_init_main_loop() is called, which is already required
> to make use of those facilities, so existing users shouldn't be
> affected.
>
> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>

Applied.  Thanks.

Regards,

Anthony Liguori
> ---
>   main-loop.c |    2 +-
>   main-loop.h |   12 ++++++++++++
>   qemu-tool.c |    3 ++-
>   vl.c        |    5 +++++
>   4 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/main-loop.c b/main-loop.c
> index 62d95b9..db23de0 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -199,7 +199,7 @@ static int qemu_signal_init(void)
>   }
>   #endif
>
> -int qemu_init_main_loop(void)
> +int main_loop_init(void)
>   {
>       int ret;
>
> diff --git a/main-loop.h b/main-loop.h
> index f971013..4987041 100644
> --- a/main-loop.h
> +++ b/main-loop.h
> @@ -41,10 +41,22 @@
>    * SIGUSR2, thread signals (SIGFPE, SIGILL, SIGSEGV, SIGBUS) and real-time
>    * signals if available.  Remember that Windows in practice does not have
>    * signals, though.
> + *
> + * In the case of QEMU tools, this will also start/initialize timers.
>    */
>   int qemu_init_main_loop(void);
>
>   /**
> + * main_loop_init: Initializes main loop
> + *
> + * Internal (but shared for compatibility reasons) initialization routine
> + * for the main loop. This should not be used by applications directly,
> + * use qemu_init_main_loop() instead.
> + *
> + */
> +int main_loop_init(void);
> +
> +/**
>    * main_loop_wait: Run one iteration of the main loop.
>    *
>    * If @nonblocking is true, poll for events, otherwise suspend until
> diff --git a/qemu-tool.c b/qemu-tool.c
> index 6b69668..183a583 100644
> --- a/qemu-tool.c
> +++ b/qemu-tool.c
> @@ -83,11 +83,12 @@ void qemu_clock_warp(QEMUClock *clock)
>   {
>   }
>
> -static void __attribute__((constructor)) init_main_loop(void)
> +int qemu_init_main_loop(void)
>   {
>       init_clocks();
>       init_timer_alarm();
>       qemu_clock_enable(vm_clock, false);
> +    return main_loop_init();
>   }
>
>   void slirp_select_fill(int *pnfds, fd_set *readfds,
> diff --git a/vl.c b/vl.c
> index ba55b35..74a47e6 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2136,6 +2136,11 @@ static void free_and_trace(gpointer mem)
>       free(mem);
>   }
>
> +int qemu_init_main_loop(void)
> +{
> +    return main_loop_init();
> +}
> +
>   int main(int argc, char **argv, char **envp)
>   {
>       const char *gdbstub_dev = NULL;

  parent reply	other threads:[~2012-02-01 22:10 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-21  1:08 [Qemu-trivial] [PATCH] main-loop: Fix SetEvent() on uninitialized handle on win32 Michael Roth
2012-01-21  1:08 ` [Qemu-devel] " Michael Roth
2012-01-21  7:49 ` Paolo Bonzini
2012-01-21 17:13 ` [Qemu-trivial] [PATCH] main-loop: For tools, initialize timers as part of qemu_init_main_loop() Michael Roth
2012-01-21 17:13   ` [Qemu-devel] " Michael Roth
2012-01-21 20:39   ` [Qemu-trivial] " Jamie Lokier
2012-01-21 20:39     ` Jamie Lokier
2012-01-22 12:32     ` Paolo Bonzini
2012-01-23  0:12       ` Michael Roth
2012-01-23  7:49         ` Paolo Bonzini
2012-01-27  5:46   ` [Qemu-trivial] " Stefan Hajnoczi
2012-01-27  5:46     ` [Qemu-devel] " Stefan Hajnoczi
2012-01-27  8:09     ` Paolo Bonzini
2012-01-27  8:09       ` [Qemu-devel] " Paolo Bonzini
2012-01-27  5:46   ` Stefan Hajnoczi
2012-01-27  5:46     ` [Qemu-devel] " Stefan Hajnoczi
2012-02-01 22:10   ` Anthony Liguori [this message]
2012-02-01 22:10     ` [Qemu-devel] " Anthony Liguori
2012-01-27  5:41 ` [Qemu-trivial] [PATCH] main-loop: Fix SetEvent() on uninitialized handle on win32 Stefan Hajnoczi
2012-01-27  5:41   ` [Qemu-devel] " Stefan Hajnoczi
2012-01-27 14:52   ` [Qemu-trivial] [PATCH v2] " Michael Roth
2012-01-27 14:52     ` [Qemu-devel] " Michael Roth
2012-02-01 22:10 ` [Qemu-trivial] [Qemu-devel] [PATCH] " Anthony Liguori
2012-02-01 22:10   ` Anthony Liguori

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=4F29B853.4070805@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@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.