From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:57115)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <hdegoede@redhat.com>) id 1VTcqy-0001Cw-1y
	for qemu-devel@nongnu.org; Tue, 08 Oct 2013 15:22:02 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <hdegoede@redhat.com>) id 1VTcqs-0000OD-2v
	for qemu-devel@nongnu.org; Tue, 08 Oct 2013 15:21:56 -0400
Received: from mx1.redhat.com ([209.132.183.28]:3566)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <hdegoede@redhat.com>) id 1VTcqr-0000O3-QU
	for qemu-devel@nongnu.org; Tue, 08 Oct 2013 15:21:49 -0400
Message-ID: <52545B44.70005@redhat.com>
Date: Tue, 08 Oct 2013 21:21:40 +0200
From: Hans de Goede <hdegoede@redhat.com>
MIME-Version: 1.0
References: <1381259403-7386-1-git-send-email-hdegoede@redhat.com>
	<52545950.5070403@redhat.com>
In-Reply-To: <52545950.5070403@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads
 when main_loop_tlg has pending events
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, Alex Bligh <alex@alex.org.uk>

Hi,

On 10/08/2013 09:13 PM, Paolo Bonzini wrote:
> Il 08/10/2013 21:10, Hans de Goede ha scritto:
>> @@ -480,6 +480,11 @@ int main_loop_wait(int nonblocking)
>>                                         timerlistgroup_deadline_ns(
>>                                             &main_loop_tlg));
>>
>> +    /* When not non-blocking always allow io-threads to acquire the lock */
>> +    if (timeout != 0 && timeout_ns == 0) {
>> +        timeout_ns = 1;
>> +    }
>> +
>
> This _is_ an I/O thread, so I guess this should be changed to "other
> threads".

Ok.

> Also, perhaps timeout_ns can be changed to a higher value
> such as 250 us that were used up to the offending commit?

Wasn't it 1 ms until the offending commit (note 250 us does
sound better to me). More over I wonder how useful is this, if threads
are waiting for the lock at his point, they should all get it and do
work (and then release it) before this thread will be able to re-aquire it.

The only case which I can see where going to sleep will help is when a
thread takes the lock, does some thing, releases it, then does something
else quite quickly (so within 250 us), and then tries to re-aquire the lock
to do more work.

Note either solution (1 ns versus 250 us) is fine with me, I'm by no means
the expert on this, just let me know which one you think is better and I'll
do a v2.

Regards,

Hans