Date: Wed, 6 May 2015 15:57:08 +0100
From: Stefan Hajnoczi <stefanha@gmail.com>
Message-ID: <20150506145708.GD2386@stefanha-thinkpad.redhat.com>
References: <cover.1430388393.git.berto@igalia.com>
	<9b9c36f8c567da1f9561828380e8aa42c08b7efd.1430388393.git.berto@igalia.com>
	<20150501142321.GB2386@stefanha-thinkpad.redhat.com>
	<20150504105813.GD3676@noname.str.redhat.com>
	<20150505102858.GC3149@stefanha-thinkpad.redhat.com>
	<20150505112019.GB3866@noname.redhat.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
	protocol="application/pgp-signature"; boundary="VV4b6MQE+OnNyhkM"
Content-Disposition: inline
In-Reply-To: <20150505112019.GB3866@noname.redhat.com>
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 1/6] qcow2: use one single
 memory block for the L2/refcount cache tables
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-block@nongnu.org, Alberto Garcia <berto@igalia.com>, qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>


--VV4b6MQE+OnNyhkM
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, May 05, 2015 at 01:20:19PM +0200, Kevin Wolf wrote:
> Am 05.05.2015 um 12:28 hat Stefan Hajnoczi geschrieben:
> > On Mon, May 04, 2015 at 12:58:13PM +0200, Kevin Wolf wrote:
> > > Am 01.05.2015 um 16:23 hat Stefan Hajnoczi geschrieben:
> > > > On Thu, Apr 30, 2015 at 01:11:40PM +0300, Alberto Garcia wrote:
> > > > >  Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tab=
les)
> > > > >  {
> > > > >      BDRVQcowState *s =3D bs->opaque;
> > > > >      Qcow2Cache *c;
> > > > > -    int i;
> > > > > =20
> > > > >      c =3D g_new0(Qcow2Cache, 1);
> > > > >      c->size =3D num_tables;
> > > > > +    c->table_size =3D s->cluster_size;
> > > >=20
> > > > This assumes c->table_size meets bs' memory alignment requirements.=
  The
> > > > following would be safer:
> > > >=20
> > > > c->table_size =3D QEMU_ALIGN_UP(c->table_size, bdrv_opt_mem_align(b=
s->file));
> > >=20
> > > You can't just access more than one cluster. You might be caching data
> > > and later write it back when it's stale.
> >=20
> > I don't mean I/O alignment, just memory alignment (i.e. the start
> > address of the data buffer in memory).
>=20
> The start address is already taken care of by qemu_blockalign(). With
> rounding c->table_size, you'd align the length, and that would be wrong.

It wasn't clear to me that c->table + n * c->table_size for all n is
aligned, but I agree with you now:

bdrv_qiov_is_aligned() checks both address and size against memory
alignment.  This means that if the I/O is memory aligned for table_size,
then all multiples of table_size are also aligned.

Stefan

--VV4b6MQE+OnNyhkM
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJVSivEAAoJEJykq7OBq3PIR+sH/RXUipQtWEs5WN/kORzPadru
NyHUk/afvYJZuRucH4HQCMkeOddNKBoanwvzd9Ezec+YET0JsChLVLHS+QU4EpVd
MizbLRt7+NH46s0ZBwug4iGdrlt498VnAwGKKfBNiHb2Cx6L6JW52MWxfL+MqLxs
pH+lla2Ws5ZkTx9Y0ZmTWx8GWQVYUjBJXxmu4z+m+0w1sw4NydZAh94PGMxb9jFM
qzUrCBXeEEiyRQ7c1Un6BsKa0oyyTJHbmBYMx1zg44wnuI96+6j+euoVN4uRUq1v
ZxqAPA9H5ekEXVdNXrfuPs2VfbgfDDFCj1nE59s4Ln9vb4eb+op9HOMo9TxzLKA=
=Vx/y
-----END PGP SIGNATURE-----

--VV4b6MQE+OnNyhkM--