Re: MIPS, io-thread, icount and wfi

Re: MIPS, io-thread, icount and wfi

[Jan Kiszka]

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

Re: MIPS, io-thread, icount and wfi

[Jan Kiszka]

On 2011-01-18 11:00, Jan Kiszka wrote:
> 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.

Correction: qemu-kvm does _not_ use a fixed timeout value for the
iothread nor the removed code path (as CONFIG_IOTHREAD is off in
qemu-kvm, that tree does not even build when you enable it).

Still, the reason for once introducing this difference to qemu should be
reflected.

Jan

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

Re: MIPS, io-thread, icount and wfi

[Marcelo Tosatti]

On Tue, Jan 18, 2011 at 11:00:57AM +0100, Jan Kiszka wrote:
> 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

This timeout is for the main select() call. So there is not a lot
of reasoning, how long to wait when there's no activity on the file
descriptors.

Re: MIPS, io-thread, icount and wfi

[Edgar E. Iglesias]

On Wed, Jan 19, 2011 at 03:02:26PM -0200, Marcelo Tosatti wrote:
> On Tue, Jan 18, 2011 at 11:00:57AM +0100, Jan Kiszka wrote:
> > 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
> 
> This timeout is for the main select() call. So there is not a lot
> of reasoning, how long to wait when there's no activity on the file
> descriptors.

OK, I suspected something like that. Thanks both of you for the info.
I'll give people a couple of days to complain at the patch, if noone
does I'll apply it.

Cheers

Re: MIPS, io-thread, icount and wfi

[Edgar E. Iglesias]

On Wed, Jan 19, 2011 at 08:02:28PM +0100, Edgar E. Iglesias wrote:
> On Wed, Jan 19, 2011 at 03:02:26PM -0200, Marcelo Tosatti wrote:
> > On Tue, Jan 18, 2011 at 11:00:57AM +0100, Jan Kiszka wrote:
> > > 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
> > 
> > This timeout is for the main select() call. So there is not a lot
> > of reasoning, how long to wait when there's no activity on the file
> > descriptors.
> 
> OK, I suspected something like that. Thanks both of you for the info.
> I'll give people a couple of days to complain at the patch, if noone
> does I'll apply it.

Silence - so I've applied this one, thanks.

Cheers