From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33796) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bLILq-0008AN-Kv for qemu-devel@nongnu.org; Thu, 07 Jul 2016 19:04:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bLILp-0007OM-D4 for qemu-devel@nongnu.org; Thu, 07 Jul 2016 19:04:58 -0400 References: <1467643124-29778-1-git-send-email-den@openvz.org> <1467643124-29778-5-git-send-email-den@openvz.org> From: Eric Blake Message-ID: <577EE010.7090106@redhat.com> Date: Thu, 7 Jul 2016 17:04:48 -0600 MIME-Version: 1.0 In-Reply-To: <1467643124-29778-5-git-send-email-den@openvz.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="5pav4uAF45oTUnQSoMokAIm9WHh6Wljn6" Subject: Re: [Qemu-devel] [PATCH v5 4/4] block: ignore flush requests when storage is clean List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: Kevin Wolf , Fam Zheng , Evgeny Yakovlev , Max Reitz , Stefan Hajnoczi , John Snow This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --5pav4uAF45oTUnQSoMokAIm9WHh6Wljn6 From: Eric Blake To: "Denis V. Lunev" , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: Kevin Wolf , Fam Zheng , Evgeny Yakovlev , Max Reitz , Stefan Hajnoczi , John Snow Message-ID: <577EE010.7090106@redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 4/4] block: ignore flush requests when storage is clean References: <1467643124-29778-1-git-send-email-den@openvz.org> <1467643124-29778-5-git-send-email-den@openvz.org> In-Reply-To: <1467643124-29778-5-git-send-email-den@openvz.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/04/2016 08:38 AM, Denis V. Lunev wrote: > From: Evgeny Yakovlev >=20 > Some guests (win2008 server for example) do a lot of unnecessary > flushing when underlying media has not changed. This adds additional > overhead on host when calling fsync/fdatasync. >=20 > This change introduces a write generation scheme in BlockDriverState. > Current write generation is checked against last flushed generation to > avoid unnessesary flushes. >=20 > The problem with excessive flushing was found by a performance test > which does parallel directory tree creation (from 2 processes). > Results improved from 0.424 loops/sec to 0.432 loops/sec. > Each loop creates 10^3 directories with 10 files in each. >=20 > +++ b/block/io.c > @@ -1294,6 +1294,7 @@ static int coroutine_fn bdrv_aligned_pwritev(Bloc= kDriverState *bs, > } > bdrv_debug_event(bs, BLKDBG_PWRITEV_DONE); > =20 > + ++bs->write_gen; Why pre-increment? Most code uses post-increment when done as a statement in isolation. > bdrv_set_dirty(bs, start_sector, end_sector - start_sector); > =20 > if (bs->wr_highest_offset < offset + bytes) { > @@ -2211,6 +2212,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *= bs) > { > int ret; > BdrvTrackedRequest req; > + int current_gen =3D bs->write_gen; > =20 > if (!bs || !bdrv_is_inserted(bs) || bdrv_is_read_only(bs) || > bdrv_is_sg(bs)) { > @@ -2219,6 +2221,12 @@ int coroutine_fn bdrv_co_flush(BlockDriverState = *bs) > =20 > tracked_request_begin(&req, bs, 0, 0, BDRV_TRACKED_FLUSH); > =20 > + /* Wait until any previous flushes are completed */ > + while (bs->flush_started_gen !=3D bs->flushed_gen) { Should this be an inequality, as in s/!=3D/ + qemu_co_queue_wait(&bs->flush_queue); > + } > + bs->flush_started_gen =3D current_gen; > + > /* Write back all layers by calling one driver function */ > if (bs->drv->bdrv_co_flush) { > ret =3D bs->drv->bdrv_co_flush(bs); > @@ -2239,6 +2247,11 @@ int coroutine_fn bdrv_co_flush(BlockDriverState = *bs) > goto flush_parent; > } > =20 > + /* Check if we really need to flush anything */ > + if (bs->flushed_gen =3D=3D current_gen) { Likewise, if you are tracking generations, should this be s/=3D=3D/<=3D/ = (am I getting the direction correct)? > +++ b/include/block/block_int.h > @@ -420,6 +420,11 @@ struct BlockDriverState { > note this is a reference count */ > bool probed; > =20 > + CoQueue flush_queue; /* Serializing flush queue */ > + unsigned int write_gen; /* Current data generation */ > + unsigned int flush_started_gen; /* Generation for which flush has = started */ > + unsigned int flushed_gen; /* Flushed write generation */ Should these be 64-bit integers to avoid risk of overflow after just 2^32 flush attempts? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --5pav4uAF45oTUnQSoMokAIm9WHh6Wljn6 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXfuAQAAoJEKeha0olJ0NqtOIH/jnW4/Auc+dzFat338HgpIFi cUFF9Rx86tz7DGFAIO1m63BechHjqPxmCgHMKgfP1tqwbVOD/JpBUjHRjA3y/JOr TiDht7tD8lwLVGj+r4/eWNOeENB4Ml7HQOZ8f3ehTq/oMiFHeq+KwANQjG3xAqjv htX/3EuXFpuD7F3NVoq/4qmHCf8M47AhiYbU2JtGUYgGdeC5FAMVQzJ+Bn7NdkXq Ux8wlpLdik3nkOAbVNcMuoWtOhZN4yN29VmF8qO3CoY4Kq7u4G4xt5f3JzNyeAqP bBUfFGPJ4PP3Q8ikAHaS4l/I+xaSPiKcPaXsSIz+qmLTteb7ZpbDxykrlNFNcXI= =eczu -----END PGP SIGNATURE----- --5pav4uAF45oTUnQSoMokAIm9WHh6Wljn6--