Re: [Qemu-devel] [PATCH 1/1] qcow2: avoid extra flushes in qcow2

[Kevin Wolf]

Am 01.06.2016 um 11:12 hat Denis V. Lunev geschrieben:
> qcow2_cache_flush() calls bdrv_flush() unconditionally after writing
> cache entries of a particular cache. This can lead to as many as
> 2 additional fdatasyncs inside bdrv_flush.
> 
> We can simply skip all fdatasync calls inside qcow2_co_flush_to_os
> as bdrv_flush for sure will do the job.

This looked wrong at first because flushes are needed to keep the right
order of writes to the different caches. However, I see that you keep
the flush in qcow2_cache_flush_dependency(), so in the code this is
actually fine.

Can you make that more explicit in the commit message?

> This seriously affects the
> performance of database operations inside the guest.
> 
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Pavel Borzenkov <address@hidden>
> CC: Kevin Wolf <address@hidden>
> CC: Max Reitz <address@hidden>

Do you have performance numbers for master and with your patch applied?
(No performance related patch should come without numbers in its commit
message!)

What I find interesting is that this seems to help even though
duplicated flushes should actually be really cheap because there is no
new data that could be flushed in the second request. Makes me wonder if
guests send duplicated flushes, too, and whether we should optimise
that.

Maybe it would also be interesting to measure how things perform if we
removed the flush from qcow2_cache_flush_dependency(). This would be
incorrect code (corruption possible after host crash), but I'd like to
know how much performance we actually lose here. This is performance
that could potentially be gained by using a journal.

Kevin