From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44171) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VC5h3-0006AX-GF for qemu-devel@nongnu.org; Wed, 21 Aug 2013 06:31:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VC5gw-00085W-1Z for qemu-devel@nongnu.org; Wed, 21 Aug 2013 06:31:13 -0400 Received: from mail6.webfaction.com ([74.55.86.74]:60903 helo=smtp.webfaction.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VC5gv-000859-RB for qemu-devel@nongnu.org; Wed, 21 Aug 2013 06:31:05 -0400 Message-ID: <521496E9.4010901@ctshepherd.com> Date: Wed, 21 Aug 2013 11:31:05 +0100 From: Charlie Shepherd MIME-Version: 1.0 References: <1377023667-20256-1-git-send-email-charlie@ctshepherd.com> <5213D628.4030409@redhat.com> <5213F37C.8090300@ctshepherd.com> <521476ED.5030308@redhat.com> <52148453.8030109@ctshepherd.com> <5214861D.6090304@redhat.com> In-Reply-To: <5214861D.6090304@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] Make cow_co_is_allocated and cow_update_bitmap more efficient List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, stefanha@gmail.com, gabriel@kerneis.info, qemu-devel@nongnu.org On 21/08/2013 10:19, Paolo Bonzini wrote: > Il 21/08/2013 11:11, Charlie Shepherd ha scritto: >> It still seems >> worthwhile to me to be as efficient as possible, I guess that means >> processing a sector's worth of metadata at a time? > Yes, that's what my patches do. My is_allocated and flushing strategy + > something like your replacement of cow_set_bit (just without the > unbounded allocation) should be pretty good. > > Perhaps you can use a cow_co_is_allocated loop after writing the data. > If it returns 0, you flush (the first time only) and call your > cow_update_bitmap. Then you advance by num_same sectors and go on until > you did all the nb_sectors. The disadvantage is that it does two reads > (one in cow_co_is_allocated, one in cow_update_bitmap). The advantage > is that unbounded allocation goes away because cow_co_is_allocated will > never consider more than a sector of bitmap data. And you can reuse all > your cow_update_bitmap code. Agreed. But can the two functions not share the same read data? As in: cow_is_allocated(char *buf) { return test_bits(buf) } cow_co_is_allocated(Bdrv *b) { char sector[SECTOR_SIZE] bdrv_read(b, sector, sector_num) return cow_is_allocated(sector) } cow_update_bitmap(Bdrv *b) { while (sector) { char buf[SECTOR_SIZE] bdrv_read(b, buf, sector_num) if (cow_is_allocated(buf)) { if (!flushed) bdrv_flush() cow_update_bitmap_sector(buf) bdrv_write(buf) } } } > To be as efficient as possible, you could keep a memory copy of the > bitmap, so that you only have to do writes, not reads. The memory copy > would be somewhat expensive of course but perhaps reasonable (256M of > memory for a 1T image). The largest cost would be loading the bitmap > from memory at startup. But that can be done later. Yes I considered that option too. I guess it can be considered as another patch, with the performance implicated measured against the results of this patch. Charlie