qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for 1.2] qemu-timer: properly arm alarm timer for timers set by device initialization
@ 2012-09-03 15:34 Paolo Bonzini
  2012-09-03 15:43 ` Jan Kiszka
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Paolo Bonzini @ 2012-09-03 15:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, aurelien

QEMU will hang when fed the following command-line

  qemu-system-mips -kernel vmlinux-2.6.32-5-4kc-malta -append "console=ttyS0" -nographic -net none

The -net none is important otherwise it seems some events are generated
causing the things to work. When it doesn't work, the guest hangs when
measuring the CPU frequency, after the following line:

  [    0.000000] NR_IRQS:256

Pressing a key on the serial port unblocks it, hinting that the problem
is due to the recent elimination of the 1 second timeout in the main
loop.

The problem is that because init_timer_alarm sets the timer's pending
flag to true, the alarm timer is never armed until after the first time
through the main loop.  Thus the bug started when QEMU started testing
the pending flag in qemu_mod_timer (commit 1828be3, more alarm timer
cleanup, 2010-03-10).

But actually, it isn't true at all that a timer is pending when the
alarm timer is created, and the real bug has been latent forever: the
fix is to remove the bogus setting of pending flag.

Reported-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-timer.c | 3 ---
 1 file modificato, 3 rimozioni(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index 5aea94e..c7a1551 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -759,11 +759,8 @@ int init_timer_alarm(void)
         goto fail;
     }
 
-    /* first event is at time 0 */
     atexit(quit_timers);
-    t->pending = true;
     alarm_timer = t;
-
     return 0;
 
 fail:
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH for 1.2] qemu-timer: properly arm alarm timer for timers set by device initialization
  2012-09-03 15:34 [Qemu-devel] [PATCH for 1.2] qemu-timer: properly arm alarm timer for timers set by device initialization Paolo Bonzini
@ 2012-09-03 15:43 ` Jan Kiszka
  2012-09-03 15:45 ` Aurelien Jarno
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Jan Kiszka @ 2012-09-03 15:43 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: aliguori, qemu-devel, aurelien

On 2012-09-03 17:34, Paolo Bonzini wrote:
> QEMU will hang when fed the following command-line
> 
>   qemu-system-mips -kernel vmlinux-2.6.32-5-4kc-malta -append "console=ttyS0" -nographic -net none
> 
> The -net none is important otherwise it seems some events are generated
> causing the things to work. When it doesn't work, the guest hangs when
> measuring the CPU frequency, after the following line:
> 
>   [    0.000000] NR_IRQS:256
> 
> Pressing a key on the serial port unblocks it, hinting that the problem
> is due to the recent elimination of the 1 second timeout in the main
> loop.
> 
> The problem is that because init_timer_alarm sets the timer's pending
> flag to true, the alarm timer is never armed until after the first time
> through the main loop.  Thus the bug started when QEMU started testing
> the pending flag in qemu_mod_timer (commit 1828be3, more alarm timer
> cleanup, 2010-03-10).
> 
> But actually, it isn't true at all that a timer is pending when the
> alarm timer is created, and the real bug has been latent forever: the
> fix is to remove the bogus setting of pending flag.
> 
> Reported-by: Aurelien Jarno <aurelien@aurel32.net>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-timer.c | 3 ---
>  1 file modificato, 3 rimozioni(-)
> 
> diff --git a/qemu-timer.c b/qemu-timer.c
> index 5aea94e..c7a1551 100644
> --- a/qemu-timer.c
> +++ b/qemu-timer.c
> @@ -759,11 +759,8 @@ int init_timer_alarm(void)
>          goto fail;
>      }
>  
> -    /* first event is at time 0 */
>      atexit(quit_timers);
> -    t->pending = true;
>      alarm_timer = t;
> -
>      return 0;
>  
>  fail:
> 

Funnily, I just create the same problem with my "run timer handlers in
signal handler" patch (*). Same solution there.

Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com>

Jan


(*) I will go for select-based timers once I have time for the necessary
refactorings.

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH for 1.2] qemu-timer: properly arm alarm timer for timers set by device initialization
  2012-09-03 15:34 [Qemu-devel] [PATCH for 1.2] qemu-timer: properly arm alarm timer for timers set by device initialization Paolo Bonzini
  2012-09-03 15:43 ` Jan Kiszka
