All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: qemu-devel@nongnu.org, kvm <kvm@vger.kernel.org>,
	Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: MIPS, io-thread, icount and wfi
Date: Tue, 18 Jan 2011 11:00:57 +0100	[thread overview]
Message-ID: <4D3564D9.6020104@siemens.com> (raw)
In-Reply-To: <20110118001950.GA11802@laped.lan>

On 2011-01-18 01:19, Edgar E. Iglesias wrote:
> On Mon, Jan 17, 2011 at 11:03:08AM +0100, Edgar E. Iglesias wrote:
>> Hi,
>>
>> I'm running an io-thread enabled qemu-system-mipsel with icount.
>> When the guest (linux) goes to sleep through the wait insn (waiting
>> to be woken up by future timer interrupts), the thing deadlocks.
>>
>> IIUC, this is because vm timers are driven by icount, but the CPU is
>> halted so icount makes no progress and time stands still.
>>
>> I've locally disabled vcpu halting when icount is enabled, that
>> works around my problem but of course makes qemu consume 100% host cpu.
>>
>> I don't know why I only see this problem with io-thread builds?
>> Could be related timing and luck.
>>
>> Would be interesting to know if someone has any info on how this was
>> intended to work (if it was)? And if there are ideas for better
>> workarounds or fixes that don't disable vcpu halting entirely.
> 
> Hi,
> 
> I've found the problem. For some reason io-thread builds use a
> static timeout for wait loops. The entire chunk of code that
> makes sure qemu_icount makes forward progress when the CPU's
> are idle has been ifdef'ed away...
> 
> This fixes the problem for me, hopefully without affecting
> io-thread runs without icount.
> 
> commit 0f4f3a919952500b487b438c5520f07a1c6be35b
> Author: Edgar E. Iglesias <edgar@axis.com>
> Date:   Tue Jan 18 01:01:57 2011 +0100
> 
>     qemu-timer: Fix timeout calc for io-thread with icount
>     
>     Make sure we always make forward progress with qemu_icount to
>     avoid deadlocks. For io-thread, use the static 1000 timeout
>     only if icount is disabled.
>     
>     Signed-off-by: Edgar E. Iglesias <edgar@axis.com>
> 
> diff --git a/qemu-timer.c b/qemu-timer.c
> index 95814af..db1ec49 100644
> --- a/qemu-timer.c
> +++ b/qemu-timer.c
> @@ -110,7 +110,6 @@ static int64_t cpu_get_clock(void)
>      }
>  }
>  
> -#ifndef CONFIG_IOTHREAD
>  static int64_t qemu_icount_delta(void)
>  {
>      if (!use_icount) {
> @@ -124,7 +123,6 @@ static int64_t qemu_icount_delta(void)
>          return cpu_get_icount() - cpu_get_clock();
>      }
>  }
> -#endif
>  
>  /* enable cpu_get_ticks() */
>  void cpu_enable_ticks(void)
> @@ -1077,9 +1075,17 @@ void quit_timers(void)
>  
>  int qemu_calculate_timeout(void)
>  {
> -#ifndef CONFIG_IOTHREAD
>      int timeout;
>  
> +#ifdef CONFIG_IOTHREAD
> +    /* When using icount, making forward progress with qemu_icount when the
> +       guest CPU is idle is critical. We only use the static io-thread timeout
> +       for non icount runs.  */
> +    if (!use_icount) {
> +        return 1000;
> +    }
> +#endif
> +
>      if (!vm_running)
>          timeout = 5000;
>      else {
> @@ -1110,8 +1116,5 @@ int qemu_calculate_timeout(void)
>      }
>  
>      return timeout;
> -#else /* CONFIG_IOTHREAD */
> -    return 1000;
> -#endif
>  }
>  
> 
> 

This logic and timeout values were imported on iothread merge. And I bet
at least the timeout value of 1s (vs. 5s) can still be found in
qemu-kvm. Maybe someone over there can remember the rationales behind
choosing this value.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

WARNING: multiple messages have this Message-ID (diff)
From: Jan Kiszka <jan.kiszka@siemens.com>
To: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>,
	qemu-devel@nongnu.org, kvm <kvm@vger.kernel.org>
Subject: [Qemu-devel] Re: MIPS, io-thread, icount and wfi
Date: Tue, 18 Jan 2011 11:00:57 +0100	[thread overview]
Message-ID: <4D3564D9.6020104@siemens.com> (raw)
In-Reply-To: <20110118001950.GA11802@laped.lan>

