Re: [Qemu-devel] [PATCH] mainloop.c: Keep unlocking BQL during busy-wait spin-out

[Peter Crosthwaite]

Hi Peter,

On Fri, Apr 19, 2013 at 5:01 PM, Peter Lieven <address@hidden> wrote:
> On 18.04.2013 16:35, Peter Crosthwaite wrote:
>>
>> Hi Peter,
>>
>> On Fri, Apr 19, 2013 at 12:22 AM, Peter Lieven <address@hidden>
>> wrote:
>>>
>>> On 15.04.2013 15:08, Anthony Liguori wrote:
>>>>
>>>> Peter Crosthwaite <address@hidden> writes:
>>>>
>>>>> Modify Anthony's starvation detection logic to keep the BQL unlocked
>>>>> until the starvation condition goes away. Otherwise the counter has to
>>>>> count up to 1000 for each needed iteration until the busy-wait is
>>>>> lifted.
>>>>>
>>>>> Reset the counter back to zero once glib_pollfds_fill returns with a
>>>>> non-zero timout, (indicating a return to normality). The 1000 iteration
>>>>> wait now only happens once on the transition from normal operation to
>>>>> busy-wait starvation.
>>>>>
>>>>> Anthony's original patch fixed the serial paste bug, but this patch is
>>>>> also needed to restore performance.
>>>>>
>>>>> Signed-off-by: Peter Crosthwaite <address@hidden>
>>>>
>>>> I'm going through patches for 1.5 candidates.
>>>>
>>>> I believe the paste performance issue has been resolved now and this
>>>> patch is no longer needed.  I can't find a definitive statement on the
>>>> list for that though.
>>>
>>>
>>> I am also hitting a problem that occured first after Anthonys original
>>> patch.
>>> In my testing environment I had 3 vServers that indepently of the load
>>> became
>>> heavily unresponsive after reporting "main-loop: WARNING: I/O thread spun
>>> for 1000 iterations".
>>> Even QMP is not responsing from time to time. But I am not using serial.
>>>  From the load statistics
>>> it seems that the vServers is using one complete core busy waiting.
>>>
>>> I haven't seen this before this patch.
>>
>> Are you referring to my patch or Anthonys patch here? Does this patch
>> introduce a regression (or even a change in behaviour) for you?
>
> I only noticed that after Anthonys patch (or at that time) I got

Can you re-confirm with a quick test? bisect could work, or more
quickly a straight up git-revert of Anthonys patch ontop of whatever
your are currently building to see if it makes any difference. Also
try applying this patch. See if it clears up your issue as this patch
is designed to improve performance in the spin-out case.

> VMs that became unresponsive after the thread spun notification.
> Can you imagine that the fake timeout that is introduced by this
> patch can somehow itself can cause a problem?
>

No I cannot. Unless somehow there is a mutex issue with the lock,
which would probably indicate a bug elsewhere in the code base. The
affected unlock-relock is purely for optimization purposes and the
QEMU should be able to work without it at all.

Regards,
Peter

> Peter
>