From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O1HWg-0004yn-VN for qemu-devel@nongnu.org; Mon, 12 Apr 2010 07:09:59 -0400 Received: from [140.186.70.92] (port=50851 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O1HWf-0004xd-IE for qemu-devel@nongnu.org; Mon, 12 Apr 2010 07:09:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O1HWd-0004Jo-EC for qemu-devel@nongnu.org; Mon, 12 Apr 2010 07:09:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:3241) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O1HWd-0004JY-4L for qemu-devel@nongnu.org; Mon, 12 Apr 2010 07:09:55 -0400 Message-ID: <4BC2FF77.8020603@redhat.com> Date: Mon, 12 Apr 2010 14:09:43 +0300 From: Avi Kivity MIME-Version: 1.0 References: <1270515084-24120-1-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <1270515084-24120-4-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <4BC2D562.7050507@redhat.com> <4BC2FCDF.2000909@lab.ntt.co.jp> In-Reply-To: <4BC2FCDF.2000909@lab.ntt.co.jp> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH v2 3/6] Modifies wrapper functions for byte-based phys_ram_dirty bitmap to bit-based phys_ram_dirty bitmap. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yoshiaki Tamura Cc: aliguori@us.ibm.com, kvm@vger.kernel.org, ohmura.kei@lab.ntt.co.jp, mtosatti@redhat.com, qemu-devel@nongnu.org On 04/12/2010 01:58 PM, Yoshiaki Tamura wrote: >> Is it necessary to update migration and vga bitmaps? >> >> We can simply update the master bitmap, and update the migration and vga >> bitmaps only when they need it. That can be done in a different patch. > > > Let me explain the role of the master bitmap here. > > In the previous discussion, the master bitmap represented at least one > dirty type is actually dirty. While implementing this approach, I > though there is one downside that upon resetting the bitmap, it needs > to check all dirty types whether they are already cleared to unset the > master bitmap, and this might hurt the performance in general. The way I see it, the master bitmap is a buffer between the guest and all the other client bitmaps (migration and vga). So, a bit can be set in vga and cleared in master. Let's look at the following scenario: 1. guest touches address 0xa0000 (page 0xa0) 2. qemu updates master bitmap bit 0xa0 3. vga timer fires 4. vga syncs the dirty bitmap for addresses 0xa0000-0xc0000 4.1 vga_bitmap[0xa0-0xc0] |= master_bitmap[0xa0-0xc0] 4.2 migration_bitmap[0xa0-0xc0] |= master_bitmap[0xa0-0xc0] 4.3 master_bitmap[0xa0-0xc0] = 0 5. vga draws based on vga_bitmap the nice thing is that step 4.2 can be omitted if migration is not active. so, client bitmaps are always updated when the client requests them, master bitmap is only a buffer. > > In this patch, master bitmap represents all types are dirty, similar > to existing 0xff. With this approach, resetting the master bitmap can > be done without checking the other types. set_dirty_flags is actually > taking the burden in this case though. Anyway, IIUC somebody would be > unhappy depending on the role of the master bitmap. Yes, the problem is that set_dirty_flags is the common case and also uses random access, we'd want to make it touch only a single bit. client access is rare, and also sequential, and therefore efficient. > >> Note that we should only allocate the migration and vga bitmaps when >> migration or vga is active. > > Right. May I do it in a different patch? Sure, more patches is usually better. > I think this is about optimization. In this patch, I focused on not > breaking the existing function. Yes, that's the best approach. But we do need to see how all the incremental steps end up. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.