On 04/13/2010 11:01 AM, Yoshiaki Tamura wrote:
Avi Kivity wrote:
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.
Thanks. I think I'm finally getting your point.
At 4.2 and 4.3, is syncing to client necessary?
Yes. We want to clear the master bitmap, otherwise on the next iteration
we will get dirty bits that may be spurious. But if we clear the dirty
bitmap, we must copy it to all client bitmaps, not just vga, otherwise
we lose data.
Simply doing OR at get_dirty like in this patch not enough?
If you don't clear the master bitmap, then you will get the same bits at
the next iteration.
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.
I see.
So set_dirty_flags would be like,
1. set master first regard less of dirty type.
2. if dirty type was CODE, set the client.
There really should be two APIs, one for general dirtyness and one for
code dirty. As Anthony said, the code dirty bitmap is completely
separate from the other uses.
Consider starting by separating the two uses, it may clear up the code
and make things easier later on.
In summary, I'll prepare two patch sets.
1. v3 of this patch set.
- Change FLAGS value to (1,2,4,8), and add IDX (0,1,2,3)
- Use ffsl to convert FLAGS to IDX.
- Add help function which takes IDX.
- Change the behavior of set_dirty_flags as above.
- Change dirty bitmap access to a loop.
- Add brace after if ()
- Move some macros to qemu-common.h.
2. Allocate vga and migration dirty bitmap dynamically.
Please modify or add items if I were missing.
I would add separation of CODE and the other dirty bitmaps.