Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 10/09/2013 08:36 PM, Alex Bligh wrote:
>
> On 9 Oct 2013, at 19:28, Alex Bligh wrote:
>
>>>
>>> static void audio_reset_timer (AudioState *s)
>>> {
>>>    if (audio_is_timer_needed ()) {
>>>        timer_mod (s->ts, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 1);
>>>    }
>>>    else {
>>>        timer_del (s->ts);
>>>    }
>>> }
>>>
>>> static void audio_timer (void *opaque)
>>> {
>>>    audio_run ("timer");
>>>    audio_reset_timer (opaque);
>>> }
>>>
>>> Note how that is using a timer which expires every freaking nano second,
>>> I think it is very likely that is the culprit.
>>
>> Indeed. I am hoping that it is not my automated perl conversion code that
>> did that, because if it is, it may have broken other stuff :-/
>>
>> Thanks for finding this - let me see whether the bug existed before
>> my automated changes commit.
>
> I think this was broken prior to my changes. Here's audio/audio.c before
> my changes:
>
> static void audio_reset_timer (AudioState *s)
> {
>      if (audio_is_timer_needed ()) {
>          qemu_mod_timer (s->ts, qemu_get_clock_ns (vm_clock) + 1);
>      }
>      else {
>          qemu_del_timer (s->ts);
>      }
> }
>
>
> Now qemu_get_clock_ns can only return something in nanoseconds, so it's
> adding 1 nanosecond. This is thus either broken because:
>    1. ts->scale is in nanoseconds, in which case that timer expires in
>       one nano-second's time; or
>    2. ts->scale is not in nanoseconds, in which case nanosecond VM clock
>       is going to be a huge time in the future, and it's never going
>       to expire.
>
> I wonder whether it's meant to be 1 millisecond or 1 microsecond?

Maybe once it was 1 ms, this code just exists to keep the buffers
of a soundcard filled / emptied in time. 100 times / second is more
then plenty for that, so that is what I'm going to use in the patch I'm
about to submit.

Regards,

Hans

p.s.

I still think we should unlock the io-thread in the main_loop_wait when
called with nonblocking == 0, even if there are expired timers. Any
suggestions on how to best do that ?