Quoting Alexey Kardashevskiy (2015-05-26 09:17:42)
> On 05/27/2015 12:03 AM, Paolo Bonzini wrote:
> >
> >
> > On 26/05/2015 16:00, Alexey Kardashevskiy wrote:
> >> On 05/26/2015 11:48 PM, Paolo Bonzini wrote:
> >>>
> >>>
> >>> On 26/05/2015 15:42, Alexey Kardashevskiy wrote:
> >>>>
> >>>>
> >>>> The next patch of this patchset changes:
> >>>> spapr_tce_table_do_enable()
> >>>> memory_region_init_iommu(&iommu)
> >>>> memory_region_add_subregion(&root, &iommu)
> >>>>
> >>>> spapr_tce_table_disable()
> >>>> memory_region_del_subregion(&root, &iommu)
> >>>> object_unref(&iommu)
> >>>>
> >>>> These spapr_tce_xxx are called by request from the guest. &root is a
> >>>> container and exists as long as sPAPRTCETable exists.
> >>>>
> >>>> Where do I get a leaking child property here?
> >>>
> >>> When you unref iommu and not unparent it. The next
> >>> memory_region_init_iommu creates a second child property, and the first
> >>> is gone.
> >>
> >> But when do I get this child property? In memory_region_add_subregion()?
> >> And memory_region_del_subregion() does not do the opposite thing
> >> (unparent)?
> >
> > In memory_region_init_iommu.
>
> Ah. So I need at least s/object_unref/object_unparent/ in my current code,
> right?
I've actually tried that. In that case, FlatView still holds a
reference to the region, and when RCU thread finally unrefs, it sees
that the MR no longer as an owner in the code below:
void memory_region_unref(MemoryRegion *mr)
{
Object *obj = OBJECT(mr);
if (obj && obj->parent) {
object_unref(obj->parent);
} else {
object_unref(obj);
}
g_free(ref_tag);
}
Since the region, prior to the object_unparent(), had an owner, it
gave it's ownership over to the owner (which takes a ref on it as
a child property) and unref'd itself. Now that it's been orphaned,
any attempts by RCU to call memory_region_unref() on it result in
it MR attempting to unref itself, rather than it's owner, but it's
ref is already 0: object_unparent() caused it to be finalized already,
so we actually end up triggering the same assertion as with
object_unref: g_assert(obj->ref > 0);
When we use object_unref() instead of object_unparent(), we hit that
same assertion when the owner get's finalized, since it still sees the
MR as a child and attempts to unref it again.
>
> >>> What is different between the various IOMMU regions, so that you cannot
> >>> create just one?
> >>
> >> There are two DMA windows on the same PCI bus (in hardware too), at
> >> different offset and with a different page size.
> >
> > Why do you need different regions? Why can't you have always the same
> > IOMMU regions, and either:
>
> They may change a size. These are dynamic DMA windows, guest may remove all
> and create randomly. Each region is backed by a separate TCE table with
> different page size.
>
> > 1) create/destroy an alias to that region
>
> How does this change things compared to iommus in regard to parenting?
>
>
> > 2) change the behavior of the translation function, while keeping a
> > single region?
>
> Have one sPAPRTCETable object with 0, 1 or 2 (and potentially more) actual
> TCE tables? I can do that too but I thought subregions are just natural for
> that. I even wanted to create sPAPRTCETable' dynamically but this would
> break migration (because we cannot start QEMU with an additional
> sPAPRTCETable if it exists in the source which is not always the case).
>
> Ok. I'll redo this thing again and try using less QOM objects...
>
>
> --
> Alexey
>
