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 06:26 PM, Paolo Bonzini wrote:
> Il 09/10/2013 18:19, Alex Bligh ha scritto:
>>> Do you also agree that the equivalent workaround, before
>>>> Alex's patch, was MIN_REARM_TIMER_NS (and thus 250 microseconds)?
>> I don't think this was the case, as if it's a timer constantly
>> expiring we'd have seen select() exit as soon as it was entered
>> by the fd poked by the signal.
>
> The signal itself was clamped to be at least 250 microseconds...
>
>> That might be far more frequent.
>
> ... it's true though that it could have been less than 250 microseconds
> (more precisely, 250 microseconds minus the time from qemu_mod_timer_ns
> to select).
>
> Since the CPU usage with Hans's patch is 100% and used to be 50%, it was
> also more than 1 ns that Hans's patch is using.
>
>> I think the equivalent would be something like: if the 'zero'
>> timeout comes from the deadline calculation (and not
>> nonblocking=true) then release the lock anyway. I think
>> that would be a reasonable approach.
>>
>> I would however like to get to the bottom of what's causing
>> this as even pre my changes playing sound was apparently taking
>> 50% CPU, which is not good. I am completely packed until the
>> weekend but I propose producing a timer debug patch which
>> will instrument what is expiring constantly (unless the
>> problem with spice is obvious to someone).
>
> I think Hans already debugged it to the (supposedly) 33 Hz timer that
> spice audio uses.

Correction, I've been looking at timers in the spice audio path which could
potentially cause this, and this one stood out. The real problem could be
a completely different timer!

> If it turns out the bug is in the QEMU part of spice, I think it makes
> sense _not_ to include this patch at all.

As you said yourself before in a previous mail, at a minimum we should
ensure we always unlock the iolock when called in blocking mode, to give
other threads a chance to aquire it, so we need some form of my patch, or
some other patch which achieves the unlocking,

I welcome other proposals which have the same end result :)

Regards,

Hans