[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH 1/6] qcow2: use one single memory b
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH 1/6] qcow2: use one single memory block for the L2/refcount cache tables |
Date: |
Wed, 6 May 2015 15:57:08 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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_tables)
> > > > > {
> > > > > BDRVQcowState *s = bs->opaque;
> > > > > Qcow2Cache *c;
> > > > > - int i;
> > > > >
> > > > > c = g_new0(Qcow2Cache, 1);
> > > > > c->size = num_tables;
> > > > > + c->table_size = s->cluster_size;
> > > >
> > > > This assumes c->table_size meets bs' memory alignment requirements. The
> > > > following would be safer:
> > > >
> > > > c->table_size = QEMU_ALIGN_UP(c->table_size,
> > > > bdrv_opt_mem_align(bs->file));
> > >
> > > You can't just access more than one cluster. You might be caching data
> > > and later write it back when it's stale.
> >
> > I don't mean I/O alignment, just memory alignment (i.e. the start
> > address of the data buffer in memory).
>
> 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
pgpbsrSdrUBjG.pgp
Description: PGP signature