@ 2012-09-03 15:45 ` Aurelien Jarno
  2012-09-04  7:06 ` Michael Tokarev
  2012-09-04 10:32 ` Aurelien Jarno
  3 siblings, 0 replies; 5+ messages in thread
From: Aurelien Jarno @ 2012-09-03 15:45 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: aliguori, qemu-devel

On Mon, Sep 03, 2012 at 05:34:32PM +0200, Paolo Bonzini wrote:
> QEMU will hang when fed the following command-line
> 
>   qemu-system-mips -kernel vmlinux-2.6.32-5-4kc-malta -append "console=ttyS0" -nographic -net none
> 
> The -net none is important otherwise it seems some events are generated
> causing the things to work. When it doesn't work, the guest hangs when
> measuring the CPU frequency, after the following line:
> 
>   [    0.000000] NR_IRQS:256
> 
> Pressing a key on the serial port unblocks it, hinting that the problem
> is due to the recent elimination of the 1 second timeout in the main
> loop.
> 
> The problem is that because init_timer_alarm sets the timer's pending
> flag to true, the alarm timer is never armed until after the first time
> through the main loop.  Thus the bug started when QEMU started testing
> the pending flag in qemu_mod_timer (commit 1828be3, more alarm timer
> cleanup, 2010-03-10).
> 
> But actually, it isn't true at all that a timer is pending when the
> alarm timer is created, and the real bug has been latent forever: the
> fix is to remove the bogus setting of pending flag.
> 
> Reported-by: Aurelien Jarno <aurelien@aurel32.net>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-timer.c | 3 ---
>  1 file modificato, 3 rimozioni(-)
> 
> diff --git a/qemu-timer.c b/qemu-timer.c
> index 5aea94e..c7a1551 100644
> --- a/qemu-timer.c
> +++ b/qemu-timer.c
> @@ -759,11 +759,8 @@ int init_timer_alarm(void)
>          goto fail;
>      }
>  
> -    /* first event is at time 0 */
>      atexit(quit_timers);
> -    t->pending = true;
>      alarm_timer = t;
> -
>      return 0;
>  
>  fail:
>

Thanks a lot that was quite fast!

Tested-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH for 1.2] qemu-timer: properly arm alarm timer for timers set by device initialization
  2012-09-03 15:34 [Qemu-devel] [PATCH for 1.2] qemu-timer: properly arm alarm timer for timers set by device initialization Paolo Bonzini
  2012-09-03 15:43 ` Jan Kiszka
  2012-09-03 15:45 ` Aurelien Jarno
@ 2012-09-04  7:06 ` Michael Tokarev
  2012-09-04 10:32 ` Aurelien Jarno
  3 siblings, 0 replies; 5+ messages in thread
From: Michael Tokarev @ 2012-09-04  7:06 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: aliguori, qemu-devel, aurelien, Qemu-stable@nongnu.org

On 03.09.2012 19:34, Paolo Bonzini wrote:
> QEMU will hang when fed the following command-line
> 
>   qemu-system-mips -kernel vmlinux-2.6.32-5-4kc-malta -append "console=ttyS0" -nographic -net none
> 
> The -net none is important otherwise it seems some events are generated
> causing the things to work. When it doesn't work, the guest hangs when
> measuring the CPU frequency, after the following line:
> 
>   [    0.000000] NR_IRQS:256
> 
> Pressing a key on the serial port unblocks it, hinting that the problem
> is due to the recent elimination of the 1 second timeout in the main
> loop.
> 
> The problem is that because init_timer_alarm sets the timer's pending
> flag to true, the alarm timer is never armed until after the first time
> through the main loop.  Thus the bug started when QEMU started testing
> the pending flag in qemu_mod_timer (commit 1828be3, more alarm timer
> cleanup, 2010-03-10).
> 
> But actually, it isn't true at all that a timer is pending when the
> alarm timer is created, and the real bug has been latent forever: the
> fix is to remove the bogus setting of pending flag.
> 
> Reported-by: Aurelien Jarno <aurelien@aurel32.net>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-timer.c | 3 ---
>  1 file modificato, 3 rimozioni(-)
> 
> diff --git a/qemu-timer.c b/qemu-timer.c
> index 5aea94e..c7a1551 100644
> --- a/qemu-timer.c
> +++ b/qemu-timer.c
> @@ -759,11 +759,8 @@ int init_timer_alarm(void)
>          goto fail;
>      }
>  
> -    /* first event is at time 0 */
>      atexit(quit_timers);
> -    t->pending = true;
>      alarm_timer = t;
> -
>      return 0;
>  
>  fail:

This also fixes the pty-char hang I reported yesterday
in thread "apparently missing yet another notify_event()".

Tested-By: Michael Tokarev <mjt@tls.msk.ru>

This should go to 1.1-stable too, as this problem exists
there, with both -net none and -serial pty being reproducers.
Cc'ing -stable.

Thanks!

/mjt

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH for 1.2] qemu-timer: properly arm alarm timer for timers set by device initialization
  2012-09-03 15:34 [Qemu-devel] [PATCH for 1.2] qemu-timer: properly arm alarm timer for timers set by device initialization Paolo Bonzini
                   ` (2 preceding siblings ...)
  2012-09-04  7:06 ` Michael Tokarev
@ 2012-09-04 10:32 ` Aurelien Jarno
  3 siblings, 0 replies; 5+ messages in thread
