From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34131) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8T4P-0003Wb-Pr for qemu-devel@nongnu.org; Thu, 02 Jun 2016 09:53:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8T4K-0000fE-Jf for qemu-devel@nongnu.org; Thu, 02 Jun 2016 09:53:56 -0400 Received: from mail-db3on0105.outbound.protection.outlook.com ([157.55.234.105]:36204 helo=emea01-db3-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8T4J-0000es-PH for qemu-devel@nongnu.org; Thu, 02 Jun 2016 09:53:52 -0400 Date: Thu, 2 Jun 2016 16:38:42 +0300 From: Pavel Borzenkov Message-ID: <20160602133842.GC34448@phobos> References: <1464772324-7354-1-git-send-email-den@openvz.org> <20160601100701.GD5356@noname.str.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20160601100701.GD5356@noname.str.redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/1] qcow2: avoid extra flushes in qcow2 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: "Denis V. Lunev" , qemu-block@nongnu.org, qemu-devel@nongnu.org, Max Reitz , jsnow@redhat.com On Wed, Jun 01, 2016 at 12:07:01PM +0200, Kevin Wolf wrote: > 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 > > CC: Pavel Borzenkov > > CC: Kevin Wolf > > CC: Max Reitz > > 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. Here are the results of the following testcase: sequential write of 8Gb file by 64Kb blocks, on unallocated qcow2 image, with fsync() after each 64 block. Lazy refcounts are disabled, so we have a dependent cache here. Results from SSD machine are as follows (every result is a 10 iterations average): w/o patches: ~420 blocks/sec with Den's patch: ~650 blocks/sec with Den's patch + qcow2_cache_flush_dependency() switched to qcow2_cache_flush_nosync(): ~720 blocks/sec > > Kevin