Re: [Qemu-devel] [PULL v1 0/7] MMIO Exec pull request

From: KONRAD Frederic <frederic.konrad@adacore.com>
To: Peter Maydell <peter.maydell@linaro.org>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PULL v1 0/7] MMIO Exec pull request
Date: Fri, 21 Jul 2017 11:38:16 +0200	[thread overview]
Message-ID: <b3fb9a23-79e9-8dda-527d-a4095be04dfd@adacore.com> (raw)
In-Reply-To: <CAFEAcA9Odi+ZtT1FmDoD8pjk=cgHS54BOVwopqXRi+TTf4PA8g@mail.gmail.com>

On 07/21/2017 11:29 AM, Peter Maydell wrote:
> On 21 July 2017 at 10:13, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
>> I don't fully understand the way memory_region_do_invalidate_mmio_ptr
>> works; I see it dropping the memory region; if that's also dropping
>> the RAMBlock then it will upset migration.   Even if the CPU is stopped
>> I dont think that stops the migration thread walking through the list of
>> RAMBlocks.
> 
> memory_region_do_invalidate_mmio_ptr() calls memory_region_unref(),
> which will eventually result in memory_region_finalize() being
> called, which will call the MR destructor, which in this case is
> memory_region_destructor_ram(), which calls qemu_ram_free() on
> the RAMBlock, which removes the RAMBlock from the list (after
> taking the ramlist lock).
> 
>> Even then, the problem is migration keeps a 'dirty_pages' count which is
>> calculated at the start of migration and updated as we dirty and send
>> pages; if we add/remove a RAMBlock then that dirty_pages count is wrong
>> and we either never finish migration (since dirty_pages never reaches
>> zero) or finish early with some unsent data.
>> And then there's the 'received' bitmap currently being added for
>> postcopy which tracks each page that's been received (that's not in yet
>> though).
> 
> It sounds like we really need to make migration robust against
> RAMBlock changes -- in the hotplug case it's certainly possible
> for RAMBlocks to be newly created or destroyed while migration
> is in progress.
> 

For the RAMBlock destruction maybe we can just ref the
MemoryRegion but I think I remember some strangeness like
memory_region_ref actually add a reference on the owner..

And it doesn't help for hotplug..

Fred

> thanks
> -- PMM
> 