From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33570) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLlvd-0003Re-Ls for qemu-devel@nongnu.org; Wed, 20 Jan 2016 01:07:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLlvc-00036n-M9 for qemu-devel@nongnu.org; Wed, 20 Jan 2016 01:07:37 -0500 Date: Wed, 20 Jan 2016 14:07:24 +0800 From: Fam Zheng Message-ID: <20160120060724.GA28390@ad.usersys.redhat.com> References: <1451903234-32529-1-git-send-email-famz@redhat.com> <1451903234-32529-9-git-send-email-famz@redhat.com> <568EBCD3.10203@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <568EBCD3.10203@redhat.com> Subject: Re: [Qemu-devel] [PATCH 08/13] block: Support meta dirty bitmap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: Kevin Wolf , Jeff Cody , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org, qemu-block@nongnu.org On Thu, 01/07 14:30, John Snow wrote: > > +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap) > > +{ > > + assert(bitmap->meta); > > + hbitmap_free(bitmap->meta); > > This leaves a dangling pointer inside the Hbitmap, no? Yes, will fix. > > > + bitmap->meta = NULL; > > +} > > + > > +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs, > > + BdrvDirtyBitmap *bitmap, int64_t sector, > > + int nb_sectors) > > +{ > > + uint64_t i; > > + int gran = bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS; > > + > > + /* To optimize: we can make hbitmap to internally check the range in a > > + * coarse level, or at least do it word by word. */ > > + for (i = sector; i < sector + nb_sectors; i += gran) { > > + if (hbitmap_get(bitmap->meta, i)) { > > + return true; > > + } > > + } > > + return false; > > +} > > + > > In essence get_meta() is a greedy algorithm that simply returns true if > anything is set between [sector, sector + nb_sectors], yes? > > Is this more useful than just using an iterator directly on the > meta-bitmap? > > I haven't finished reading but, I imagine that: > > - If we need to check to see what is dirty specifically, we can just use > the iterator. If the iterator doesn't return anything, we know it's > empty. If it does return, we know exactly what's dirty. > - If we need to explicitly check for emptiness in general, we can use > the internal popcount. > > > I'm not sure when a 'dirty range bool' will be explicitly useful all by > itself, but maybe that becomes obvious later. It's for the meta bitmap user to decide. In the case of persistent dirty bitmap driver, I simply check whether the range of write request is meta-dirty, and write the corresponding dirty bitmap range accordingly, rather than splitting one write req into potentially multiple bit ranges that are meta-dirty. I think this is reasonable, hence the interface.