Re: [PATCH 0/5] Separate thread for VM migration

Umesh Deshpande
2011-08-31T03:53:30Z

[-- Attachment #1: Type: text/plain, Size: 3011 bytes --]

On Mon, Aug 29, 2011 at 6:20 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 08/27/2011 08:09 PM, Umesh Deshpande wrote:
>
>> Following patch series deals with VCPU and iothread starvation during the
>> migration of a guest. Currently the iothread is responsible for performing
>> the
>> guest migration. It holds qemu_mutex during the migration and doesn't
>> allow VCPU
>> to enter the qemu mode and delays its return to the guest. The guest
>> migration,
>> executed as an iohandler also delays the execution of other iohandlers.
>> In the following patch series,
>>
>> The migration has been moved to a separate thread to
>> reduce the qemu_mutex contention and iohandler starvation.
>>
>> Umesh Deshpande (5):
>> vm_stop from non-io threads
>> MRU ram block list
>> migration thread mutex
>> separate migration bitmap
>> separate migration thread
>>
>> arch_init.c | 38 +++++++++++++----
>> buffered_file.c | 76 ++++++++++++++++++------------**---
>> cpu-all.h | 42 ++++++++++++++++++
>> cpus.c | 4 +-
>> exec.c | 97 ++++++++++++++++++++++++++++++**++++++++++--
>> migration.c | 117 ++++++++++++++++++++++++++++++**
>> ++-------------------
>> migration.h | 9 ++++
>> qemu-common.h | 2 +
>> qemu-thread-posix.c | 10 ++++
>> qemu-thread.h | 1 +
>> 10 files changed, 302 insertions(+), 94 deletions(-)
>>
>>
> I think this patchset is quite good. These are the problems I found:
>
> 1) the locking rules in patch 3 are a bit too clever, and the cleverness
> will become obsolete once RCU is in place. The advantage of the clever
> stuff is that rwlock code looks more like RCU code; the disadvantage is that
> it is harder to read and easier to bikeshed about.
>
> 2) it breaks Windows build, but that's easy to fix.
>
> 3) there are a _lot_ of cleanups possible on top of patch 5 (I would not be
> too surprised if the final version has an almost-neutral diffstat), and
> whether to prefer good or perfect is another very popular topic.
>
> 4) I'm not sure block migration has been tested in all scenarios (I'm
> curious about coroutine-heavy ones). This may mean that the migration
> thread is blocked onto Marcelo's live streaming work, which would provide
> the ingredients to remove block migration altogether. A round of Autotest
> testing is the minimum required to include this, and I'm not sure if this
> was done either.
>
>
> That said, I find the code to be quite good overall, and I wouldn't oppose
> inclusion with only (2) fixed---may even take care of it myself---and more
> testing results apart from the impressive performance numbers.
>
> About performance, I'm curious how you measured it. Was the buffer cache
> empty? That is, how many compressible pages were found? I toyed with
> vectorizing is_dup_page, but I need to get some numbers before posting.

Above tests were run with an idle VM. I didn't measure the number of
compressible pages.

- Umesh

[-- Attachment #2: Type: text/html, Size: 3591 bytes --]