[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 37/47] Page request: Consume pages off the po
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v4 37/47] Page request: Consume pages off the post-copy queue |
Date: |
Tue, 7 Oct 2014 12:35:10 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Paolo Bonzini (address@hidden) wrote:
> Il 03/10/2014 19:47, Dr. David Alan Gilbert (git) ha scritto:
> > + /*
> > + * Don't break host-page chunks up with queue items
> > + * so only unqueue if,
> > + * a) The last item came from the queue anyway
> > + * b) The last sent item was the last target-page in a host page
> > + */
> > + if (last_was_from_queue || (!last_sent_block) ||
>
> Extra parentheses.
Fixed.
> Is the last_was_from_queue check necessary? Or
> would one of the other checks be true anyway if last_was_from_queue is true?
if (last_was_from_queue || !last_sent_block ||
((last_offset & (hps - 1)) == (hps - TARGET_PAGE_SIZE))) {
tmpblock = ram_save_unqueue_page(ms, &tmpoffset, &bitoffset);
}
The last_was_from_queue is needed. We're going around this loop in Target-page
chunks (that correspond to one bit in the migration bitmap) and want to make
sure
we don't break into the middle of an existing host-page with an entry off the
queue.
So (going backwards in that if):
We can take something from the queue if:
a) We just sent the last TP in a HP
b) We didn't send anything yet (unlikely)
c) The last thing came from the queue anyway
(c) is needed to override (a) when we've just sent a TP from the queue but it's
not the last TP in the HP that came from the queue; otherwise we'd send the
first
TP from the queue and resume taking pages from the background scan.
> > + /* We're sending this page, and since it's postcopy nothing
> > else
> > + * will dirty it, and we must make sure it doesn't get sent
> > again.
> > + */
> > + if (!migration_bitmap_clear_dirty(bitoffset <<
> > TARGET_PAGE_BITS)) {
> > + DPRINTF("%s: Not dirty for postcopy %s/%zx bito=%zx
> > (sent=%d)",
> > + __func__, tmpblock->idstr, tmpoffset, bitoffset,
> > + test_bit(bitoffset, ms->sentmap));
>
> If a DPRINTF occurs in a loop, please change it to a tracepoint.
OK, I'll look at that - most of arch_init (and migration) still use DPRINTF.
> This function looks like a candidate for cleaning its logic up and/or
> splitting it. But it can be done later by the poor soul who will touch
> it next. :)
Yep, I've already done it once (see 14bcfdc7f - Split ram_save_block).
Dave
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- Re: [Qemu-devel] [PATCH v4 33/47] Postcopy: Postcopy startup in migration thread, (continued)
[Qemu-devel] [PATCH v4 37/47] Page request: Consume pages off the post-copy queue, Dr. David Alan Gilbert (git), 2014/10/03
[Qemu-devel] [PATCH v4 38/47] Add assertion to check migration_dirty_pages, Dr. David Alan Gilbert (git), 2014/10/03
[Qemu-devel] [PATCH v4 40/47] Postcopy: Use helpers to map pages during migration, Dr. David Alan Gilbert (git), 2014/10/03
[Qemu-devel] [PATCH v4 41/47] qemu_ram_block_from_host, Dr. David Alan Gilbert (git), 2014/10/03
[Qemu-devel] [PATCH v4 42/47] Don't sync dirty bitmaps in postcopy, Dr. David Alan Gilbert (git), 2014/10/03
[Qemu-devel] [PATCH v4 43/47] Host page!=target page: Cleanup bitmaps, Dr. David Alan Gilbert (git), 2014/10/03
[Qemu-devel] [PATCH v4 44/47] Postcopy; Handle userfault requests, Dr. David Alan Gilbert (git), 2014/10/03
[Qemu-devel] [PATCH v4 45/47] Start up a postcopy/listener thread ready for incoming page data, Dr. David Alan Gilbert (git), 2014/10/03