On Mon, Aug 29, 2011 at 6:20 AM, Paolo Bonzini <pbonz...@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
