|
From: | Paolo Bonzini |
Subject: | Re: [Qemu-devel] [PATCH v2 1/3] block: block: introduce bdrv_io_plug() and bdrv_io_unplug() |
Date: | Wed, 02 Jul 2014 10:56:18 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 |
Il 02/07/2014 10:39, Ming Lei ha scritto:
Then start to read payload in original path, but no plug/unplug any more. Also another request may follows, and another plug&unplug comes too, which makes thing more complicated, so I suggest to enable plug&unplug only for raw driver now.
That's just a performance issue (and actually one that wasn't in 2.0 because qcow2 on dataplane wasn't supported there). In many cases the cache hit of the qcow2 metadata cache can be very high, and avoiding plug/unplug would prevent an easy performance bonus.
I don't especially like plug/unplug as an API (I think it's better to extend aio_multiwrite to include other kind of requests), but:
- either we have qualms on the correctness of it, and then we should live with the regressions
- or if the patches are not messy and reverting them is easy, we should go for it. This is what we did for dataplane in the first place, and we can keep doing it in the 2.1 dataplane code.
> If you could make it an assertion, that would also be great. (BTW, it's > probably best to add a nesting count to plug/unplug). I'd rather to add it in future if we really need that.
I think this is a matter of correctness as Kevin's review pointed out. Paolo
[Prev in Thread] | Current Thread | [Next in Thread] |