On Tue, May 3, 2011 at 5:53 PM, Kevin Wolf <kw...@redhat.com> wrote: > Am 03.05.2011 17:56, schrieb Stefan Hajnoczi: >> On Tue, May 3, 2011 at 2:26 PM, Kevin Wolf <kw...@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 <kw...@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