From: Aurelien Jarno @ 2012-09-04 10:32 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: aliguori, qemu-devel

On Mon, Sep 03, 2012 at 05:34:32PM +0200, Paolo Bonzini wrote:
> QEMU will hang when fed the following command-line
> 
>   qemu-system-mips -kernel vmlinux-2.6.32-5-4kc-malta -append "console=ttyS0" -nographic -net none
> 
> The -net none is important otherwise it seems some events are generated
> causing the things to work. When it doesn't work, the guest hangs when
> measuring the CPU frequency, after the following line:
> 
>   [    0.000000] NR_IRQS:256
> 
> Pressing a key on the serial port unblocks it, hinting that the problem
> is due to the recent elimination of the 1 second timeout in the main
> loop.
> 
> The problem is that because init_timer_alarm sets the timer's pending
> flag to true, the alarm timer is never armed until after the first time
> through the main loop.  Thus the bug started when QEMU started testing
> the pending flag in qemu_mod_timer (commit 1828be3, more alarm timer
> cleanup, 2010-03-10).
> 
> But actually, it isn't true at all that a timer is pending when the
> alarm timer is created, and the real bug has been latent forever: the
> fix is to remove the bogus setting of pending flag.
> 
> Reported-by: Aurelien Jarno <aurelien@aurel32.net>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-timer.c | 3 ---
>  1 file modificato, 3 rimozioni(-)
> 
> diff --git a/qemu-timer.c b/qemu-timer.c
> index 5aea94e..c7a1551 100644
> --- a/qemu-timer.c
> +++ b/qemu-timer.c
> @@ -759,11 +759,8 @@ int init_timer_alarm(void)
>          goto fail;
>      }
>  
> -    /* first event is at time 0 */
>      atexit(quit_timers);
> -    t->pending = true;
>      alarm_timer = t;
> -
>      return 0;
>  
>  fail:

Thanks, I have applied it.


-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-09-04 10:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-03 15:34 [Qemu-devel] [PATCH for 1.2] qemu-timer: properly arm alarm timer for timers set by device initialization Paolo Bonzini
2012-09-03 15:43 ` Jan Kiszka
2012-09-03 15:45 ` Aurelien Jarno
2012-09-04  7:06 ` Michael Tokarev
2012-09-04 10:32 ` Aurelien Jarno

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).