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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 02/07/2014 10:38, Kevin Wolf ha scritto:
> > On top of this, there _could_ be reasons for formats to implement
> > plug/unplug themselves.  They could coalesce metadata reads or
> > copy-on-write operations, for example.  This however is independent
> > from the default behavior, which IMO is "plugging should propagate
> > along bs->file" (and possibly bs->backing_hd too, but not now
> > because that opens more cans of worms).
>
> Yes, enabling it for bs->backing_hd was my alternative option for the
> case that we keep bs->file. What can of worms do you see there? From the
> reasoning above, I can't see why bs->backing_hd should be any different
> from bs->file.

The only one I thought of, is someone changing the backing file chain 
while the backing file is plugged.  IIRC bs->file doesn't change when 
you do bdrv_reopen.

Though changing the backing file chain probably would call 
bdrv_drain_all too.

> Another point I was thinking about (not for this series, but in a
> follow-up) is what to do with bdrv_aio_multiwrite(). We could either
> leave the two different interfaces there, just that multiwrite should
> probably call plug/unplug before it sends its requests (this would apply
> the improvements to virtio-blk without dataplane; could be done easily
> even now), or we try to implement request merging with the plug/unplug
> interface and convert the callers. Not sure how we would do the latter,
> though, because we don't have a list of requests any more, except in the
> lowest layer that actually queues.
>
> Or should we have a default plug/unplug implementation in block.c that
> queues any reads and writes, and on unplug merges requests, then plugs
> bs->file and sends the requests to the driver?

Longer term this is a good point.  Plug/unplug certainly provides many 
optimization opportunities, but as long as we expose the right API to 
the device model we need not care about it now.

In fact, there are plenty of other places where we can plug/unplug: 
mirroring, I/O throttling, image formats could also use aio to submit 
requests to a fragmented bs->file together and plug/unplug around the 
loop.  Benchmarking is needed of course to see if this actually improves 
performance (unlikely except in the last case) or at least reduces CPU 
usage.

Paolo