Re: [Qemu-devel] [PATCH] posix-aio-compat: Fix idle_threads counter

[Stefan Hajnoczi <stefanha@gmail.com>]

On Tue, May 3, 2011 at 5:53 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 03.05.2011 17:56, schrieb Stefan Hajnoczi:
>> On Tue, May 3, 2011 at 2:26 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>>> A thread should only be counted as idle when it really is waiting for new
>>> requests. Without this patch, sometimes too few threads are started as busy
>>> threads are counted as idle.
>>>
>>> Not sure if it makes a difference in practice outside some artificial
>>> qemu-io/qemu-img tests, but I think the change makes sense in any case.
>>>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>>  posix-aio-compat.c |    6 ++----
>>>  1 files changed, 2 insertions(+), 4 deletions(-)
>>
>> I think the critical change here is that idle_threads is not being
>> incremented by spawn_thread().  This means that we will keep spawning
>> threads as new requests come in and until the first thread goes idle.
>>
>> Previously you could imagine a scenario where we spawn a thread but
>> don't schedule it yet.  Then we immediately submit more I/O and since
>> idle_threads was incremented we don't spawn additional threads to
>> handle the requests.
>>
>> Are these the cases you were thinking about?
>
> Yes, this is the case that I noticed.
>
> However, I'm not sure if this is really the critical change. In this
> case, it would take a bit longer until you get your full 64 threads, but
> you should get there eventually and then it shouldn't impact performance
> any more.
>
> However, what I saw in my test case (qemu-img always running 16
> sequential read requests in parallel) was that I only got 16 threads.
> This sounds logical, but in fact you seem to need always one thread more
> for good performance (I don't fully understand this yet). And with this
> patch, you actually get 17 threads. The difference was like 8s vs. 22s
> for the same requests, and using more than 17 threads doesn't further
> improve it.

Wow, 8s vs 22s is a big difference.  Did you run any guest benchmarks?

Stefan