On 2011-01-18 01:19, Edgar E. Iglesias wrote:
> On Mon, Jan 17, 2011 at 11:03:08AM +0100, Edgar E. Iglesias wrote:
>> Hi,
>>
>> I'm running an io-thread enabled qemu-system-mipsel with icount.
>> When the guest (linux) goes to sleep through the wait insn (waiting
>> to be woken up by future timer interrupts), the thing deadlocks.
>>
>> IIUC, this is because vm timers are driven by icount, but the CPU is
>> halted so icount makes no progress and time stands still.
>>
>> I've locally disabled vcpu halting when icount is enabled, that
>> works around my problem but of course makes qemu consume 100% host cpu.
>>
>> I don't know why I only see this problem with io-thread builds?
>> Could be related timing and luck.
>>
>> Would be interesting to know if someone has any info on how this was
>> intended to work (if it was)? And if there are ideas for better
>> workarounds or fixes that don't disable vcpu halting entirely.
> 
> Hi,
> 
> I've found the problem. For some reason io-thread builds use a
> static timeout for wait loops. The entire chunk of code that
> makes sure qemu_icount makes forward progress when the CPU's
> are idle has been ifdef'ed away...
> 
> This fixes the problem for me, hopefully without affecting
> io-thread runs without icount.
> 
> commit 0f4f3a919952500b487b438c5520f07a1c6be35b
> Author: Edgar E. Iglesias <edgar@axis.com>
> Date:   Tue Jan 18 01:01:57 2011 +0100
> 
>     qemu-timer: Fix timeout calc for io-thread with icount
>     
>     Make sure we always make forward progress with qemu_icount to
>     avoid deadlocks. For io-thread, use the static 1000 timeout
>     only if icount is disabled.
>     
>     Signed-off-by: Edgar E. Iglesias <edgar@axis.com>
> 
> diff --git a/qemu-timer.c b/qemu-timer.c
> index 95814af..db1ec49 100644
> --- a/qemu-timer.c
> +++ b/qemu-timer.c
> @@ -110,7 +110,6 @@ static int64_t cpu_get_clock(void)
>      }
>  }
>  
> -#ifndef CONFIG_IOTHREAD
>  static int64_t qemu_icount_delta(void)
>  {
>      if (!use_icount) {
> @@ -124,7 +123,6 @@ static int64_t qemu_icount_delta(void)
>          return cpu_get_icount() - cpu_get_clock();
>      }
>  }
> -#endif
>  
>  /* enable cpu_get_ticks() */
>  void cpu_enable_ticks(void)
> @@ -1077,9 +1075,17 @@ void quit_timers(void)
>  
>  int qemu_calculate_timeout(void)
>  {
> -#ifndef CONFIG_IOTHREAD
>      int timeout;
>  
> +#ifdef CONFIG_IOTHREAD
> +    /* When using icount, making forward progress with qemu_icount when the
> +       guest CPU is idle is critical. We only use the static io-thread timeout
> +       for non icount runs.  */
> +    if (!use_icount) {
> +        return 1000;
> +    }
> +#endif
> +
>      if (!vm_running)
>          timeout = 5000;
>      else {
> @@ -1110,8 +1116,5 @@ int qemu_calculate_timeout(void)
>      }
>  
>      return timeout;
> -#else /* CONFIG_IOTHREAD */
> -    return 1000;
> -#endif
>  }
>  
> 
> 

This logic and timeout values were imported on iothread merge. And I bet
at least the timeout value of 1s (vs. 5s) can still be found in
qemu-kvm. Maybe someone over there can remember the rationales behind
choosing this value.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

  reply	other threads:[~2011-01-18 10:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-17 10:03 [Qemu-devel] MIPS, io-thread, icount and wfi Edgar E. Iglesias
2011-01-18  0:19 ` [Qemu-devel] " Edgar E. Iglesias
2011-01-18 10:00   ` Jan Kiszka [this message]
2011-01-18 10:00     ` Jan Kiszka
2011-01-18 10:17     ` Jan Kiszka
2011-01-18 10:17       ` [Qemu-devel] " Jan Kiszka
2011-01-19 17:02     ` Marcelo Tosatti
2011-01-19 17:02       ` [Qemu-devel] " Marcelo Tosatti
2011-01-19 19:02       ` Edgar E. Iglesias
2011-01-19 19:02         ` [Qemu-devel] " Edgar E. Iglesias
2011-01-23  4:09         ` Edgar E. Iglesias
2011-01-23  4:09           ` [Qemu-devel] " Edgar E. Iglesias

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=4D3564D9.6020104@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=edgar.iglesias@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --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.