From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51586) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aM0aa-0000HH-0n for qemu-devel@nongnu.org; Wed, 20 Jan 2016 16:46:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aM0aZ-00019k-41 for qemu-devel@nongnu.org; Wed, 20 Jan 2016 16:46:51 -0500 References: <1451903234-32529-1-git-send-email-famz@redhat.com> <1451903234-32529-9-git-send-email-famz@redhat.com> <568EBCD3.10203@redhat.com> <20160120060724.GA28390@ad.usersys.redhat.com> From: John Snow Message-ID: <56A00043.8000100@redhat.com> Date: Wed, 20 Jan 2016 16:46:43 -0500 MIME-Version: 1.0 In-Reply-To: <20160120060724.GA28390@ad.usersys.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: Fam Zheng Cc: Kevin Wolf , Jeff Cody , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org, qemu-block@nongnu.org On 01/20/2016 01:07 AM, Fam Zheng wrote: > 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. > OK, I think I see what the use case is, thanks.