Re: [Qemu-ppc] [PATCH qemu v7 06/14] spapr_iommu: Introduce "enabled" state for TCE table

[Alexey Kardashevskiy]

On 05/26/2015 11:31 PM, Paolo Bonzini wrote:
> On 26/05/2015 15:28, Alexey Kardashevskiy wrote:
> > My initial concern was if I can or cannot do:
> >
> > memory_region_init_iommu + memory_region_add_subregion
> > and
> > memory_region_del_subregion + object_unref
> >
> > outside of init/realize/unrealize/finalize.
> >
> > You said I cannot do unparenting but as I am not doing this (and I just
> > do unref()) - I am fine. That's what I meant.
>
> Well, if you do the above you have two different bugs:
>
> 1) you leak the original child property
>
> 2) you initialize the second region on top of the first, so you have two
> regions pointing to the same memory


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?


> This is even worse than unparenting :) and would have been wrong even
> without the RCU changes.

I believe you :) But do not understand :)


--
Alexey