Re: [Qemu-devel] [PATCH v2 1/3] block: block: introduce bdrv_io_plug() and bdrv_io_unplug()

[Kevin Wolf]

Am 02.07.2014 um 10:56 hat Paolo Bonzini geschrieben:
> 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.

Fully agree. This series is small enough and obviously fixes a
dataplane problem, so at least for 2.1 we should go for it.

My thoughts in the other mail were more about where to go in the long
term. We need to have a decision about what API we commit to - something
multiwrite-like or something plug/unplug-like - before we want to start
converting everything to that interface.

This is why I think we should be thinking about how to implement certain
optimisations (like the request merging with plug/unplug, as I mentioned;
or mixing read and writes in one batch with multiwrite) in both models.
Only when we have a reasonbly good idea of what the result would look
like in either case we can make an informed decision.

Kevin