On Wed, Jul 08, 2015 at 04:43:55PM +0200, Igor Mammedov wrote:
> On Wed, 8 Jul 2015 12:58:55 +0300
> "Michael S. Tsirkin" <m...@redhat.com> wrote:
>
> > On Wed, Jul 08, 2015 at 11:46:48AM +0200, Igor Mammedov wrote:
> > > Although memory_region_del_subregion() removes MemoryRegion
> > > from current address space, it's possible that it's still
> > > in use/referenced until old address space view is destroyed.
> > > That doesn't allow to unmap it from HVA region at the time
> > > of memory_region_del_subregion().
> > > As a solution track HVA mapped MemoryRegions in a list and
> > > don't allow to map another MemoryRegion at the same address
> > > until respective MemoryRegion is destroyed, delaying unmapping
> > > from HVA range to the time MemoryRegion destructor is called.
> > >
> > > In memory hotplug terms it would mean that user should delete
> > > corresponding backend along with pc-dimm device:
> > > device_del dimm1
> > > object_del dimm1_backend_memdev
> > > after that dimm1_backend_memdev's MemoryRegion will be destroyed
> > > once all accesses to it are gone and old flatview is destroyed as
> > > well.
> > > As result it's possible that a following "device_add pc-dimm" at
> > > the same address may fail due to old mapping is still being present,
> >
> >
> > s/is still/still/
> fixed
>
> >
> > > hence add error argument to memory_region_add_subregion() API
> > > so it could report error and hotplug could be cancelled gracefully.
> > >
> > > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> >
> > The commit log seems a bit confusing.
> > API was added in previous patch, and this one actually
> > uses it.
> previous patch added qemu_ram_* API utilities at exec.c
> but this patch adds memory_region_* API changes that would allow
> to delete HVA mapped region safely.
I see two changes in memory.h, all private fields.
> Is there any suggestion how to make commit message less confusing.
Just make it match what the patch does.
> > > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > > index ce0320a..d9c53f9 100644
> > > --- a/include/exec/memory.h
> > > +++ b/include/exec/memory.h
> > > @@ -174,6 +174,7 @@ struct MemoryRegion {
> > > bool romd_mode;
> > > bool ram;
> > > void *rsvd_hva;
> > > + bool hva_mapped;
> > > bool skip_dump;
> > > bool readonly; /* For RAM regions */
> > > bool enabled;
> > > @@ -188,6 +189,7 @@ struct MemoryRegion {
> > > QTAILQ_HEAD(subregions, MemoryRegion) subregions;
> > > QTAILQ_ENTRY(MemoryRegion) subregions_link;
> > > QTAILQ_HEAD(coalesced_ranges, CoalescedMemoryRange) coalesced;
> > > + QTAILQ_ENTRY(MemoryRegion) hva_link;
> > > const char *name;
> > > uint8_t dirty_log_mask;
> > > unsigned ioeventfd_nb;
